public inbox for [email protected]
 help / color / mirror / Atom feed
From: Mateusz Guzik <[email protected]>
To: [email protected], [email protected]
Cc: [email protected], [email protected],
	[email protected], [email protected],
	[email protected], [email protected],
	Mateusz Guzik <[email protected]>
Subject: [PATCH] fs: dodge an atomic in putname if ref == 1
Date: Tue, 11 Mar 2025 19:18:04 +0100	[thread overview]
Message-ID: <[email protected]> (raw)

While the structure is refcounted, the only consumer incrementing it is
audit and even then the atomic operation is only needed when it
interacts with io_uring.

If putname spots a count of 1, there is no legitimate way for anyone to
bump it.

If audit is disabled, the count is guaranteed to be 1, which
consistently elides the atomic for all path lookups. If audit is
enabled, it still manages to elide the last decrement.

Note the patch does not do anything to prevent audit from suffering
atomics. See [1] and [2] for a different approach.

Benchmarked on Sapphire Rapids issuing access() (ops/s):
before: 5106246
after:  5269678 (+3%)

Link 1:	https://lore.kernel.org/linux-fsdevel/[email protected]/
Link 2: https://lore.kernel.org/linux-fsdevel/20250307164216.GI2023217@ZenIV/
Signed-off-by: Mateusz Guzik <[email protected]>
---

This is an alternative to the patch I linked above.

I think the improved commit message should also cover the feedback
Christian previously shared concerning it.

This is a trivial win until the atomic issue gets resolved, I don't see
any reason to NOT include it. At the same time I don't have that much
interest arguing about it either.

That is to say, here is a different take, if you don't like it, I'm
dropping the subject.

cheers

 fs/namei.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 06765d320e7e..add90981cfcd 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -275,14 +275,19 @@ EXPORT_SYMBOL(getname_kernel);
 
 void putname(struct filename *name)
 {
+	int refcnt;
+
 	if (IS_ERR_OR_NULL(name))
 		return;
 
-	if (WARN_ON_ONCE(!atomic_read(&name->refcnt)))
-		return;
+	refcnt = atomic_read(&name->refcnt);
+	if (refcnt != 1) {
+		if (WARN_ON_ONCE(!refcnt))
+			return;
 
-	if (!atomic_dec_and_test(&name->refcnt))
-		return;
+		if (!atomic_dec_and_test(&name->refcnt))
+			return;
+	}
 
 	if (name->name != name->iname) {
 		__putname(name->name);
-- 
2.43.0


             reply	other threads:[~2025-03-11 18:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-11 18:18 Mateusz Guzik [this message]
2025-03-12  8:36 ` [PATCH] fs: dodge an atomic in putname if ref == 1 Christian Brauner
2025-03-12 15:51 ` Jan Kara
2025-03-12 16:07 ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    [email protected] \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox