On Mon, 7 Jul 2025 16:34:13 Luis Henriques wrote:
>+static void fuse_dentry_tree_add_node(struct dentry *dentry)
>+{
>+ struct fuse_conn *fc = get_fuse_conn_super(dentry->d_sb);
>+ struct fuse_dentry *fd = dentry->d_fsdata;
>+ struct fuse_dentry *cur;
>+ struct rb_node **p, *parent = NULL;
>+ bool start_work = false;
>+
>+ if (!fc->inval_wq)
>+ return;
First check.
>+
>+ spin_lock(&fc->dentry_tree_lock);
>+
>+ if (!fc->inval_wq) {
>+ spin_unlock(&fc->dentry_tree_lock);
>+ return;
>+ }
Check again.
I don't think the if (!fc->inval_wq) check needs to be re-evaluated
while holding the lock. The reason is that the inval_wq variable
doesn't appear to require lock protection. It only gets assigned
during fuse_conn_init and fuse_conn_destroy. Furthermore,
in fuse_conn_destroy we set inval_wq to zero without holding a lock,
and then synchronously cancel any pending work items.
Therefore, performing this check twice with if (!fc->inval_wq)
seems unnecessary.
Also, in the subject, it would be more appropriate to change
"work queue" to "workqueue".
Thanks
Chunsheng Luo
>+
>+ start_work = RB_EMPTY_ROOT(&fc->dentry_tree);
>+ __fuse_dentry_tree_del_node(fc, fd);
>+
>+ p = &fc->dentry_tree.rb_node;
>+ while (*p) {
>+ parent = *p;
>+ cur = rb_entry(*p, struct fuse_dentry, node);
>+ if (fd->time > cur->time)
>+ p = &(*p)->rb_left;
>+ else
>+ p = &(*p)->rb_right;
>+ }
>+ rb_link_node(&fd->node, parent, p);
>+ rb_insert_color(&fd->node, &fc->dentry_tree);
>+ spin_unlock(&fc->dentry_tree_lock);
>+
>+ if (start_work)
>+ schedule_delayed_work(&fc->dentry_tree_work,
>+ secs_to_jiffies(fc->inval_wq));
>+}
