linux-kernel - Re: [PATCH v4] fuse: new work queue to periodically invalidate expired dentries

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250819035208.540-1-luochunsheng@ustc.edu>
Date: Tue, 19 Aug 2025 11:52:08 +0800
From: Chunsheng Luo <luochunsheng@...c.edu>
To: luis@...lia.com
Cc: bernd@...ernd.com,
	david@...morbit.com,
	kernel-dev@...lia.com,
	laura.promberger@...n.ch,
	linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	mharvey@...ptrading.com,
	miklos@...redi.hu
Subject: Re: [PATCH v4] fuse: new work queue to periodically invalidate expired dentries

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));
>+}