[PATCH v2] fs: fall back to file_ref_put() for non-last reference

[PATCH v2] fs: fall back to file_ref_put() for non-last reference

[Mateusz Guzik]

This reduces the slowdown in face of multiple callers issuing close on
what turns out to not be the last reference.

Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202504171513.6d6f8a16-lkp@intel.com
---

v2:
- fix a copy pasto with bad conditional

 fs/file.c                |  2 +-
 include/linux/file_ref.h | 19 ++++++-------------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/fs/file.c b/fs/file.c
index dc3f7e120e3e..3a3146664cf3 100644
--- a/fs/file.c
+++ b/fs/file.c
@@ -26,7 +26,7 @@
 
 #include "internal.h"
 
-bool __file_ref_put_badval(file_ref_t *ref, unsigned long cnt)
+static noinline bool __file_ref_put_badval(file_ref_t *ref, unsigned long cnt)
 {
 	/*
 	 * If the reference count was already in the dead zone, then this
diff --git a/include/linux/file_ref.h b/include/linux/file_ref.h
index 7db62fbc0500..31551e4cb8f3 100644
--- a/include/linux/file_ref.h
+++ b/include/linux/file_ref.h
@@ -61,7 +61,6 @@ static inline void file_ref_init(file_ref_t *ref, unsigned long cnt)
 	atomic_long_set(&ref->refcnt, cnt - 1);
 }
 
-bool __file_ref_put_badval(file_ref_t *ref, unsigned long cnt);
 bool __file_ref_put(file_ref_t *ref, unsigned long cnt);
 
 /**
@@ -178,20 +177,14 @@ static __always_inline __must_check bool file_ref_put(file_ref_t *ref)
  */
 static __always_inline __must_check bool file_ref_put_close(file_ref_t *ref)
 {
-	long old, new;
+	long old;
 
 	old = atomic_long_read(&ref->refcnt);
-	do {
-		if (unlikely(old < 0))
-			return __file_ref_put_badval(ref, old);
-
-		if (old == FILE_REF_ONEREF)
-			new = FILE_REF_DEAD;
-		else
-			new = old - 1;
-	} while (!atomic_long_try_cmpxchg(&ref->refcnt, &old, new));
-
-	return new == FILE_REF_DEAD;
+	if (likely(old == FILE_REF_ONEREF)) {
+		if (likely(atomic_long_try_cmpxchg(&ref->refcnt, &old, FILE_REF_DEAD)))
+			return true;
+	}
+	return file_ref_put(ref);
 }
 
 /**
-- 
2.48.1

Re: [PATCH v2] fs: fall back to file_ref_put() for non-last reference

[Christian Brauner]

On Fri, 18 Apr 2025 14:57:56 +0200, Mateusz Guzik wrote:
> This reduces the slowdown in face of multiple callers issuing close on
> what turns out to not be the last reference.
> 
> 

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] fs: fall back to file_ref_put() for non-last reference
      https://git.kernel.org/vfs/vfs/c/ca38ac96d96b

Re: [PATCH v2] fs: fall back to file_ref_put() for non-last reference

[Jan Kara]

On Fri 18-04-25 14:57:56, Mateusz Guzik wrote:
> This reduces the slowdown in face of multiple callers issuing close on
> what turns out to not be the last reference.
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202504171513.6d6f8a16-lkp@intel.com

Yeah, this looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
> 
> v2:
> - fix a copy pasto with bad conditional
> 
>  fs/file.c                |  2 +-
>  include/linux/file_ref.h | 19 ++++++-------------
>  2 files changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/file.c b/fs/file.c
> index dc3f7e120e3e..3a3146664cf3 100644
> --- a/fs/file.c
> +++ b/fs/file.c
> @@ -26,7 +26,7 @@
>  
>  #include "internal.h"
>  
> -bool __file_ref_put_badval(file_ref_t *ref, unsigned long cnt)
> +static noinline bool __file_ref_put_badval(file_ref_t *ref, unsigned long cnt)
>  {
>  	/*
>  	 * If the reference count was already in the dead zone, then this
> diff --git a/include/linux/file_ref.h b/include/linux/file_ref.h
> index 7db62fbc0500..31551e4cb8f3 100644
> --- a/include/linux/file_ref.h
> +++ b/include/linux/file_ref.h
> @@ -61,7 +61,6 @@ static inline void file_ref_init(file_ref_t *ref, unsigned long cnt)
>  	atomic_long_set(&ref->refcnt, cnt - 1);
>  }
>  
> -bool __file_ref_put_badval(file_ref_t *ref, unsigned long cnt);
>  bool __file_ref_put(file_ref_t *ref, unsigned long cnt);
>  
>  /**
> @@ -178,20 +177,14 @@ static __always_inline __must_check bool file_ref_put(file_ref_t *ref)
>   */
>  static __always_inline __must_check bool file_ref_put_close(file_ref_t *ref)
>  {
> -	long old, new;
> +	long old;
>  
>  	old = atomic_long_read(&ref->refcnt);
> -	do {
> -		if (unlikely(old < 0))
> -			return __file_ref_put_badval(ref, old);
> -
> -		if (old == FILE_REF_ONEREF)
> -			new = FILE_REF_DEAD;
> -		else
> -			new = old - 1;
> -	} while (!atomic_long_try_cmpxchg(&ref->refcnt, &old, new));
> -
> -	return new == FILE_REF_DEAD;
> +	if (likely(old == FILE_REF_ONEREF)) {
> +		if (likely(atomic_long_try_cmpxchg(&ref->refcnt, &old, FILE_REF_DEAD)))
> +			return true;
> +	}
> +	return file_ref_put(ref);
>  }
>  
>  /**
> -- 
> 2.48.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR