All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@kernel.org>
To: Anna Schumaker <anna@kernel.org>, Trond Myklebust <trondmy@kernel.org>
Cc: linux-nfs@vger.kernel.org,
	Jonathan Flynn <jonathan.flynn@hammerspace.com>
Subject: [PATCH 05/11] NFSv4/flexfiles: drop the redundant atomic from the busy timer
Date: Mon,  2 Nov 2026 14:11:03 -0500	[thread overview]
Message-ID: <20261102191109.13470-6-snitzer@kernel.org> (raw)
In-Reply-To: <20261102191109.13470-1-snitzer@kernel.org>

From: Mike Snitzer <snitzer@hammerspace.com>

nfs4_ff_busy_timer::n_ops is an atomic_t, but every access to it is
already serialised by the stripe lock:

  nfs4_ff_start_busy_timer() has one caller, nfs4_ff_layoutstat_start_io(),
  reached only from ff_layout_{read,write}_record_layoutstats_start()
  with dss[dss_id].lock held.

  nfs4_ff_end_busy_timer() has one caller,
  nfs4_ff_layout_stat_io_update_completed(), reached only from
  ff_layout_{read,write}_record_layoutstats_done() with the same lock
  held.

The two reporting paths, ff_layout_mirror_prepare_stats() and
ff_layout_encode_ff_layoutupdate(), take the stripe lock too and do not
touch n_ops at all.  There is no lockless reader.

So the atomic_inc_return()/atomic_dec_return() are locked read-modify-
writes on a line the CPU already owns exclusively -- pure serialisation
overhead, paid twice per DS RPC on the hottest path in this file, at the
275k-450k RPC/s the per-stripe lock work was measured at.

Make n_ops a plain int and document the locking on the struct.  Its size
is unchanged, so the stripe layout is unaffected.  The underflow check is
preserved.

Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 6 ++----
 fs/nfs/flexfilelayout/flexfilelayout.h | 6 +++++-
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 7c328f2ec900d..6a474fa7b0446 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -711,9 +711,8 @@ static void
 nfs4_ff_start_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
 {
 	/* first IO request? */
-	if (atomic_inc_return(&timer->n_ops) == 1) {
+	if (++timer->n_ops == 1)
 		timer->start_time = now;
-	}
 }
 
 static ktime_t
@@ -721,8 +720,7 @@ nfs4_ff_end_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
 {
 	ktime_t start;
 
-	if (atomic_dec_return(&timer->n_ops) < 0)
-		WARN_ON_ONCE(1);
+	WARN_ON_ONCE(--timer->n_ops < 0);
 
 	start = timer->start_time;
 	timer->start_time = now;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 658940979883a..d97001abc0d22 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -64,9 +64,13 @@ struct nfs4_ff_io_stat {
 	ktime_t				aggregate_completion_time;
 };
 
+/*
+ * Both members are serialised by the enclosing stripe's
+ * nfs4_ff_layout_ds_stripe::lock; see nfs4_ff_{start,end}_busy_timer().
+ */
 struct nfs4_ff_busy_timer {
 	ktime_t start_time;
-	atomic_t n_ops;
+	int n_ops;
 };
 
 struct nfs4_ff_layoutstat {
-- 
2.52.0


  parent reply	other threads:[~2026-11-02 19:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-02 19:10 [PATCH 00/11] NFSv4/flexfiles: scale layoutstats for striped layouts Mike Snitzer
2026-11-02 19:10 ` [PATCH 01/11] NFSv4/flexfiles: move layoutstats accounting to a per-stripe lock Mike Snitzer
2026-11-02 19:11 ` [PATCH 02/11] NFSv4/flexfiles: drop the now-unused per-mirror lock Mike Snitzer
2026-11-02 19:11 ` [PATCH 03/11] NFSv4/flexfiles: cacheline align the per-stripe layoutstats Mike Snitzer
2026-11-02 19:11 ` [PATCH 04/11] NFSv4/flexfiles: make dss_info->start_time write-once Mike Snitzer
2026-11-02 19:11 ` Mike Snitzer [this message]
2026-11-02 19:11 ` [PATCH 06/11] NFSv4/flexfiles: pair the in-flight count with the layoutstats updates Mike Snitzer
2026-11-02 19:11 ` [PATCH 07/11] NFSv4/flexfiles: derive ffil_ops_requested rather than storing it Mike Snitzer
2026-11-02 19:11 ` [PATCH 08/11] NFSv4/flexfiles: give each direction its own lock and cacheline Mike Snitzer
2026-11-02 19:11 ` [PATCH 09/11] NFSv4/flexfiles: drop NFS4_FF_MIRROR_STAT_AVAIL Mike Snitzer
2026-11-02 19:11 ` [PATCH 10/11] NFSv4/flexfiles: rotate LAYOUTSTATS reporting across a mirror's stripes Mike Snitzer
2026-11-02 19:11 ` [PATCH 11/11] NFSv4/flexfiles: take the report decision out of the critical section Mike Snitzer

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 \
    --in-reply-to=20261102191109.13470-6-snitzer@kernel.org \
    --to=snitzer@kernel.org \
    --cc=anna@kernel.org \
    --cc=jonathan.flynn@hammerspace.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=trondmy@kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.