All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luis Henriques <luis@igalia.com>
To: Sang-Heon Jeon <ekffu200098@gmail.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	 fuse-devel@lists.linux.dev, linux-kernel@vger.kernel.org,
	 Miklos Szeredi <mszeredi@redhat.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH] fuse: defer dentry_tree_work until fuse_dentry_tree_init()
Date: Wed, 11 Nov 2026 13:59:55 +0000	[thread overview]
Message-ID: <87mrrf8o4k.fsf@igalia.com> (raw)
In-Reply-To: <CABFDxMGJ+Uum7dde3AN5Ab+H4Gwb23Upj7r9=SO21NWCo+Mhow@mail.gmail.com>

On Wed, Nov 11 2026, Sang-Heon Jeon wrote:

> 2026년 11월 11일 (수) 오후 8:27, Sang-Heon Jeon <ekffu200098@gmail.com>님이 작성:
>>
>> Hi, Luis
>>
>> 2026년 11월 11일 (수) 오후 6:28, Luis Henriques <luis@igalia.com>님이 작성:
>> >
>> > On Wed, Nov 11 2026, Sang-Heon Jeon wrote:
>> >
>> > > inval_wq_set() schedules dentry_tree_work when inval_wq is enabled.
>> > >
>> > > But inval_wq_set() can be called at boot or module load before
>> > > fuse_dentry_tree_init() initializes the work, so __queue_delayed_work()
>> > > warns and the work never starts.
>> > >
>> > > So defer scheduling the work until fuse_dentry_tree_init() sets it up.
>> > >
>> > > Fixes: ab84ad597386 ("fuse: new work queue to periodically invalidate expired dentries")
>> > > Cc: stable@vger.kernel.org
>> > > Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
>> > > ---
>> > > QEMU based test result
>> > > - booted with "fuse.inval_wq=5"
>> >
>> > Thank you, Sang-Heon.  I've sent a fix for this already[1], but sashiko
>> > still has some race complains which I haven't looked into yet.  But maybe
>> > Miklos is happier with your patch, in which case you can add my
>>
>> Sorry, I missed that thread.. sashiko also complains about the same
>> race condition between init and sysfs setup. But I'm not sure it is a
>> realistic scenario. So I want to hear other voices as well.
>
> I also checked the other sashiko's complaints from your patch, the one
> about race between module cleanup and sysfs. It looks easier to
> reproduce, but still I'm not sure this is a realistic problem. If it
> is worth fixing, I will sent v2 with serialization between module
> init, cleanup and inval_wq_set()

I agree that the first race doesn't look realistic, though it could be
fixed by adding an extra spinlock (which could protect both inval_wq and
dentry_tree_is_initialised).

The second race would be easier to fix by setting
dentry_tree_is_initialised back to 'false' in fuse_dentry_tree_cleanup().

One thing I like with my version of the fix is that it replaces the usage
of schedule_delayed_work() by mod_delayed_work().  But maybe that's out
of the scope of this fix, and I should send that separately.

Cheers,
-- 
Luís


>> > Reviewed-by: Luis Henriques <luis@igalia.com>
>>
>> Thanks for your review :)
>>
>> > [1] https://lore.kernel.org/20261106155512.24801-1-luis%40igalia.com
>> >
>> > Cheers,
>> > --
>> > Luís
>> >
>> > > 1) AS-IS (before fix)
>> > > [    0.011073] Kernel command line: rootwait root=/dev/vda console=tty1 console=ttyS0 nokaslr  fuse.inval_wq=5
>> > > [    0.011114] ------------[ cut here ]------------
>> > > [    0.011115] WARNING: kernel/workqueue.c:2704 at __queue_delayed_work+0x11a/0x140, CPU#0: swapper/0
>> > > [    0.011124] Modules linked in:
>> > > [    0.011128] CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 7.4.0-rc1-00010-gab9787d7e66a #1 PREEMPT(undef)
>> > > [    0.011130] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-debian-1.17.0-1ubuntu1 04/01/2014
>> > > [    0.011131] RIP: 0010:__queue_delayed_work+0x11a/0x140
>> > > [ 0.011133] Code: c4 08 48 89 df 5b e9 85 28 0a 00 90 0f 0b 90 4c 8b 42 08 48 8d
>> > > 42 08 49 39 c0 0f 84 1c ff ff ff 90 0f 0b 90 e9 13 ff ff ff 90 <0f> 0b 90 e9 ec
>> > > fe ff ff bff
>> > > [    0.011134] RSP: 0000:ffffffff82e03e00 EFLAGS: 00010003 ORIG_RAX: 0000000000000000
>> > > [    0.011135] RAX: 0000000000000000 RBX: 0000000000000040 RCX: 0000000000001388
>> > > [    0.011136] RDX: ffffffff83a3f320 RSI: 0000000000000000 RDI: 0000000000000040
>> > > [    0.011136] RBP: 0000000000000000 R08: 0000000000000082 R09: 0000000000000001
>> > > [    0.011136] R10: 000000000000000a R11: 000000000000000a R12: ffffffff82c09fd0
>> > > [    0.011137] R13: ffff88807ffcfb89 R14: 0000000000000000 R15: 0000000000001388
>> > > [    0.011140] FS:  0000000000000000(0000) GS:ffff8880fa26e000(0000) knlGS:0000000000000000
>> > > [    0.011141] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> > > [    0.011141] CR2: ffff888003a67000 CR3: 0000000002e30000 CR4: 00000000000000b0
>> > > [    0.011142] Call Trace:
>> > > [    0.011143]  <TASK>
>> > > [    0.011143]  queue_delayed_work_on+0x6a/0x80
>> > > [    0.011145]  inval_wq_set+0xc4/0xe0
>> > > [    0.011149]  parse_args+0x299/0x3c0
>> > > [    0.011151]  ? __pfx_unknown_bootoption+0x10/0x10
>> > > [    0.011154]  start_kernel+0x3b8/0x7c0
>> > > [    0.011156]  ? __pfx_unknown_bootoption+0x10/0x10
>> > > [    0.011157]  x86_64_start_reservations+0x24/0x30
>> > > [    0.011160]  x86_64_start_kernel+0xd1/0xe0
>> > > [    0.011162]  common_startup_64+0x13e/0x158
>> > > [    0.011165]  </TASK>
>> > > [    0.011166] ---[ end trace 0000000000000000 ]---
>> > > [    0.011173] ------------[ cut here ]------------
>> > >
>> > > 2) TO-BE (after fix)
>> > > - no error while booting, and also fuse_dentry_tree_work works well.
>> > >
>> > > ---
>> > >  fs/fuse/dir.c | 10 ++++++++++
>> > >  1 file changed, 10 insertions(+)
>> > >
>> > > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> > > index da23ccd8685c..6965a77a871f 100644
>> > > --- a/fs/fuse/dir.c
>> > > +++ b/fs/fuse/dir.c
>> > > @@ -39,6 +39,7 @@ static struct delayed_work dentry_tree_work;
>> > >  /* Minimum invalidation work queue frequency */
>> > >  #define FUSE_DENTRY_INVAL_FREQ_MIN 5
>> > >
>> > > +static bool inval_wq_ready;
>> > >  unsigned __read_mostly inval_wq;
>> > >  static int inval_wq_set(const char *val, const struct kernel_param *kp)
>> > >  {
>> > > @@ -62,6 +63,10 @@ static int inval_wq_set(const char *val, const struct kernel_param *kp)
>> > >
>> > >       *((unsigned int *)kp->arg) = num;
>> > >
>> > > +     /* Defer the work until fuse_dentry_tree_init() sets it up. */
>> > > +     if (!inval_wq_ready)
>> > > +             return 0;
>> > > +
>> > >       if (num && !old)
>> > >               schedule_delayed_work(&dentry_tree_work,
>> > >                                     secs_to_jiffies(num));
>> > > @@ -224,6 +229,11 @@ void fuse_dentry_tree_init(void)
>> > >               dentry_hash[i].tree = RB_ROOT;
>> > >       }
>> > >       INIT_DELAYED_WORK(&dentry_tree_work, fuse_dentry_tree_work);
>> > > +     inval_wq_ready = true;
>> > > +
>> > > +     if (inval_wq)
>> > > +             schedule_delayed_work(&dentry_tree_work,
>> > > +                                   secs_to_jiffies(inval_wq));
>> > >  }
>> > >
>> > >  void fuse_dentry_tree_cleanup(void)
>> > >
>> > > base-commit: ab9787d7e66ac7e6e94fc975b37c5b56fff98ca9
>> > > --
>> > > 2.53.0
>> > >
>> >
>>
>> Best regards,
>> Sang-Heon Jeon

  reply	other threads:[~2026-11-11 13:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-10 15:02 [PATCH] fuse: defer dentry_tree_work until fuse_dentry_tree_init() Sang-Heon Jeon
2026-11-11  9:30 ` Luis Henriques
2026-11-11 11:27   ` Sang-Heon Jeon
2026-11-11 13:26     ` Sang-Heon Jeon
2026-11-11 13:59       ` Luis Henriques [this message]
2026-11-11 15:24         ` Sang-Heon Jeon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87mrrf8o4k.fsf@igalia.com \
    --to=luis@igalia.com \
    --cc=ekffu200098@gmail.com \
    --cc=fuse-devel@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@redhat.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.