[PATCH v2 0/2] fuse/passthrough: simplify daemon crash recovery

[PATCH v2 0/2] fuse/passthrough: simplify daemon crash recovery

[Chunsheng Luo]

Hello maintainers and community,

This patch series addresses crash recovery issues for FUSE daemons
with passthrough support, following up on the previous RFC
discussion [1].

Problem Summary
===============
To simplify FUSE daemon crash recovery and reduce performance
overhead, passthrough backing_id information is not persisted.
However, this design choice introduces two issues when the daemon
restarts:

1. Non-persistent backing_ids prevent proper resource cleanup during
   daemon restart, potentially causing resource leaks.

2. New backing_ids allocated for the same backing file trigger -EIO
   errors due to strict pointer validation in
   fuse_inode_uncached_io_start(), even though the backing inodes
   are identical.

Changes from RFC v1
===================
Based on Amir Goldstein's suggestion, this v2 series implements:

Patch 1/2: "fuse: introduce close-all ioctl for passthrough backing
           cleanup"
  - Replaces the magic -1 value with a dedicated
    FUSE_DEV_IOC_BACKING_CLOSE_ALL ioctl for comprehensive cleanup
    during daemon restart.

Patch 2/2: "fuse: relax backing inode validation for crash recovery"
  - Drops the FOPEN_PASSTHROUGH_INODE_CACHE flag approach.
  - Modifies fuse_inode_uncached_io_start() to compare backing
    inodes instead of fuse_backing pointers, allowing safe reuse
    of identical backing files after recovery.

Testing
=======
The patches have been tested with FUSE daemon crash and recovery
scenarios on kernel 6.19-rc4, successfully resolving the -EIO
errors and backing file resource leaks.

This approach maintains the simplicity of non-persistent backing_ids
while ensuring proper recovery semantics.

Thank you for your review and previous guidance.

[1] https://lore.kernel.org/linux-fsdevel/20260115072032.402-1-luochunsheng@ustc.edu/
---
Chunsheng Luo (2):
  fuse: add ioctl to cleanup all backing files
  fuse: Relax backing file validation to compare backing inodes

 fs/fuse/backing.c         | 19 +++++++++++++++++++
 fs/fuse/dev.c             | 16 ++++++++++++++++
 fs/fuse/fuse_i.h          |  1 +
 fs/fuse/iomode.c          |  2 +-
 include/uapi/linux/fuse.h |  1 +
 5 files changed, 38 insertions(+), 1 deletion(-)

-- 
2.43.0

[PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Chunsheng Luo]

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@ustc.edu>
---
 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;
+}
+
 struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc, int backing_id)
 {
 	struct fuse_backing *fb;
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 6d59cbc877c6..f05d55302598 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2654,6 +2654,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
 	return fuse_backing_close(fud->fc, backing_id);
 }
 
+static long fuse_dev_ioctl_backing_close_all(struct file *file)
+{
+	struct fuse_dev *fud = fuse_get_dev(file);
+
+	if (IS_ERR(fud))
+		return PTR_ERR(fud);
+
+	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
+		return -EOPNOTSUPP;
+
+	return fuse_backing_close_all(fud->fc);
+}
+
 static long fuse_dev_ioctl_sync_init(struct file *file)
 {
 	int err = -EINVAL;
@@ -2682,6 +2695,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 	case FUSE_DEV_IOC_BACKING_CLOSE:
 		return fuse_dev_ioctl_backing_close(file, argp);
 
+	case FUSE_DEV_IOC_BACKING_CLOSE_ALL:
+		return fuse_dev_ioctl_backing_close_all(file);
+
 	case FUSE_DEV_IOC_SYNC_INIT:
 		return fuse_dev_ioctl_sync_init(file);
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..33e91a5d3765 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -1573,6 +1573,7 @@ void fuse_backing_files_init(struct fuse_conn *fc);
 void fuse_backing_files_free(struct fuse_conn *fc);
 int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map);
 int fuse_backing_close(struct fuse_conn *fc, int backing_id);
+int fuse_backing_close_all(struct fuse_conn *fc);
 
 /* passthrough.c */
 static inline struct fuse_backing *fuse_inode_backing(struct fuse_inode *fi)
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index c13e1f9a2f12..e4ff28a4ff40 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -1139,6 +1139,7 @@ struct fuse_backing_map {
 					     struct fuse_backing_map)
 #define FUSE_DEV_IOC_BACKING_CLOSE	_IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
 #define FUSE_DEV_IOC_SYNC_INIT		_IO(FUSE_DEV_IOC_MAGIC, 3)
+#define FUSE_DEV_IOC_BACKING_CLOSE_ALL	_IO(FUSE_DEV_IOC_MAGIC, 4)
 
 struct fuse_lseek_in {
 	uint64_t	fh;
-- 
2.43.0

[PATCH 2/2] fuse: Relax backing file validation to compare backing inodes

[Chunsheng Luo]

To simplify crash recovery and reduce performance impact, backing_ids
are not persisted across daemon restarts. However, when the daemon
restarts and another process open the same FUSE file and assigning it
the same backing file (with the same inode) will also cause the
fuse_inode_uncached_io_start() function to fail due to a mismatch in
the fb pointer.

So Relax the validation in fuse_inode_uncached_io_start() to compare
backing inodes instead of fuse_backing pointers.

Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
---
 fs/fuse/iomode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
index 3728933188f3..ca7619958b0d 100644
--- a/fs/fuse/iomode.c
+++ b/fs/fuse/iomode.c
@@ -90,7 +90,7 @@ int fuse_inode_uncached_io_start(struct fuse_inode *fi, struct fuse_backing *fb)
 	spin_lock(&fi->lock);
 	/* deny conflicting backing files on same fuse inode */
 	oldfb = fuse_inode_backing(fi);
-	if (fb && oldfb && oldfb != fb) {
+	if (fb && oldfb && file_inode(oldfb->file) != file_inode(fb->file)) {
 		err = -EBUSY;
 		goto unlock;
 	}
-- 
2.43.0

Re: [PATCH 2/2] fuse: Relax backing file validation to compare backing inodes

[Amir Goldstein]

On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>
> To simplify crash recovery and reduce performance impact, backing_ids
> are not persisted across daemon restarts. However, when the daemon
> restarts and another process open the same FUSE file and assigning it
> the same backing file (with the same inode) will also cause the
> fuse_inode_uncached_io_start() function to fail due to a mismatch in
> the fb pointer.
>
> So Relax the validation in fuse_inode_uncached_io_start() to compare
> backing inodes instead of fuse_backing pointers.
>
> Signed-off-by: Chunsheng Luo <luochunsheng@ustc.edu>
Reviewed-by: Amir Goldstein <amir73il@gmail.com>

> ---
>  fs/fuse/iomode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/fuse/iomode.c b/fs/fuse/iomode.c
> index 3728933188f3..ca7619958b0d 100644
> --- a/fs/fuse/iomode.c
> +++ b/fs/fuse/iomode.c
> @@ -90,7 +90,7 @@ int fuse_inode_uncached_io_start(struct fuse_inode *fi, struct fuse_backing *fb)
>         spin_lock(&fi->lock);
>         /* deny conflicting backing files on same fuse inode */
>         oldfb = fuse_inode_backing(fi);
> -       if (fb && oldfb && oldfb != fb) {
> +       if (fb && oldfb && file_inode(oldfb->file) != file_inode(fb->file)) {
>                 err = -EBUSY;
>                 goto unlock;
>         }
> --
> 2.43.0
>

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Amir Goldstein]

On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
> ---
>  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(),
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.

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Chunsheng Luo]

On 1/16/26 11:39 PM, Amir Goldstein wrote:
> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
>> ---
>>   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.

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.

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.

- 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;
+}
+

--

Thanks,
Chunsheng Luo

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Amir Goldstein]

On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>
>
>
> On 1/16/26 11:39 PM, Amir Goldstein wrote:
> > On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
> >> ---
> >>   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.

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Chunsheng Luo]

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



On 1/18/26 1:00 AM, Amir Goldstein wrote:
> On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>
>>
>>
>> On 1/16/26 11:39 PM, Amir Goldstein wrote:
>>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
>>>> ---
>>>>    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.

Thanks,
Chunsheng Luo.

[-- Attachment #2: 0001-fuse-add-ioctl-to-cleanup-all-backing-files.patch --]
[-- Type: text/plain, Size: 7935 bytes --]

From b674dcef4e58318c92b061c62899d5598203547e Mon Sep 17 00:00:00 2001
From: Chunsheng Luo <luochunsheng@ustc.edu>
Date: Mon, 12 Jan 2026 16:56:36 +0800
Subject: [PATCH] fuse: add ioctl to cleanup all backing files

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@ustc.edu>
---
 fs/fuse/backing.c         | 77 +++++++++++++++++++++++++++++++++++----
 fs/fuse/dev.c             | 16 ++++++++
 fs/fuse/fuse_i.h          |  5 ++-
 fs/fuse/inode.c           | 11 +++---
 include/uapi/linux/fuse.h |  1 +
 5 files changed, 95 insertions(+), 15 deletions(-)

diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
index 4afda419dd14..2da024dc003d 100644
--- a/fs/fuse/backing.c
+++ b/fs/fuse/backing.c
@@ -32,19 +32,29 @@ void fuse_backing_put(struct fuse_backing *fb)
 		fuse_backing_free(fb);
 }
 
-void fuse_backing_files_init(struct fuse_conn *fc)
+int fuse_backing_files_init(struct fuse_conn *fc)
 {
-	idr_init(&fc->backing_files_map);
+	struct idr *idr;
+
+	idr = kzalloc(sizeof(*idr), GFP_KERNEL);
+	if (!idr)
+		return -ENOMEM;
+	idr_init(idr);
+	rcu_assign_pointer(fc->backing_files_map, idr);
+	return 0;
 }
 
 static int fuse_backing_id_alloc(struct fuse_conn *fc, struct fuse_backing *fb)
 {
+	struct idr *idr;
 	int id;
 
 	idr_preload(GFP_KERNEL);
 	spin_lock(&fc->lock);
+	idr = rcu_dereference_protected(fc->backing_files_map,
+					lockdep_is_held(&fc->lock));
 	/* FIXME: xarray might be space inefficient */
-	id = idr_alloc_cyclic(&fc->backing_files_map, fb, 1, 0, GFP_ATOMIC);
+	id = idr_alloc_cyclic(idr, fb, 1, 0, GFP_ATOMIC);
 	spin_unlock(&fc->lock);
 	idr_preload_end();
 
@@ -55,10 +65,13 @@ static int fuse_backing_id_alloc(struct fuse_conn *fc, struct fuse_backing *fb)
 static struct fuse_backing *fuse_backing_id_remove(struct fuse_conn *fc,
 						   int id)
 {
+	struct idr *idr;
 	struct fuse_backing *fb;
 
 	spin_lock(&fc->lock);
-	fb = idr_remove(&fc->backing_files_map, id);
+	idr = rcu_dereference_protected(fc->backing_files_map,
+					lockdep_is_held(&fc->lock));
+	fb = idr_remove(idr, id);
 	spin_unlock(&fc->lock);
 
 	return fb;
@@ -75,8 +88,13 @@ static int fuse_backing_id_free(int id, void *p, void *data)
 
 void fuse_backing_files_free(struct fuse_conn *fc)
 {
-	idr_for_each(&fc->backing_files_map, fuse_backing_id_free, NULL);
-	idr_destroy(&fc->backing_files_map);
+	struct idr *idr = rcu_dereference_protected(fc->backing_files_map, 1);
+
+	if (idr) {
+		idr_for_each(idr, fuse_backing_id_free, NULL);
+		idr_destroy(idr);
+		kfree(idr);
+	}
 }
 
 int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
@@ -166,12 +184,57 @@ int fuse_backing_close(struct fuse_conn *fc, int backing_id)
 	return err;
 }
 
+int fuse_backing_close_all(struct fuse_conn *fc)
+{
+	struct idr *old_idr, *new_idr;
+	struct fuse_backing *fb;
+	int id;
+
+	if (!fc->passthrough || !capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	new_idr = kzalloc(sizeof(*new_idr), GFP_KERNEL);
+	if (!new_idr)
+		return -ENOMEM;
+
+	idr_init(new_idr);
+
+	/*
+	 * Atomically exchange the old IDR with a new empty one under lock.
+	 * This avoids long lock hold times and races with concurrent
+	 * open/close operations.
+	 */
+	spin_lock(&fc->lock);
+	old_idr = rcu_dereference_protected(fc->backing_files_map,
+					    lockdep_is_held(&fc->lock));
+	rcu_assign_pointer(fc->backing_files_map, new_idr);
+	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();
+
+	if (old_idr) {
+		idr_for_each_entry(old_idr, fb, id)
+			fuse_backing_put(fb);
+
+		idr_destroy(old_idr);
+		kfree(old_idr);
+	}
+
+	return 0;
+}
+
 struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc, int backing_id)
 {
+	struct idr *idr;
 	struct fuse_backing *fb;
 
 	rcu_read_lock();
-	fb = idr_find(&fc->backing_files_map, backing_id);
+	idr = rcu_dereference(fc->backing_files_map);
+	fb = idr ? idr_find(idr, backing_id) : NULL;
 	fb = fuse_backing_get(fb);
 	rcu_read_unlock();
 
diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 6d59cbc877c6..f05d55302598 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -2654,6 +2654,19 @@ static long fuse_dev_ioctl_backing_close(struct file *file, __u32 __user *argp)
 	return fuse_backing_close(fud->fc, backing_id);
 }
 
+static long fuse_dev_ioctl_backing_close_all(struct file *file)
+{
+	struct fuse_dev *fud = fuse_get_dev(file);
+
+	if (IS_ERR(fud))
+		return PTR_ERR(fud);
+
+	if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
+		return -EOPNOTSUPP;
+
+	return fuse_backing_close_all(fud->fc);
+}
+
 static long fuse_dev_ioctl_sync_init(struct file *file)
 {
 	int err = -EINVAL;
@@ -2682,6 +2695,9 @@ static long fuse_dev_ioctl(struct file *file, unsigned int cmd,
 	case FUSE_DEV_IOC_BACKING_CLOSE:
 		return fuse_dev_ioctl_backing_close(file, argp);
 
+	case FUSE_DEV_IOC_BACKING_CLOSE_ALL:
+		return fuse_dev_ioctl_backing_close_all(file);
+
 	case FUSE_DEV_IOC_SYNC_INIT:
 		return fuse_dev_ioctl_sync_init(file);
 
diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
index 7f16049387d1..f45c5042e31a 100644
--- a/fs/fuse/fuse_i.h
+++ b/fs/fuse/fuse_i.h
@@ -979,7 +979,7 @@ struct fuse_conn {
 
 #ifdef CONFIG_FUSE_PASSTHROUGH
 	/** IDR for backing files ids */
-	struct idr backing_files_map;
+	struct idr __rcu *backing_files_map;
 #endif
 
 #ifdef CONFIG_FUSE_IO_URING
@@ -1569,10 +1569,11 @@ static inline struct fuse_backing *fuse_backing_lookup(struct fuse_conn *fc,
 }
 #endif
 
-void fuse_backing_files_init(struct fuse_conn *fc);
+int fuse_backing_files_init(struct fuse_conn *fc);
 void fuse_backing_files_free(struct fuse_conn *fc);
 int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map);
 int fuse_backing_close(struct fuse_conn *fc, int backing_id);
+int fuse_backing_close_all(struct fuse_conn *fc);
 
 /* passthrough.c */
 static inline struct fuse_backing *fuse_inode_backing(struct fuse_inode *fi)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 819e50d66622..b63a067d50f8 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1001,9 +1001,6 @@ void fuse_conn_init(struct fuse_conn *fc, struct fuse_mount *fm,
 	fc->name_max = FUSE_NAME_LOW_MAX;
 	fc->timeout.req_timeout = 0;
 
-	if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH))
-		fuse_backing_files_init(fc);
-
 	INIT_LIST_HEAD(&fc->mounts);
 	list_add(&fm->fc_entry, &fc->mounts);
 	fm->fc = fc;
@@ -1439,9 +1436,11 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 			    arg->max_stack_depth > 0 &&
 			    arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH &&
 			    !(flags & FUSE_WRITEBACK_CACHE))  {
-				fc->passthrough = 1;
-				fc->max_stack_depth = arg->max_stack_depth;
-				fm->sb->s_stack_depth = arg->max_stack_depth;
+				if (fuse_backing_files_init(fc) == 0) {
+					fc->passthrough = 1;
+					fc->max_stack_depth = arg->max_stack_depth;
+					fm->sb->s_stack_depth = arg->max_stack_depth;
+				}
 			}
 			if (flags & FUSE_NO_EXPORT_SUPPORT)
 				fm->sb->s_export_op = &fuse_export_fid_operations;
diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
index c13e1f9a2f12..e4ff28a4ff40 100644
--- a/include/uapi/linux/fuse.h
+++ b/include/uapi/linux/fuse.h
@@ -1139,6 +1139,7 @@ struct fuse_backing_map {
 					     struct fuse_backing_map)
 #define FUSE_DEV_IOC_BACKING_CLOSE	_IOW(FUSE_DEV_IOC_MAGIC, 2, uint32_t)
 #define FUSE_DEV_IOC_SYNC_INIT		_IO(FUSE_DEV_IOC_MAGIC, 3)
+#define FUSE_DEV_IOC_BACKING_CLOSE_ALL	_IO(FUSE_DEV_IOC_MAGIC, 4)
 
 struct fuse_lseek_in {
 	uint64_t	fh;
-- 
2.41.0

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Amir Goldstein]

On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>
>
>
> On 1/18/26 1:00 AM, Amir Goldstein wrote:
> > On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> >>
> >>
> >>
> >> On 1/16/26 11:39 PM, Amir Goldstein wrote:
> >>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
> >>>> ---
> >>>>    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.

Thanks,
Amir.

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Amir Goldstein]

On Sun, Jan 18, 2026 at 6:07 PM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> >
> >
> >
> > On 1/18/26 1:00 AM, Amir Goldstein wrote:
> > > On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
> > >>
> > >>
> > >>
> > >> On 1/16/26 11:39 PM, Amir Goldstein wrote:
> > >>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
> > >>>> ---
> > >>>>    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@gmail.com>

Thanks,
Amir.

Re: [PATCH 1/2] fuse: add ioctl to cleanup all backing files

[Chunsheng Luo]

On 1/19/26 1:08 AM, Amir Goldstein wrote:
> On Sun, Jan 18, 2026 at 6:07 PM Amir Goldstein <amir73il@gmail.com> wrote:
>>
>> On Sun, Jan 18, 2026 at 12:47 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>>
>>>
>>>
>>> On 1/18/26 1:00 AM, Amir Goldstein wrote:
>>>> On Sat, Jan 17, 2026 at 5:14 PM Chunsheng Luo <luochunsheng@ustc.edu> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 1/16/26 11:39 PM, Amir Goldstein wrote:
>>>>>> On Fri, Jan 16, 2026 at 3:28 PM Chunsheng Luo <luochunsheng@ustc.edu> 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@ustc.edu>
>>>>>>> ---
>>>>>>>     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@gmail.com>
> 
> Thanks,
> Amir.
> 
> 

Ok, agree. Using rcu_replace_pointer is more concise.

Thanks,
Chunsheng Luo