* [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation
@ 2026-10-19 12:53 Luis Henriques
2026-11-06 10:18 ` Luis Henriques
2026-11-06 11:33 ` Miklos Szeredi
0 siblings, 2 replies; 4+ messages in thread
From: Luis Henriques @ 2026-10-19 12:53 UTC (permalink / raw)
To: Miklos Szeredi
Cc: luis, mszeredi, fuse-devel, linux-kernel, Matt Harvey,
kernel-dev, stable
If the fuse module is loaded with a 'inval_wq' parameter the inval_wq_set()
function will schedule work into dentry_tree_work. However this worker is
not initialised at this point because fuse_dentry_tree_init() will not have
been executed yet.
This can be fixed by delaying the work scheduling until later if the module
is still being initialised.
While there, also simplify the sched/cancel by using mod_delayed_work()
instead of schedule_delayed_work(). This will result in having the period
being changed immediately instead of waiting for the next reschedule.
Fixes: ab84ad597386 ("fuse: new work queue to periodically invalidate expired dentries")
Cc: stable@vger.kernel.org
Signed-off-by: Luis Henriques <luis@igalia.com>
---
Hi!
My initial approach to fix this was to use DECLARE_DELAYED_WORK() to
initialise the wq statically. But this would then move the problem to
fuse_dentry_tree_work(), as the worker could still be scheduled before
the module finishes initialisation.
Cheers,
--
Luis
fs/fuse/dir.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index e49b4e874b15..23c8259725c9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -43,7 +43,6 @@ unsigned __read_mostly inval_wq;
static int inval_wq_set(const char *val, const struct kernel_param *kp)
{
unsigned int num;
- unsigned int old = inval_wq;
int ret;
if (!val)
@@ -62,11 +61,14 @@ static int inval_wq_set(const char *val, const struct kernel_param *kp)
*((unsigned int *)kp->arg) = num;
- if (num && !old)
- schedule_delayed_work(&dentry_tree_work,
- secs_to_jiffies(num));
- else if (!num && old)
- cancel_delayed_work_sync(&dentry_tree_work);
+ /* Only start/stop workqueue if we're past module initialisation */
+ if (!module_is_coming(kp->mod)) {
+ if (num)
+ mod_delayed_work(system_percpu_wq, &dentry_tree_work,
+ secs_to_jiffies(num));
+ else
+ cancel_delayed_work_sync(&dentry_tree_work);
+ }
return 0;
}
@@ -224,6 +226,14 @@ void fuse_dentry_tree_init(void)
dentry_hash[i].tree = RB_ROOT;
}
INIT_DELAYED_WORK(&dentry_tree_work, fuse_dentry_tree_work);
+
+ /*
+ * Start workqueue immediately if the module was loaded with the
+ * 'inval_wq' parameter set
+ */
+ if (inval_wq)
+ schedule_delayed_work(&dentry_tree_work,
+ secs_to_jiffies(inval_wq));
}
void fuse_dentry_tree_cleanup(void)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation
2026-10-19 12:53 [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation Luis Henriques
@ 2026-11-06 10:18 ` Luis Henriques
2026-11-06 11:33 ` Miklos Szeredi
1 sibling, 0 replies; 4+ messages in thread
From: Luis Henriques @ 2026-11-06 10:18 UTC (permalink / raw)
To: Miklos Szeredi
Cc: mszeredi, fuse-devel, linux-kernel, Matt Harvey, kernel-dev, stable
On Mon, Oct 19, 2026 at 01:53:29PM +0100, Luis Henriques wrote:
> If the fuse module is loaded with a 'inval_wq' parameter the inval_wq_set()
> function will schedule work into dentry_tree_work. However this worker is
> not initialised at this point because fuse_dentry_tree_init() will not have
> been executed yet.
>
> This can be fixed by delaying the work scheduling until later if the module
> is still being initialised.
>
> While there, also simplify the sched/cancel by using mod_delayed_work()
> instead of schedule_delayed_work(). This will result in having the period
> being changed immediately instead of waiting for the next reschedule.
>
> Fixes: ab84ad597386 ("fuse: new work queue to periodically invalidate expired dentries")
> Cc: stable@vger.kernel.org
> Signed-off-by: Luis Henriques <luis@igalia.com>
> ---
> Hi!
>
> My initial approach to fix this was to use DECLARE_DELAYED_WORK() to
> initialise the wq statically. But this would then move the problem to
> fuse_dentry_tree_work(), as the worker could still be scheduled before
> the module finishes initialisation.
Ping.
Any comments, or suggestion on a better way to fix this problem?
Cheers,
--
Luis
> fs/fuse/dir.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index e49b4e874b15..23c8259725c9 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -43,7 +43,6 @@ unsigned __read_mostly inval_wq;
> static int inval_wq_set(const char *val, const struct kernel_param *kp)
> {
> unsigned int num;
> - unsigned int old = inval_wq;
> int ret;
>
> if (!val)
> @@ -62,11 +61,14 @@ static int inval_wq_set(const char *val, const struct kernel_param *kp)
>
> *((unsigned int *)kp->arg) = num;
>
> - if (num && !old)
> - schedule_delayed_work(&dentry_tree_work,
> - secs_to_jiffies(num));
> - else if (!num && old)
> - cancel_delayed_work_sync(&dentry_tree_work);
> + /* Only start/stop workqueue if we're past module initialisation */
> + if (!module_is_coming(kp->mod)) {
> + if (num)
> + mod_delayed_work(system_percpu_wq, &dentry_tree_work,
> + secs_to_jiffies(num));
> + else
> + cancel_delayed_work_sync(&dentry_tree_work);
> + }
>
> return 0;
> }
> @@ -224,6 +226,14 @@ void fuse_dentry_tree_init(void)
> dentry_hash[i].tree = RB_ROOT;
> }
> INIT_DELAYED_WORK(&dentry_tree_work, fuse_dentry_tree_work);
> +
> + /*
> + * Start workqueue immediately if the module was loaded with the
> + * 'inval_wq' parameter set
> + */
> + if (inval_wq)
> + schedule_delayed_work(&dentry_tree_work,
> + secs_to_jiffies(inval_wq));
> }
>
> void fuse_dentry_tree_cleanup(void)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation
2026-10-19 12:53 [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation Luis Henriques
2026-11-06 10:18 ` Luis Henriques
@ 2026-11-06 11:33 ` Miklos Szeredi
2026-11-06 12:29 ` Luis Henriques
1 sibling, 1 reply; 4+ messages in thread
From: Miklos Szeredi @ 2026-11-06 11:33 UTC (permalink / raw)
To: Luis Henriques
Cc: mszeredi, fuse-devel, linux-kernel, Matt Harvey, kernel-dev, stable
On Mon, 19 Oct 2026 at 14:52, Luis Henriques <luis@igalia.com> wrote:
> @@ -62,11 +61,14 @@ static int inval_wq_set(const char *val, const struct kernel_param *kp)
>
> *((unsigned int *)kp->arg) = num;
>
> - if (num && !old)
> - schedule_delayed_work(&dentry_tree_work,
> - secs_to_jiffies(num));
> - else if (!num && old)
> - cancel_delayed_work_sync(&dentry_tree_work);
> + /* Only start/stop workqueue if we're past module initialisation */
> + if (!module_is_coming(kp->mod)) {
See sashiko review:
- kp->mod might be NULL which will Oops module_is_coming()
- it's still racy against fuse_dentry_tree_init
My question is: does the inval_wq_set() even need to work before the
module is initialized? Can't it just return an error and it wouldn't
need to deal with the race.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation
2026-11-06 11:33 ` Miklos Szeredi
@ 2026-11-06 12:29 ` Luis Henriques
0 siblings, 0 replies; 4+ messages in thread
From: Luis Henriques @ 2026-11-06 12:29 UTC (permalink / raw)
To: Miklos Szeredi
Cc: mszeredi, fuse-devel, linux-kernel, Matt Harvey, kernel-dev, stable
On Fri, Nov 06, 2026 at 12:33:50PM +0100, Miklos Szeredi wrote:
> On Mon, 19 Oct 2026 at 14:52, Luis Henriques <luis@igalia.com> wrote:
>
> > @@ -62,11 +61,14 @@ static int inval_wq_set(const char *val, const struct kernel_param *kp)
> >
> > *((unsigned int *)kp->arg) = num;
> >
> > - if (num && !old)
> > - schedule_delayed_work(&dentry_tree_work,
> > - secs_to_jiffies(num));
> > - else if (!num && old)
> > - cancel_delayed_work_sync(&dentry_tree_work);
> > + /* Only start/stop workqueue if we're past module initialisation */
> > + if (!module_is_coming(kp->mod)) {
>
> See sashiko review:
*sigh*
I keep forgetting to check sashiko website. Sorry about that :-/
> - kp->mod might be NULL which will Oops module_is_coming()
Hmm... I guess this should be fixable by adding a '!kp->mod' to the
condition:
if (!kp->mod || !module_is_coming(kp->mod)) ...
> - it's still racy against fuse_dentry_tree_init
This one is a bit trickier indeed. I could try to play a bit with the
modules notifiers (register_module_notifier). But I haven't used that
before, so not sure it can be used, for example, kicking off the worker
later when the module has been initialised.
>
> My question is: does the inval_wq_set() even need to work before the
> module is initialized? Can't it just return an error and it wouldn't
> need to deal with the race.
That's probably the simplest option, but that would prevent us from, for
example, setting inval_wq as a kernel parameter, right? Or doing:
# modprobe fuse inval_wq=10
Do you think that's acceptable?
Cheers,
--
Luis
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-06 12:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 12:53 [PATCH] fuse: fix 'inval_wq' parameter setting during module initialisation Luis Henriques
2026-11-06 10:18 ` Luis Henriques
2026-11-06 11:33 ` Miklos Szeredi
2026-11-06 12:29 ` Luis Henriques
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®