Re: [PATCH v2 0/2] fuse: add timeout option for requests

[Yafang Shao <laoar.shao@gmail.com>]

On Wed, Jul 31, 2024 at 2:16 AM Joanne Koong <joannelkoong@gmail.com> wrote:
>
> On Mon, Jul 29, 2024 at 11:00 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> >
> > On Tue, Jul 30, 2024 at 8:28 AM Joanne Koong <joannelkoong@gmail.com> wrote:
> > >
> > > There are situations where fuse servers can become unresponsive or take
> > > too long to reply to a request. Currently there is no upper bound on
> > > how long a request may take, which may be frustrating to users who get
> > > stuck waiting for a request to complete.
> > >
> > > This patchset adds a timeout option for requests and two dynamically
> > > configurable fuse sysctls "default_request_timeout" and "max_request_timeout"
> > > for controlling/enforcing timeout behavior system-wide.
> > >
> > > Existing fuse servers will not be affected unless they explicitly opt into the
> > > timeout.
> > >
> > > v1: https://lore.kernel.org/linux-fsdevel/20240717213458.1613347-1-joannelkoong@gmail.com/
> > > Changes from v1:
> > > - Add timeout for background requests
> > > - Handle resend race condition
> > > - Add sysctls
> > >
> > > Joanne Koong (2):
> > >   fuse: add optional kernel-enforced timeout for requests
> > >   fuse: add default_request_timeout and max_request_timeout sysctls
> > >
> > >  Documentation/admin-guide/sysctl/fs.rst |  17 +++
> > >  fs/fuse/Makefile                        |   2 +-
> > >  fs/fuse/dev.c                           | 187 +++++++++++++++++++++++-
> > >  fs/fuse/fuse_i.h                        |  30 ++++
> > >  fs/fuse/inode.c                         |  24 +++
> > >  fs/fuse/sysctl.c                        |  42 ++++++
> > >  6 files changed, 293 insertions(+), 9 deletions(-)
> > >  create mode 100644 fs/fuse/sysctl.c
> > >
> > > --
> > > 2.43.0
> > >
> >
> > Hello Joanne,
> >
> > Thanks for your update.
> >
> > I have tested your patches using my test case, which is similar to the
> > hello-fuse [0] example, with an additional change as follows:
> >
> > @@ -125,6 +125,8 @@ static int hello_read(const char *path, char *buf,
> > size_t size, off_t offset,
> >         } else
> >                 size = 0;
> >
> > +       // TO trigger timeout
> > +       sleep(60);
> >         return size;
> >  }
> >
> > [0] https://github.com/libfuse/libfuse/blob/master/example/hello.c
> >
> > However, it triggered a crash with the following setup:
> >
> > 1. Set FUSE timeout:
> >   sysctl -w fs.fuse.default_request_timeout=10
> >   sysctl -w fs.fuse.max_request_timeout = 20
> >
> > 2. Start FUSE daemon:
> >   ./hello /tmp/fuse
> >
> > 3. Read from FUSE:
> >   cat /tmp/fuse/hello
> >
> > 4. Kill the process within 10 seconds (to avoid the timeout being triggered).
> >    Then the crash will be triggered.
>
> Hi Yafang,
>
> Thanks for trying this out on your use case!
>
> How consistently are you able to repro this?

It triggers the crash every time.

> I tried reproing using
> your instructions above but I'm not able to get the crash.

Please note that it is the `cat /tmp/fuse/hello` process that was
killed, not the fuse daemon.
The crash seems to occur when the fuse daemon wakes up after
sleep(60). Please ensure that the fuse daemon can be woken up.

>
> From the crash logs you provided below, it looks like what's happening
> is that if the process gets killed, the timer isn't getting deleted.
> I'll look more into what happens in fuse when a process is killed and
> get back to you on this.

Thanks

--
Regards
Yafang