Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files
From: Chunsheng Luo
Date: Sun Jan 18 2026 - 21:44:46 EST
On 1/19/26 1:08 AM, Amir Goldstein wrote:
On Sun, Jan 18, 2026 at 6:07 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@xxxxxxxx> wrote:
On 1/18/26 1:00 AM, Amir Goldstein wrote:
On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@xxxxxxxx> wrote:
On 1/16/26 11:39 PM, Amir Goldstein wrote:
On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@xxxxxxxx> wrote:
To simplify crash recovery and reduce performance impact, backing_ids
are not persisted across daemon restarts. After crash recovery, this
may lead to resource leaks if backing file resources are not properly
cleaned up.
Add FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl to release all backing_ids
and put backing files. When the FUSE daemon restarts, it can use this
ioctl to cleanup all backing file resources.
Signed-off-by: Chunsheng Luo <luochunsheng@xxxxxxxx>
---
fs/fuse/backing.c | 19 +++++++++++++++++++
fs/fuse/dev.c | 16 ++++++++++++++++
fs/fuse/fuse_i.h | 1 +
include/uapi/linux/fuse.h | 1 +
4 files changed, 37 insertions(+)
diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 4afda419dd14..e93d797a2cde 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -166,6 +166,25 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
return err;
}
+static int fuse_backing_close_one(int id, void *p, void *data)
+{
+ struct fuse_conn *fc = data;
+
+ fuse_backing_close(fc, id);
+
+ return 0;
+}
+
+int fuse_backing_close_all(struct fuse_conn *fc)
+{
+ if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ idr_for_each(&fc->backing_files_map, fuse_backing_close_one, fc);
+
+ return 0;
+}
+
This is not safe and not efficient.
For safety from racing with _open/_close, iteration needs at least
rcu_read_lock(),
Yes, you're absolutely right. Additionally, calling idr_remove within
idr_for_each maybe presents safety risks.
but I think it will be much more efficient to zap the entire map with
fuse_backing_files_free()/fuse_backing_files_init().
This of course needs to be synchronized with concurrent _open/_close/_lookup.
This could be done by making c->backing_files_map a struct idr __rcu *
and replace the old and new backing_files_map under spin_lock(&fc->lock);
Then you can call fuse_backing_files_free() on the old backing_files_map
without a lock.
As a side note, fuse_backing_files_free() iteration looks like it may need
cond_resched() if there are a LOT of backing ids, but I am not sure and
this is orthogonal to your change.
Thanks,
Amir.
Thank you for your helpful suggestions. However, it cannot use
fuse_backing_files_free() in the close_all implementation because it
directly frees backing files without respecting reference counts. This
function requires that no one is actively using the backing file (it
even has WARN_ON_ONCE(refcount_read(&fb->count) != 1)), which cannot be
guaranteed after a crash recovery scenario where backing files may still
be in use.
Right.
Instead, the implementation uses fuse_backing_put() to safely decrement
the reference count and allow the backing file to be freed when no
longer in use.
OK.
Additionally, the implementation addresses two race conditions:
- Race between idr_for_each and lookup: Uses synchronize_rcu() to ensure
all concurrent RCU readers (i.e., in-flight fuse_backing_lookup() calls)
complete before releasing backing files, preventing use-after-free issues.
Almost. See below.
- Race with open/close operations: Uses fc->lock to atomically swap the
old and new IDR maps, ensuring consistency with concurrent
fuse_backing_open() and fuse_backing_close() operations.
This approach provides the same as the RCU pointer suggestion, but with
less code and no changes to the struct fuse_conn data structures.
I've updated it and verified the implementation. Could you please review it?
diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 4afda419dd14..047d373684f9 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -166,6 +166,45 @@ int fuse_backing_close(struct fuse_conn *fc, int
backing_id)
return err;
}
+static int fuse_backing_release_one(int id, void *p, void *data)
+{
+ struct fuse_backing *fb = p;
+
+ fuse_backing_put(fb);
+
+ return 0;
+}
+
+int fuse_backing_close_all(struct fuse_conn *fc)
+{
+ struct idr old_map;
+
+ if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ /*
+ * Swap out the old backing_files_map with a new empty one under
lock,
+ * then release all backing files outside the lock. This avoids long
+ * lock hold times and potential races with concurrent open/close
+ * operations.
+ */
+ idr_init(&old_map);
+ spin_lock(&fc->lock);
+ swap(fc->backing_files_map, old_map);
+ spin_unlock(&fc->lock);
+
+ /*
+ * Ensure all concurrent RCU readers complete before releasing
backing
+ * files, so any in-flight lookups can safely take references.
+ */
+ synchronize_rcu();
+
+ idr_for_each(&old_map, fuse_backing_release_one, NULL);
+ idr_destroy(&old_map);
+
+ return 0;
+}
+
That's almost safe but not enough.
This lookup code is not safe against the swap():
rcu_read_lock();
fb = idr_find(&fc->backing_files_map, backing_id);
That is the reason you need to make fc->backing_files_map
an rcu referenced ptr.
Instead of swap() you use xchg() to atomically exchange the
old and new struct idr pointers and for lookup:
rcu_read_lock();
fb = idr_find(rcu_dereference(fc->backing_files_map), backing_id);
Thanks,
Amir.
Yes, swap() isn't atomic, it's just copying structs, so it's not safe
when racing with lookup.
I've updated the version to make fc->backing_files_map an rcu referenced
ptr. Please review the attached patch.
You can also use rcu_replace_pointer() to swap old_idr <-> new_idr,
but otherwise the patch looks fine to me.
Feel free to add
Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
Thanks,
Amir.
Ok, agree. Using rcu_replace_pointer is more concise.
Thanks,
Chunsheng Luo