Re: [PATCH 2/7] fuse: flush pending fuse events before aborting the connection

[Bernd Schubert <bernd@bsbernd.com>]

[-- Attachment #1: Type: text/plain, Size: 4444 bytes --]



On 7/18/25 01:26, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> generic/488 fails with fuse2fs in the following fashion:
> 
> generic/488       _check_generic_filesystem: filesystem on /dev/sdf is inconsistent
> (see /var/tmp/fstests/generic/488.full for details)
> 
> This test opens a large number of files, unlinks them (which really just
> renames them to fuse hidden files), closes the program, unmounts the
> filesystem, and runs fsck to check that there aren't any inconsistencies
> in the filesystem.
> 
> Unfortunately, the 488.full file shows that there are a lot of hidden
> files left over in the filesystem, with incorrect link counts.  Tracing
> fuse_request_* shows that there are a large number of FUSE_RELEASE
> commands that are queued up on behalf of the unlinked files at the time
> that fuse_conn_destroy calls fuse_abort_conn.  Had the connection not
> aborted, the fuse server would have responded to the RELEASE commands by
> removing the hidden files; instead they stick around.
> 
> Create a function to push all the background requests to the queue and
> then wait for the number of pending events to hit zero, and call this
> before fuse_abort_conn.  That way, all the pending events are processed
> by the fuse server and we don't end up with a corrupt filesystem.
> 
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  fs/fuse/fuse_i.h |    6 ++++++
>  fs/fuse/dev.c    |   38 ++++++++++++++++++++++++++++++++++++++
>  fs/fuse/inode.c  |    1 +
>  3 files changed, 45 insertions(+)
> 
> 
> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index b54f4f57789f7f..78d34c8e445b32 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
> @@ -1256,6 +1256,12 @@ void fuse_request_end(struct fuse_req *req);
>  void fuse_abort_conn(struct fuse_conn *fc);
>  void fuse_wait_aborted(struct fuse_conn *fc);
>  
> +/**
> + * Flush all pending requests and wait for them.  Takes an optional timeout
> + * in jiffies.
> + */
> +void fuse_flush_requests(struct fuse_conn *fc, unsigned long timeout);
> +
>  /* Check if any requests timed out */
>  void fuse_check_timeout(struct work_struct *work);
>  
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index e80cd8f2c049f9..5387e4239d6aa6 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -24,6 +24,7 @@
>  #include <linux/splice.h>
>  #include <linux/sched.h>
>  #include <linux/seq_file.h>
> +#include <linux/nmi.h>
>  
>  #define CREATE_TRACE_POINTS
>  #include "fuse_trace.h"
> @@ -2385,6 +2386,43 @@ static void end_polls(struct fuse_conn *fc)
>  	}
>  }
>  
> +/*
> + * Flush all pending requests and wait for them.  Only call this function when
> + * it is no longer possible for other threads to add requests.
> + */
> +void fuse_flush_requests(struct fuse_conn *fc, unsigned long timeout)

I wonder if this should have "abort" in its name. Because it is not a
simple flush attempt, but also sets fc->blocked and fc->max_background.

> +{
> +	unsigned long deadline;
> +
> +	spin_lock(&fc->lock);
> +	if (!fc->connected) {
> +		spin_unlock(&fc->lock);
> +		return;
> +	}
> +
> +	/* Push all the background requests to the queue. */
> +	spin_lock(&fc->bg_lock);
> +	fc->blocked = 0;
> +	fc->max_background = UINT_MAX;
> +	flush_bg_queue(fc);
> +	spin_unlock(&fc->bg_lock);
> +	spin_unlock(&fc->lock);
> +
> +	/*
> +	 * Wait 30s for all the events to complete or abort.  Touch the
> +	 * watchdog once per second so that we don't trip the hangcheck timer
> +	 * while waiting for the fuse server.
> +	 */
> +	deadline = jiffies + timeout;
> +	smp_mb();
> +	while (fc->connected &&
> +	       (!timeout || time_before(jiffies, deadline)) &&
> +	       wait_event_timeout(fc->blocked_waitq,
> +			!fc->connected || atomic_read(&fc->num_waiting) == 0,
> +			HZ) == 0)
> +		touch_softlockup_watchdog();
> +}
> +
>  /*
>   * Abort all requests.
>   *
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 9572bdef49eecc..1734c263da3a77 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -2047,6 +2047,7 @@ void fuse_conn_destroy(struct fuse_mount *fm)
>  {
>  	struct fuse_conn *fc = fm->fc;
>  
> +	fuse_flush_requests(fc, 30 * HZ);

I think fc->connected should be set to 0, to avoid that new requests can
be allocated.

>  	if (fc->destroy)
>  		fuse_send_destroy(fm);
>  
> 


Please see the two attached patches, which are needed for fuse-io-uring.
I can also send them separately, if you prefer.


Thanks,
Bernd

[-- Attachment #2: 01-flush-io-uring-queue --]
[-- Type: text/plain, Size: 3552 bytes --]

fuse: Refactor io-uring bg queue flush and queue abort

From: Bernd Schubert <bschubert@ddn.com>

This is a preparation to allow fuse-io-uring bg queue
flush from flush_bg_queue()

This does two function renames:
fuse_uring_flush_bg -> fuse_uring_flush_queue_bg
fuse_uring_abort_end_requests -> fuse_uring_flush_bg

And fuse_uring_abort_end_queue_requests() is moved to
fuse_uring_stop_queues().

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dev_uring.c   |   14 +++++++-------
 fs/fuse/dev_uring_i.h |    4 ++--
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
index 249b210becb1..eca457d1005e 100644
--- a/fs/fuse/dev_uring.c
+++ b/fs/fuse/dev_uring.c
@@ -47,7 +47,7 @@ static struct fuse_ring_ent *uring_cmd_to_ring_ent(struct io_uring_cmd *cmd)
 	return pdu->ent;
 }
 
-static void fuse_uring_flush_bg(struct fuse_ring_queue *queue)
+static void fuse_uring_flush_queue_bg(struct fuse_ring_queue *queue)
 {
 	struct fuse_ring *ring = queue->ring;
 	struct fuse_conn *fc = ring->fc;
@@ -88,7 +88,7 @@ static void fuse_uring_req_end(struct fuse_ring_ent *ent, struct fuse_req *req,
 	if (test_bit(FR_BACKGROUND, &req->flags)) {
 		queue->active_background--;
 		spin_lock(&fc->bg_lock);
-		fuse_uring_flush_bg(queue);
+		fuse_uring_flush_queue_bg(queue);
 		spin_unlock(&fc->bg_lock);
 	}
 
@@ -117,11 +117,11 @@ static void fuse_uring_abort_end_queue_requests(struct fuse_ring_queue *queue)
 	fuse_dev_end_requests(&req_list);
 }
 
-void fuse_uring_abort_end_requests(struct fuse_ring *ring)
+void fuse_uring_flush_bg(struct fuse_conn *fc)
 {
 	int qid;
 	struct fuse_ring_queue *queue;
-	struct fuse_conn *fc = ring->fc;
+	struct fuse_ring *ring = fc->ring;
 
 	for (qid = 0; qid < ring->nr_queues; qid++) {
 		queue = READ_ONCE(ring->queues[qid]);
@@ -133,10 +133,9 @@ void fuse_uring_abort_end_requests(struct fuse_ring *ring)
 		WARN_ON_ONCE(ring->fc->max_background != UINT_MAX);
 		spin_lock(&queue->lock);
 		spin_lock(&fc->bg_lock);
-		fuse_uring_flush_bg(queue);
+		fuse_uring_flush_queue_bg(queue);
 		spin_unlock(&fc->bg_lock);
 		spin_unlock(&queue->lock);
-		fuse_uring_abort_end_queue_requests(queue);
 	}
 }
 
@@ -475,6 +474,7 @@ void fuse_uring_stop_queues(struct fuse_ring *ring)
 		if (!queue)
 			continue;
 
+		fuse_uring_abort_end_queue_requests(queue);
 		fuse_uring_teardown_entries(queue);
 	}
 
@@ -1326,7 +1326,7 @@ bool fuse_uring_queue_bq_req(struct fuse_req *req)
 	fc->num_background++;
 	if (fc->num_background == fc->max_background)
 		fc->blocked = 1;
-	fuse_uring_flush_bg(queue);
+	fuse_uring_flush_queue_bg(queue);
 	spin_unlock(&fc->bg_lock);
 
 	/*
diff --git a/fs/fuse/dev_uring_i.h b/fs/fuse/dev_uring_i.h
index 51a563922ce1..55f52508de3c 100644
--- a/fs/fuse/dev_uring_i.h
+++ b/fs/fuse/dev_uring_i.h
@@ -138,7 +138,7 @@ struct fuse_ring {
 bool fuse_uring_enabled(void);
 void fuse_uring_destruct(struct fuse_conn *fc);
 void fuse_uring_stop_queues(struct fuse_ring *ring);
-void fuse_uring_abort_end_requests(struct fuse_ring *ring);
+void fuse_uring_flush_bg(struct fuse_conn *fc);
 int fuse_uring_cmd(struct io_uring_cmd *cmd, unsigned int issue_flags);
 void fuse_uring_queue_fuse_req(struct fuse_iqueue *fiq, struct fuse_req *req);
 bool fuse_uring_queue_bq_req(struct fuse_req *req);
@@ -153,7 +153,7 @@ static inline void fuse_uring_abort(struct fuse_conn *fc)
 		return;
 
 	if (atomic_read(&ring->queue_refs) > 0) {
-		fuse_uring_abort_end_requests(ring);
+		fuse_uring_flush_bg(fc);
 		fuse_uring_stop_queues(ring);
 	}
 }

[-- Attachment #3: 02-flush-uring-bg --]
[-- Type: text/plain, Size: 1533 bytes --]

fuse: Flush the io-uring bg queue from fuse_uring_flush_bg

From: Bernd Schubert <bschubert@ddn.com>

This is useful to have a unique API to flush background requests.
For example when the bg queue gets flushed before
the remaining of fuse_conn_destroy().

Signed-off-by: Bernd Schubert <bschubert@ddn.com>
---
 fs/fuse/dev.c         |    2 ++
 fs/fuse/dev_uring_i.h |   10 +++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 5387e4239d6a..3f5f168cc28a 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -426,6 +426,8 @@ static void flush_bg_queue(struct fuse_conn *fc)
 		fc->active_background++;
 		fuse_send_one(fiq, req);
 	}
+
+	fuse_uring_flush_bg(fc);
 }
 
 /*
diff --git a/fs/fuse/dev_uring_i.h b/fs/fuse/dev_uring_i.h
index 55f52508de3c..fca2184e8d94 100644
--- a/fs/fuse/dev_uring_i.h
+++ b/fs/fuse/dev_uring_i.h
@@ -152,10 +152,10 @@ static inline void fuse_uring_abort(struct fuse_conn *fc)
 	if (ring == NULL)
 		return;
 
-	if (atomic_read(&ring->queue_refs) > 0) {
-		fuse_uring_flush_bg(fc);
+	/* Assumes bg queues were already flushed before */
+
+	if (atomic_read(&ring->queue_refs) > 0)
 		fuse_uring_stop_queues(ring);
-	}
 }
 
 static inline void fuse_uring_wait_stopped_queues(struct fuse_conn *fc)
@@ -206,6 +206,10 @@ static inline bool fuse_uring_request_expired(struct fuse_conn *fc)
 	return false;
 }
 
+static inline void fuse_uring_flush_bg(struct fuse_conn *fc)
+{
+}
+
 #endif /* CONFIG_FUSE_IO_URING */
 
 #endif /* _FS_FUSE_DEV_URING_I_H */