On Fri, Dec 13, 2024 at 9:29 PM Joanne Koong <joannelkoong@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> fs/fuse/dev.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/fuse/fuse_i.h | 22 +++++++++++++
> fs/fuse/inode.c | 23 ++++++++++++++
> 3 files changed, 128 insertions(+)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 27ccae63495d..e97ba860ffcd 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -45,6 +45,85 @@ 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 fuse_req *req)
> +{
> + return time_is_before_jiffies(req->create_time + fc->timeout.req_timeout);
> +}
> +
> +/*
> + * Check if any requests aren't being completed by the time the request timeout
> + * elapses. To do so, we:
> + * - check the fiq pending list
> + * - check the bg queue
> + * - check the fpq io and processing lists
> + *
> + * To make this fast, we only check against the head request on each list since
> + * these are generally queued in order of creation time (eg newer requests get
> + * queued to the tail). We might miss a few edge cases (eg requests transitioning
> + * between lists, re-sent requests at the head of the pending list having a
> + * later creation time than other requests on that list, etc.) but that is fine
> + * since if the request never gets fulfilled, it will eventually be caught.
> + */
> +void fuse_check_timeout(struct work_struct *work)
> +{
> + struct delayed_work *dwork = to_delayed_work(work);
> + struct fuse_conn *fc = container_of(dwork, struct fuse_conn,
> + timeout.work);
> + struct fuse_iqueue *fiq = &fc->iq;
> + struct fuse_req *req;
> + struct fuse_dev *fud;
> + struct fuse_pqueue *fpq;
> + bool expired = false;
> + int i;
> +
> + spin_lock(&fiq->lock);
> + req = list_first_entry_or_null(&fiq->pending, struct fuse_req, list);
> + if (req)
> + expired = request_expired(fc, req);
> + spin_unlock(&fiq->lock);
> + if (expired)
> + goto abort_conn;
> +
> + spin_lock(&fc->bg_lock);
> + req = list_first_entry_or_null(&fc->bg_queue, struct fuse_req, list);
> + if (req)
> + expired = request_expired(fc, req);
> + spin_unlock(&fc->bg_lock);
> + if (expired)
> + goto abort_conn;
> +
> + spin_lock(&fc->lock);
> + if (!fc->connected) {
> + spin_unlock(&fc->lock);
> + return;
> + }
> + list_for_each_entry(fud, &fc->devices, entry) {
> + fpq = &fud->pq;
> + spin_lock(&fpq->lock);
Can fuse_dev_release() run concurrently to this path here?
If yes say fuse_dev_release() comes in first, grab the fpq->lock and
splice the
fpq->processing[i] list into &to_end and release the fpq->lock which
unblock this
path.
Then here we start checking req off the fpq->processing[i] list which is
getting evicted on the other side by fuse_dev_release->end_requests(&to_end);
Maybe we need a cancel_delayed_work_sync() at the beginning of
fuse_dev_release ?
Thanks
Etienne
> + req = list_first_entry_or_null(&fpq->io, struct fuse_req, list);
> + if (req && request_expired(fc, req))
> + goto fpq_abort;
> +
> + for (i = 0; i < FUSE_PQ_HASH_SIZE; i++) {
> + req = list_first_entry_or_null(&fpq->processing[i], struct fuse_req, list);
> + if (req && request_expired(fc, req))
> + goto fpq_abort;
> + }
> + spin_unlock(&fpq->lock);
> + }
> + spin_unlock(&fc->lock);
> +
> + queue_delayed_work(system_wq, &fc->timeout.work,
> + secs_to_jiffies(FUSE_TIMEOUT_TIMER_FREQ));
> + return;
> +
> +fpq_abort:
> + spin_unlock(&fpq->lock);
> + spin_unlock(&fc->lock);
> +abort_conn:
> + fuse_abort_conn(fc);
> +}
> +
