Re: [PATCH v11 1/2] fuse: add kernel-enforced timeout option for requests

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2024-12-19 at 07:19 -0500, Jeff Layton wrote:
> On Wed, 2024-12-18 at 14:26 -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 seconds) 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. For example, the
> > request may take roughly an extra FUSE_TIMEOUT_TIMER_FREQ seconds beyond
> > the requested timeout due to internal implementation, in order to
> > mitigate overhead.
> > 
> > Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> > ---
> >  fs/fuse/dev.c    | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  fs/fuse/fuse_i.h | 22 +++++++++++++
> >  fs/fuse/inode.c  | 23 +++++++++++++
> >  3 files changed, 130 insertions(+)
> > 
> > diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> > index 27ccae63495d..bcf8a7994944 100644
> > --- a/fs/fuse/dev.c
> > +++ b/fs/fuse/dev.c
> > @@ -45,6 +45,87 @@ static struct fuse_dev *fuse_get_dev(struct file *file)
> >  	return READ_ONCE(file->private_data);
> >  }
> >  
> > +static bool request_expired(struct fuse_conn *fc, struct list_head *list)
> > +{
> > +	struct fuse_req *req;
> > +
> > +	req = list_first_entry_or_null(list, struct fuse_req, list);
> > +	if (!req)
> > +		return false;
> > +	return time_is_before_jiffies(req->create_time + fc->timeout.req_timeout);
> > +}
> 
> Shouldn't that be time_is_after_jiffies() ? This is going to return
> true when you've not yet hit the timeout, no?
> 

My mistake, I misinterpreted the time_is_before_jiffies() macro. You
can add:

Reviewed-by: Jeff Layton <jlayton@kernel.org>