On Fri, Dec 6, 2024 at 12:12 PM Jeff Layton <jlayton@xxxxxxxxxx> wrote:
>
> On Thu, 2024-11-14 at 11:13 -0800, Joanne Koong wrote:
> > There are situations where fuse servers can become unresponsive or
> > stuck, for example if the server is deadlocked. Currently, there's no
> > good way to detect if a server is stuck and needs to be killed manually.
> >
> > This commit adds an option for enforcing a timeout (in minutes) for
> > requests where if the timeout elapses without the server responding to
> > the request, the connection will be automatically aborted.
> >
> > Please note that these timeouts are not 100% precise. The request may
> > take an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond the requested max
> > timeout due to how it's internally implemented.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> > Reviewed-by: Bernd Schubert <bschubert@xxxxxxx>
> > ---
> > fs/fuse/dev.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++
> > fs/fuse/fuse_i.h | 21 +++++++++++++
> > fs/fuse/inode.c | 21 +++++++++++++
> > 3 files changed, 122 insertions(+)
> >
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 29fc61a072ba..536aa4525e8f 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > return -EINVAL;
> > }
> > @@ -973,6 +979,8 @@ void fuse_conn_put(struct fuse_conn *fc)
> >
> > if (IS_ENABLED(CONFIG_FUSE_DAX))
> > fuse_dax_conn_free(fc);
> > + if (fc->timeout.req_timeout)
> > + timer_shutdown_sync(&fc->timeout.timer);
> > if (fiq->ops->release)
> > fiq->ops->release(fiq);
> > put_pid_ns(fc->pid_ns);
> > @@ -1691,6 +1699,18 @@ int fuse_init_fs_context_submount(struct fs_context *fsc)
> > }
> > EXPORT_SYMBOL_GPL(fuse_init_fs_context_submount);
> >
> > +static void fuse_init_fc_timeout(struct fuse_conn *fc, struct fuse_fs_context *ctx)
> > +{
> > + if (ctx->req_timeout) {
> > + if (check_mul_overflow(ctx->req_timeout * 60, HZ, &fc->timeout.req_timeout))
> > + fc->timeout.req_timeout = ULONG_MAX;
> > + timer_setup(&fc->timeout.timer, fuse_check_timeout, 0);
> > + mod_timer(&fc->timeout.timer, jiffies + FUSE_TIMEOUT_TIMER_FREQ);
> > + } else {
> > + fc->timeout.req_timeout = 0;
> > + }
> > +}
> > +
>
>
> Does fuse_check_timeout need to run in IRQ context? It doesn't seem
> like it does. Have you considered setting up a recurring delayed
> workqueue job instead? That would run in process context, which might
> make the locking in that function less hairy.
>
Great idea, I'll use a recurring delayed workqueue job instead of a
kthread, since it's more lightweight.
Thanks,
Joanne
>
> > int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> > {
> > struct fuse_dev *fud = NULL;
> > @@ -1753,6 +1773,7 @@ int fuse_fill_super_common(struct super_block *sb, struct fuse_fs_context *ctx)
> > fc->destroy = ctx->destroy;
> > fc->no_control = ctx->no_control;
> > fc->no_force_umount = ctx->no_force_umount;
> > + fuse_init_fc_timeout(fc, ctx);
> >
> > err = -ENOMEM;
> > root = fuse_get_root_inode(sb, ctx->rootmode);
>
> --
> Jeff Layton <jlayton@xxxxxxxxxx>
