On Thu, Apr 17, 2025 at 12:02:55PM +0200, Mateusz Guzik wrote:
> bottom line though is there is a known tradeoff there and stress-ng
> manufactures a case where it is always the wrong one.
>
> fd 2 at hand is inherited (it's the tty) and shared between *all*
> workers on all CPUs.
>
> Ignoring some fluff, it's this in a loop:
> dup2(2, 1024) = 1024
> dup2(2, 1025) = 1025
> dup2(2, 1026) = 1026
> dup2(2, 1027) = 1027
> dup2(2, 1028) = 1028
> dup2(2, 1029) = 1029
> dup2(2, 1030) = 1030
> dup2(2, 1031) = 1031
> [..]
> close_range(1024, 1032, 0) = 0
>
> where fd 2 is the same file object in all 192 workers doing this.
>
the following will still have *some* impact, but the drop should be much
lower
it also has a side effect of further helping the single-threaded case by
shortening the code when it works
diff --git a/include/linux/file_ref.h b/include/linux/file_ref.h
index 7db62fbc0500..c73865ed4251 100644
--- a/include/linux/file_ref.h
+++ b/include/linux/file_ref.h
@@ -181,17 +181,15 @@ static __always_inline __must_check bool file_ref_put_close(file_ref_t *ref)
long old, new;
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)) {
+ new = FILE_REF_DEAD;
+ if (likely(atomic_long_try_cmpxchg(&ref->refcnt, &old, new)))
+ return true;
+ /*
+ * The ref has changed from under us, don't play any games.
+ */
+ }
+ return file_ref_put(ref);
}
/**
