linux-kernel - Re: [PATCH v3] fuse: new workqueue 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAJfpegue3szRGZs+ogvYjiVt0YUo-=e+hrj-r=8ZDy11Zgrt9w@mail.gmail.com>
Date: Wed, 2 Jul 2025 17:39:58 +0200
From: Miklos Szeredi <miklos@...redi.hu>
To: Luis Henriques <luis@...lia.com>
Cc: Bernd Schubert <bernd@...ernd.com>, Laura Promberger <laura.promberger@...n.ch>, 
	Dave Chinner <david@...morbit.com>, Matt Harvey <mharvey@...ptrading.com>, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] fuse: new workqueue to periodically invalidate expired dentries

On Tue, 20 May 2025 at 17:42, Luis Henriques <luis@...lia.com> wrote:
>
> This patch adds a new module parameter 'inval_wq' which is used to start a
> workqueue to periodically invalidate expired dentries.  The value of this
> new parameter is the period, in seconds, of the workqueue.  When it is set,
> every new dentry will be added to an rbtree, sorted by the dentry's expiry
> time.
>
> When the workqueue is executed, it will check the dentries in this tree and
> invalidate them if:
>
>   - The dentry has timed-out, or if
>   - The connection epoch has been incremented.

I wonder, why not make the whole infrastructure global?  There's no
reason to have separate rb-trees and workqueues for each fuse
instance.  Contention on the lock would be worse, but it's bad as it
is, so need some solution, e.g. hashed lock, which is better done with
a single instance.

>
> The workqueue will run for, at most, 5 seconds each time.  It will
> reschedule itself if the dentries tree isn't empty.

It should check need_resched() instead.

> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 1fb0b15a6088..257ca2b36b94 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -34,33 +34,153 @@ static void fuse_advise_use_readdirplus(struct inode *dir)
>         set_bit(FUSE_I_ADVISE_RDPLUS, &fi->state);
>  }
>
> -#if BITS_PER_LONG >= 64
> -static inline void __fuse_dentry_settime(struct dentry *entry, u64 time)
> +struct fuse_dentry {
> +       u64 time;
> +       struct rcu_head rcu;
> +       struct rb_node node;
> +       struct dentry *dentry;
> +};
> +

You lost the union with rcu_head.   Any other field is okay, none of
them matter in rcu protected code.  E.g.

struct fuse_dentry {
        u64 time;
        union {
                struct rcu_head rcu;
                struct rb_node node;
        };
        struct dentry *dentry;
};

Thanks,
Miklos