[PATCH v2 00/12] NFSv4/flexfiles: scale layoutstats for striped layouts

[PATCH v2 00/12] NFSv4/flexfiles: scale layoutstats for striped layouts

[Mike Snitzer]

Hi,

Each stripe of a flexfiles mirror has its own read_stat and write_stat,
but the lock that protects them is in the mirror.  For a RAID0-style
layout (one mirror, many stripes) that lock is per file, and every DS
RPC takes it twice.

A 64-core client reads one file from a 32-stripe layout with a 128KiB
stripe unit (~36.7 GB/s, ~275k NFSv3 READ RPCs/s).  perf puts 45% of
its CPU in native_queued_spin_lock_slowpath, and 73% of the lock
contention events in the two layoutstats paths.

This series removes that bottleneck.  It also fixes which stripes a
LAYOUTSTATS report contains for a striped mirror.

Patches 1-2 (Jonathan) move the lock into the stripe and remove the
mirror lock.

Patches 3-8 keep the per-stripe hot path in the stripe's own
cachelines.  After patch 8, an I/O dirties one cacheline, and reads and
writes to one stripe share no lock.

Patches 9-10 fix LAYOUTSTATS for striped mirrors.  A report contained
only the lowest usable stripe of each mirror.  It now contains each
stripe with new completions, and rotates through the stripes when there
are more than four.

Patch 11 moves the report decision out of the critical section.

Patch 12 (new in v2) combines the read and write start helpers.

The wire encoding does not change.

Changes since v1, from Anna's review:
- Shorter commit messages throughout.  Removed the paragraphs and the
  "what the server sees" passages that Anna named.
- Code comments: the cacheline explanations are now only in the commit
  messages.  Patch 3 keeps the original lock comment.  Patch 7 cuts the
  ops_requested comment to its locking note.  Patch 9 cuts the
  last_reported_ops comment to its first sentence.  Patch 10 removes the
  scan comment.
- Patch 10: renamed dss_report_start to dss_report_index and removed its
  comment.
- New patch 12: one start helper for reads and writes.
- American spelling throughout.
- try_cmpxchg64() stays: last_report_time and start_time are ktime_t,
  not atomic64_t.

v1: https://lore.kernel.org/linux-nfs/20261102191109.13470-1-snitzer@kernel.org/

The series is based on Anna's linux-next (nfs-for-7.4-1).

Thanks,
Mike

Jonathan Flynn (2):
  NFSv4/flexfiles: move layoutstats accounting to a per-stripe lock
  NFSv4/flexfiles: drop the now-unused per-mirror lock

Mike Snitzer (10):
  NFSv4/flexfiles: cacheline align the per-stripe layoutstats
  NFSv4/flexfiles: make dss_info->start_time write-once
  NFSv4/flexfiles: drop the redundant atomic from the busy timer
  NFSv4/flexfiles: pair the in-flight count with the layoutstats updates
  NFSv4/flexfiles: derive ffil_ops_requested rather than storing it
  NFSv4/flexfiles: give each direction its own lock and cacheline
  NFSv4/flexfiles: drop NFS4_FF_MIRROR_STAT_AVAIL
  NFSv4/flexfiles: rotate LAYOUTSTATS reporting across a mirror's stripes
  NFSv4/flexfiles: take the report decision out of the critical section
  NFSv4/flexfiles: share one helper for the start of a read and a write

 fs/nfs/flexfilelayout/flexfilelayout.c | 257 +++++++++++++------------
 fs/nfs/flexfilelayout/flexfilelayout.h |  35 ++--
 2 files changed, 151 insertions(+), 141 deletions(-)


base-commit: 8cdff0c1549d3fce6bb652e6bc5dd7546e1cd1e3
-- 
2.52.0

[PATCH v2 01/12] NFSv4/flexfiles: move layoutstats accounting to a per-stripe lock

[Mike Snitzer]

From: Jonathan Flynn <jonathan.flynn@hammerspace.com>

Each stripe of a mirror has its own read_stat and write_stat, but the
lock that protects them is in the mirror.  Every DS RPC takes that lock
twice: in ff_layout_{read,write}_prepare_common() and in
ff_layout_{read,write}_count_stats().  For a RAID0-style layout (one
mirror, many stripes) the lock is per file, so striping adds RPC
parallelism but no lock parallelism.

A 64-core client reads one file from a 32-stripe layout with a 128KiB
stripe unit (~36.7 GB/s, ~275k NFSv3 READ RPCs/s).  It spends 45% of its
CPU in native_queued_spin_lock_slowpath.  lock:contention_begin puts
73% of the contention events in the two layoutstats paths:

  36.4%  rpc_exit_task -> ff_layout_read_count_stats ->
         ff_layout_read_record_layoutstats_done -> _raw_spin_lock
  36.3%  ff_layout_read_prepare_v3 ->
         ff_layout_read_record_layoutstats_start -> _raw_spin_lock

Move the lock into struct nfs4_ff_layout_ds_stripe, next to the
statistics it protects, so that I/O to different stripes does not
contend.  The reporting paths take the same per-stripe lock.

Set NFS4_FF_MIRROR_STAT_AVAIL outside the lock: it is a mirror-wide bit
operation and needs no lock.

ffl->last_report_time is shared by every mirror and stripe of a layout,
so no single mirror lock ever protected it.  Update it with a cmpxchg:
the caller that wins sends the LAYOUTSTATS report.

Signed-off-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 41 ++++++++++++++------------
 fs/nfs/flexfilelayout/flexfilelayout.h |  2 ++
 2 files changed, 24 insertions(+), 19 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 9492d6bc1d1a7..7b0e6bdf758fb 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -293,8 +293,10 @@ static struct nfs4_ff_layout_mirror *ff_layout_alloc_mirror(u32 dss_count,
 		return NULL;
 	}
 
-	for (u32 dss_id = 0; dss_id < mirror->dss_count; dss_id++)
+	for (u32 dss_id = 0; dss_id < mirror->dss_count; dss_id++) {
+		spin_lock_init(&mirror->dss[dss_id].lock);
 		nfs_localio_file_init(&mirror->dss[dss_id].nfl);
+	}
 
 	return mirror;
 }
@@ -736,6 +738,7 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_mirror *mirror,
 {
 	s64 report_interval = FF_LAYOUTSTATS_REPORT_INTERVAL;
 	struct nfs4_flexfile_layout *ffl = FF_LAYOUT_FROM_HDR(mirror->layout);
+	ktime_t last_report;
 
 	nfs4_ff_start_busy_timer(&layoutstat->busy_timer, now);
 	if (!mirror->dss[dss_id].start_time)
@@ -744,13 +747,13 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_mirror *mirror,
 		report_interval = (s64)mirror->report_interval * 1000LL;
 	else if (layoutstats_timer != 0)
 		report_interval = (s64)layoutstats_timer * 1000LL;
-	if (ktime_to_ms(ktime_sub(now, ffl->last_report_time)) >=
-			report_interval) {
-		ffl->last_report_time = now;
-		return true;
-	}
 
-	return false;
+	/* Shared by every mirror and stripe: the cmpxchg winner reports. */
+	last_report = READ_ONCE(ffl->last_report_time);
+	if (ktime_to_ms(ktime_sub(now, last_report)) < report_interval)
+		return false;
+
+	return try_cmpxchg64(&ffl->last_report_time, &last_report, now);
 }
 
 static void
@@ -794,13 +797,13 @@ nfs4_ff_layout_stat_io_start_read(struct inode *inode,
 {
 	bool report;
 
-	spin_lock(&mirror->lock);
+	spin_lock(&mirror->dss[dss_id].lock);
 	report = nfs4_ff_layoutstat_start_io(
 		mirror, dss_id, &mirror->dss[dss_id].read_stat, now);
 	nfs4_ff_layout_stat_io_update_requested(
 		&mirror->dss[dss_id].read_stat, requested);
+	spin_unlock(&mirror->dss[dss_id].lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
-	spin_unlock(&mirror->lock);
 
 	if (report)
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
@@ -813,12 +816,12 @@ nfs4_ff_layout_stat_io_end_read(struct rpc_task *task,
 		__u64 requested,
 		__u64 completed)
 {
-	spin_lock(&mirror->lock);
+	spin_lock(&mirror->dss[dss_id].lock);
 	nfs4_ff_layout_stat_io_update_completed(&mirror->dss[dss_id].read_stat,
 			requested, completed,
 			ktime_get(), task->tk_start);
+	spin_unlock(&mirror->dss[dss_id].lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
-	spin_unlock(&mirror->lock);
 }
 
 static void
@@ -829,7 +832,7 @@ nfs4_ff_layout_stat_io_start_write(struct inode *inode,
 {
 	bool report;
 
-	spin_lock(&mirror->lock);
+	spin_lock(&mirror->dss[dss_id].lock);
 	report = nfs4_ff_layoutstat_start_io(
 		mirror,
 		dss_id,
@@ -838,8 +841,8 @@ nfs4_ff_layout_stat_io_start_write(struct inode *inode,
 	nfs4_ff_layout_stat_io_update_requested(
 		&mirror->dss[dss_id].write_stat,
 		requested);
+	spin_unlock(&mirror->dss[dss_id].lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
-	spin_unlock(&mirror->lock);
 
 	if (report)
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
@@ -856,11 +859,11 @@ nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
 	if (committed == NFS_UNSTABLE)
 		requested = completed = 0;
 
-	spin_lock(&mirror->lock);
+	spin_lock(&mirror->dss[dss_id].lock);
 	nfs4_ff_layout_stat_io_update_completed(&mirror->dss[dss_id].write_stat,
 			requested, completed, ktime_get(), task->tk_start);
+	spin_unlock(&mirror->dss[dss_id].lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
-	spin_unlock(&mirror->lock);
 }
 
 static struct nfs4_ff_layout_ds *
@@ -3071,13 +3074,13 @@ ff_layout_encode_ff_layoutupdate(struct xdr_stream *xdr,
 	p = xdr_reserve_space(xdr, 4 + fh->size);
 	xdr_encode_opaque(p, fh->data, fh->size);
 	/* ff_io_latency4 read */
-	spin_lock(&dss_info->mirror->lock);
+	spin_lock(&dss_info->lock);
 	ff_layout_encode_io_latency(xdr,
 				    &dss_info->read_stat.io_stat);
 	/* ff_io_latency4 write */
 	ff_layout_encode_io_latency(xdr,
 				    &dss_info->write_stat.io_stat);
-	spin_unlock(&dss_info->mirror->lock);
+	spin_unlock(&dss_info->lock);
 	/* nfstime4 */
 	ff_layout_encode_nfstime(xdr,
 				 ktime_sub(ktime_get(),
@@ -3153,7 +3156,7 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 			       NFS4_DEVICEID4_SIZE);
 			devinfo->offset = 0;
 			devinfo->length = NFS4_MAX_UINT64;
-			spin_lock(&mirror->lock);
+			spin_lock(&dss_info->lock);
 			devinfo->read_count =
 			    dss_info->read_stat.io_stat.ops_completed;
 			devinfo->read_bytes =
@@ -3162,7 +3165,7 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 			    dss_info->write_stat.io_stat.ops_completed;
 			devinfo->write_bytes =
 			    dss_info->write_stat.io_stat.bytes_completed;
-			spin_unlock(&mirror->lock);
+			spin_unlock(&dss_info->lock);
 			devinfo->layout_type = LAYOUT_FLEX_FILES;
 			devinfo->ld_private.ops = &layoutstat_ops;
 			priv->dss_info = dss_info;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 09c3cd6964fd6..54aa492bc0bac 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -86,6 +86,8 @@ struct nfs4_ff_layout_ds_stripe {
 	const struct cred __rcu		*ro_cred;
 	const struct cred __rcu		*rw_cred;
 	struct nfs_file_localio		nfl;
+	/* Protects read_stat, write_stat and start_time below */
+	spinlock_t			lock;
 	struct nfs4_ff_layoutstat	read_stat;
 	struct nfs4_ff_layoutstat	write_stat;
 	ktime_t				start_time;
-- 
2.52.0

[PATCH v2 02/12] NFSv4/flexfiles: drop the now-unused per-mirror lock

[Mike Snitzer]

From: Jonathan Flynn <jonathan.flynn@hammerspace.com>

After the previous patch, nothing takes mirror->lock.  Remove it.

Signed-off-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 1 -
 fs/nfs/flexfilelayout/flexfilelayout.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 7b0e6bdf758fb..3ca37e94dd053 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -280,7 +280,6 @@ static struct nfs4_ff_layout_mirror *ff_layout_alloc_mirror(u32 dss_count,
 	if (mirror == NULL)
 		return NULL;
 
-	spin_lock_init(&mirror->lock);
 	refcount_set(&mirror->ref, 1);
 	INIT_LIST_HEAD(&mirror->mirrors);
 
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 54aa492bc0bac..8fa2d8851166a 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -99,7 +99,6 @@ struct nfs4_ff_layout_mirror {
 	u32				dss_count;
 	struct nfs4_ff_layout_ds_stripe *dss;
 	refcount_t			ref;
-	spinlock_t			lock;
 	unsigned long			flags;
 	u32				report_interval;
 };
-- 
2.52.0

[PATCH v2 03/12] NFSv4/flexfiles: cacheline align the per-stripe layoutstats

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

With a lock per stripe, stripes no longer share a lock, but they still
share cachelines.  struct nfs4_ff_layout_ds_stripe is 296 bytes with
8-byte alignment.  So in mirror->dss[], each stripe starts 40 bytes
further into a cacheline than the one before.  Taking stripe i's lock
invalidates the line with stripe i+1's lock and counters.

Cacheline align the lock.  The guarded members then start on a line
boundary.  The struct's alignment of 64 also rounds sizeof() up to a
multiple of 64.  So each stripe in the array starts on its own line.

The stripe grows from 296 to 384 bytes, 2816 bytes for a 32-stripe
layout.  The array base is aligned too: kvmalloc() gets it from a
kmalloc bucket of 512 bytes or more, or from vmalloc().

Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 8fa2d8851166a..2797f120a8324 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -14,6 +14,7 @@
 #define FF_FLAGS_NO_IO_THRU_MDS  2
 #define FF_FLAGS_NO_READ_IO      4
 
+#include <linux/cache.h>
 #include <linux/refcount.h>
 #include "../pnfs.h"
 
@@ -87,7 +88,7 @@ struct nfs4_ff_layout_ds_stripe {
 	const struct cred __rcu		*rw_cred;
 	struct nfs_file_localio		nfl;
 	/* Protects read_stat, write_stat and start_time below */
-	spinlock_t			lock;
+	spinlock_t			lock ____cacheline_aligned_in_smp;
 	struct nfs4_ff_layoutstat	read_stat;
 	struct nfs4_ff_layoutstat	write_stat;
 	ktime_t				start_time;
-- 
2.52.0

[PATCH v2 04/12] NFSv4/flexfiles: make dss_info->start_time write-once

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

nfs4_ff_layoutstat_start_io() writes start_time under the stripe lock,
and ff_layout_encode_ff_layoutupdate() reads it without the lock.

ffl_duration is measured from the stripe's first I/O, so start_time is
written once and then only read.  Publish it with try_cmpxchg64() so
that the first I/O sets it, and read it with READ_ONCE().  Later I/O
sees the value is set and does not write it.  ffl->last_report_time is
already published this way.

start_time then does not depend on the stripe lock, which a later patch
splits in two.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 3ca37e94dd053..eddedb1c6bf38 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -729,6 +729,17 @@ nfs4_ff_end_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
 	return ktime_sub(now, start);
 }
 
+/* ffl_duration runs from the stripe's first I/O: publish start_time once. */
+static void
+nfs4_ff_layoutstat_set_start_time(struct nfs4_ff_layout_ds_stripe *dss_info,
+				  ktime_t now)
+{
+	ktime_t unset = 0;
+
+	if (!READ_ONCE(dss_info->start_time))
+		try_cmpxchg64(&dss_info->start_time, &unset, now);
+}
+
 static bool
 nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_mirror *mirror,
 			    u32 dss_id,
@@ -740,8 +751,7 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_mirror *mirror,
 	ktime_t last_report;
 
 	nfs4_ff_start_busy_timer(&layoutstat->busy_timer, now);
-	if (!mirror->dss[dss_id].start_time)
-		mirror->dss[dss_id].start_time = now;
+	nfs4_ff_layoutstat_set_start_time(&mirror->dss[dss_id], now);
 	if (mirror->report_interval != 0)
 		report_interval = (s64)mirror->report_interval * 1000LL;
 	else if (layoutstats_timer != 0)
@@ -3083,7 +3093,7 @@ ff_layout_encode_ff_layoutupdate(struct xdr_stream *xdr,
 	/* nfstime4 */
 	ff_layout_encode_nfstime(xdr,
 				 ktime_sub(ktime_get(),
-					   dss_info->start_time));
+					   READ_ONCE(dss_info->start_time)));
 	/* bool */
 	p = xdr_reserve_space(xdr, 4);
 	*p = cpu_to_be32(false);
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 2797f120a8324..8119da3073423 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -87,10 +87,11 @@ struct nfs4_ff_layout_ds_stripe {
 	const struct cred __rcu		*ro_cred;
 	const struct cred __rcu		*rw_cred;
 	struct nfs_file_localio		nfl;
-	/* Protects read_stat, write_stat and start_time below */
+	/* Protects read_stat and write_stat below */
 	spinlock_t			lock ____cacheline_aligned_in_smp;
 	struct nfs4_ff_layoutstat	read_stat;
 	struct nfs4_ff_layoutstat	write_stat;
+	/* Published once by the first I/O; see nfs4_ff_layoutstat_set_start_time() */
 	ktime_t				start_time;
 };
 
-- 
2.52.0

[PATCH v2 05/12] NFSv4/flexfiles: drop the redundant atomic from the busy timer

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

nfs4_ff_busy_timer::n_ops is an atomic_t, but the stripe lock already
serializes every access to it.  Make it a plain int.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index eddedb1c6bf38..f5591d4451ab3 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 8119da3073423..4b2aaa16ee597 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -64,9 +64,10 @@ struct nfs4_ff_io_stat {
 	ktime_t				aggregate_completion_time;
 };
 
+/* Both members are protected by the enclosing stripe's lock */
 struct nfs4_ff_busy_timer {
 	ktime_t start_time;
-	atomic_t n_ops;
+	int n_ops;
 };
 
 struct nfs4_ff_layoutstat {
-- 
2.52.0

[PATCH v2 06/12] NFSv4/flexfiles: pair the in-flight count with the layoutstats updates

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

The in-flight count of the busy timer and the requested-side statistics
are two halves of one count, but the callers update them through two
helpers.  A new path could update one and not the other.

Fold nfs4_ff_layout_stat_io_update_requested() into
nfs4_ff_layoutstat_start_io(), and rename
nfs4_ff_layout_stat_io_update_completed() to
nfs4_ff_layoutstat_end_io().  Rename n_ops to ops_in_flight, which is
what it counts.

Both helpers now take the stripe, so they can assert the lock that
protects ops_in_flight.

No functional change.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index f5591d4451ab3..db8aa2bb92406 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -711,7 +711,7 @@ static void
 nfs4_ff_start_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
 {
 	/* first IO request? */
-	if (++timer->n_ops == 1)
+	if (++timer->ops_in_flight == 1)
 		timer->start_time = now;
 }
 
@@ -720,7 +720,7 @@ nfs4_ff_end_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
 {
 	ktime_t start;
 
-	WARN_ON_ONCE(--timer->n_ops < 0);
+	WARN_ON_ONCE(--timer->ops_in_flight < 0);
 
 	start = timer->start_time;
 	timer->start_time = now;
@@ -738,18 +738,27 @@ nfs4_ff_layoutstat_set_start_time(struct nfs4_ff_layout_ds_stripe *dss_info,
 		try_cmpxchg64(&dss_info->start_time, &unset, now);
 }
 
+/*
+ * Account for an I/O about to be sent to this stripe, and return true if a
+ * LAYOUTSTATS report is due.
+ */
 static bool
-nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_mirror *mirror,
-			    u32 dss_id,
+nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_ds_stripe *dss_info,
 			    struct nfs4_ff_layoutstat *layoutstat,
-			    ktime_t now)
+			    __u64 requested, ktime_t now)
 {
-	s64 report_interval = FF_LAYOUTSTATS_REPORT_INTERVAL;
+	struct nfs4_ff_layout_mirror *mirror = dss_info->mirror;
 	struct nfs4_flexfile_layout *ffl = FF_LAYOUT_FROM_HDR(mirror->layout);
+	s64 report_interval = FF_LAYOUTSTATS_REPORT_INTERVAL;
 	ktime_t last_report;
 
+	lockdep_assert_held(&dss_info->lock);
+
 	nfs4_ff_start_busy_timer(&layoutstat->busy_timer, now);
-	nfs4_ff_layoutstat_set_start_time(&mirror->dss[dss_id], now);
+	layoutstat->io_stat.ops_requested++;
+	layoutstat->io_stat.bytes_requested += requested;
+
+	nfs4_ff_layoutstat_set_start_time(dss_info, now);
 	if (mirror->report_interval != 0)
 		report_interval = (s64)mirror->report_interval * 1000LL;
 	else if (layoutstats_timer != 0)
@@ -763,27 +772,21 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_mirror *mirror,
 	return try_cmpxchg64(&ffl->last_report_time, &last_report, now);
 }
 
+/* Account for an I/O that has completed on this stripe. */
 static void
-nfs4_ff_layout_stat_io_update_requested(struct nfs4_ff_layoutstat *layoutstat,
-		__u64 requested)
-{
-	struct nfs4_ff_io_stat *iostat = &layoutstat->io_stat;
-
-	iostat->ops_requested++;
-	iostat->bytes_requested += requested;
-}
-
-static void
-nfs4_ff_layout_stat_io_update_completed(struct nfs4_ff_layoutstat *layoutstat,
-		__u64 requested,
-		__u64 completed,
-		ktime_t time_completed,
-		ktime_t time_started)
+nfs4_ff_layoutstat_end_io(struct nfs4_ff_layout_ds_stripe *dss_info,
+			  struct nfs4_ff_layoutstat *layoutstat,
+			  __u64 requested,
+			  __u64 completed,
+			  ktime_t time_completed,
+			  ktime_t time_started)
 {
 	struct nfs4_ff_io_stat *iostat = &layoutstat->io_stat;
 	ktime_t completion_time = ktime_sub(time_completed, time_started);
 	ktime_t timer;
 
+	lockdep_assert_held(&dss_info->lock);
+
 	iostat->ops_completed++;
 	iostat->bytes_completed += completed;
 	iostat->bytes_not_delivered += requested - completed;
@@ -802,14 +805,13 @@ nfs4_ff_layout_stat_io_start_read(struct inode *inode,
 		u32 dss_id,
 		__u64 requested, ktime_t now)
 {
+	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
 	bool report;
 
-	spin_lock(&mirror->dss[dss_id].lock);
-	report = nfs4_ff_layoutstat_start_io(
-		mirror, dss_id, &mirror->dss[dss_id].read_stat, now);
-	nfs4_ff_layout_stat_io_update_requested(
-		&mirror->dss[dss_id].read_stat, requested);
-	spin_unlock(&mirror->dss[dss_id].lock);
+	spin_lock(&dss_info->lock);
+	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->read_stat,
+					     requested, now);
+	spin_unlock(&dss_info->lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 
 	if (report)
@@ -823,11 +825,13 @@ nfs4_ff_layout_stat_io_end_read(struct rpc_task *task,
 		__u64 requested,
 		__u64 completed)
 {
-	spin_lock(&mirror->dss[dss_id].lock);
-	nfs4_ff_layout_stat_io_update_completed(&mirror->dss[dss_id].read_stat,
-			requested, completed,
-			ktime_get(), task->tk_start);
-	spin_unlock(&mirror->dss[dss_id].lock);
+	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
+
+	spin_lock(&dss_info->lock);
+	nfs4_ff_layoutstat_end_io(dss_info, &dss_info->read_stat,
+				  requested, completed,
+				  ktime_get(), task->tk_start);
+	spin_unlock(&dss_info->lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 }
 
@@ -837,18 +841,13 @@ nfs4_ff_layout_stat_io_start_write(struct inode *inode,
 		u32 dss_id,
 		__u64 requested, ktime_t now)
 {
+	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
 	bool report;
 
-	spin_lock(&mirror->dss[dss_id].lock);
-	report = nfs4_ff_layoutstat_start_io(
-		mirror,
-		dss_id,
-		&mirror->dss[dss_id].write_stat,
-		now);
-	nfs4_ff_layout_stat_io_update_requested(
-		&mirror->dss[dss_id].write_stat,
-		requested);
-	spin_unlock(&mirror->dss[dss_id].lock);
+	spin_lock(&dss_info->lock);
+	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->write_stat,
+					     requested, now);
+	spin_unlock(&dss_info->lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 
 	if (report)
@@ -863,13 +862,16 @@ nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
 		__u64 completed,
 		enum nfs3_stable_how committed)
 {
+	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
+
 	if (committed == NFS_UNSTABLE)
 		requested = completed = 0;
 
-	spin_lock(&mirror->dss[dss_id].lock);
-	nfs4_ff_layout_stat_io_update_completed(&mirror->dss[dss_id].write_stat,
-			requested, completed, ktime_get(), task->tk_start);
-	spin_unlock(&mirror->dss[dss_id].lock);
+	spin_lock(&dss_info->lock);
+	nfs4_ff_layoutstat_end_io(dss_info, &dss_info->write_stat,
+				  requested, completed,
+				  ktime_get(), task->tk_start);
+	spin_unlock(&dss_info->lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 }
 
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 4b2aaa16ee597..8d24f94263b3a 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -67,7 +67,7 @@ struct nfs4_ff_io_stat {
 /* Both members are protected by the enclosing stripe's lock */
 struct nfs4_ff_busy_timer {
 	ktime_t start_time;
-	int n_ops;
+	int ops_in_flight;
 };
 
 struct nfs4_ff_layoutstat {
-- 
2.52.0

[PATCH v2 07/12] NFSv4/flexfiles: derive ffil_ops_requested rather than storing it

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

An op is in flight from when it is counted as requested until it is
counted as completed.  After the previous patch both counts are kept
together under one lock, so

        ops_requested == ops_completed + ops_in_flight

at all times.  Drop nfs4_ff_io_stat::ops_requested and compute it when
LAYOUTSTATS is encoded.  The wire format does not change.

This removes 8 bytes from each direction's statistics, which the next
patch needs.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index db8aa2bb92406..877a9cc3f46be 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -755,7 +755,6 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_ds_stripe *dss_info,
 	lockdep_assert_held(&dss_info->lock);
 
 	nfs4_ff_start_busy_timer(&layoutstat->busy_timer, now);
-	layoutstat->io_stat.ops_requested++;
 	layoutstat->io_stat.bytes_requested += requested;
 
 	nfs4_ff_layoutstat_set_start_time(dss_info, now);
@@ -3049,12 +3048,13 @@ ff_layout_encode_nfstime(struct xdr_stream *xdr,
 
 static void
 ff_layout_encode_io_latency(struct xdr_stream *xdr,
-			    struct nfs4_ff_io_stat *stat)
+			    const struct nfs4_ff_layoutstat *layoutstat)
 {
+	const struct nfs4_ff_io_stat *stat = &layoutstat->io_stat;
 	__be32 *p;
 
 	p = xdr_reserve_space(xdr, 5 * 8);
-	p = xdr_encode_hyper(p, stat->ops_requested);
+	p = xdr_encode_hyper(p, nfs4_ff_ops_requested(layoutstat));
 	p = xdr_encode_hyper(p, stat->bytes_requested);
 	p = xdr_encode_hyper(p, stat->ops_completed);
 	p = xdr_encode_hyper(p, stat->bytes_completed);
@@ -3084,11 +3084,9 @@ ff_layout_encode_ff_layoutupdate(struct xdr_stream *xdr,
 	xdr_encode_opaque(p, fh->data, fh->size);
 	/* ff_io_latency4 read */
 	spin_lock(&dss_info->lock);
-	ff_layout_encode_io_latency(xdr,
-				    &dss_info->read_stat.io_stat);
+	ff_layout_encode_io_latency(xdr, &dss_info->read_stat);
 	/* ff_io_latency4 write */
-	ff_layout_encode_io_latency(xdr,
-				    &dss_info->write_stat.io_stat);
+	ff_layout_encode_io_latency(xdr, &dss_info->write_stat);
 	spin_unlock(&dss_info->lock);
 	/* nfstime4 */
 	ff_layout_encode_nfstime(xdr,
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 8d24f94263b3a..a8f357a7000d0 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -55,7 +55,6 @@ struct nfs4_ff_layout_ds_err {
 };
 
 struct nfs4_ff_io_stat {
-	__u64				ops_requested;
 	__u64				bytes_requested;
 	__u64				ops_completed;
 	__u64				bytes_completed;
@@ -75,6 +74,14 @@ struct nfs4_ff_layoutstat {
 	struct nfs4_ff_busy_timer busy_timer;
 };
 
+/* Caller must hold the lock guarding this nfs4_ff_layoutstat. */
+static inline __u64
+nfs4_ff_ops_requested(const struct nfs4_ff_layoutstat *layoutstat)
+{
+	return layoutstat->io_stat.ops_completed +
+	       layoutstat->busy_timer.ops_in_flight;
+}
+
 struct nfs4_ff_layout_mirror;
 
 struct nfs4_ff_layout_ds_stripe {
-- 
2.52.0

[PATCH v2 08/12] NFSv4/flexfiles: give each direction its own lock and cacheline

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

A read uses only read_stat and a write only write_stat, but both take
the same stripe lock, and an I/O dirties two cachelines.

Move the lock into struct nfs4_ff_layoutstat, with the in-flight count,
and replace nfs4_ff_busy_timer with a ktime_t.  The structure is then
exactly 64 bytes:

        spinlock_t             lock              4
        int                    ops_in_flight     4
        struct nfs4_ff_io_stat io_stat          48
        ktime_t                busy_start_time   8

Align it to a cacheline.  An I/O now dirties one line, and a read and a
write to one stripe share neither a lock nor a line.  The stripe
shrinks from 384 to 320 bytes, still a multiple of 64 and of 128.

The reporting paths take the two locks in turn.  The statistics are
advisory and were never sampled atomically with in-flight I/O.

With CONFIG_DEBUG_SPINLOCK or CONFIG_PROVE_LOCKING, spinlock_t is larger
and the structure takes two lines.  This affects only the packing.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 877a9cc3f46be..192f0bd4918f9 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -293,7 +293,8 @@ static struct nfs4_ff_layout_mirror *ff_layout_alloc_mirror(u32 dss_count,
 	}
 
 	for (u32 dss_id = 0; dss_id < mirror->dss_count; dss_id++) {
-		spin_lock_init(&mirror->dss[dss_id].lock);
+		spin_lock_init(&mirror->dss[dss_id].read_stat.lock);
+		spin_lock_init(&mirror->dss[dss_id].write_stat.lock);
 		nfs_localio_file_init(&mirror->dss[dss_id].nfl);
 	}
 
@@ -708,22 +709,22 @@ static u32 calc_dss_id_from_commit(struct pnfs_layout_segment *lseg,
 }
 
 static void
-nfs4_ff_start_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
+nfs4_ff_start_busy_timer(struct nfs4_ff_layoutstat *layoutstat, ktime_t now)
 {
 	/* first IO request? */
-	if (++timer->ops_in_flight == 1)
-		timer->start_time = now;
+	if (++layoutstat->ops_in_flight == 1)
+		layoutstat->busy_start_time = now;
 }
 
 static ktime_t
-nfs4_ff_end_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
+nfs4_ff_end_busy_timer(struct nfs4_ff_layoutstat *layoutstat, ktime_t now)
 {
 	ktime_t start;
 
-	WARN_ON_ONCE(--timer->ops_in_flight < 0);
+	WARN_ON_ONCE(--layoutstat->ops_in_flight < 0);
 
-	start = timer->start_time;
-	timer->start_time = now;
+	start = layoutstat->busy_start_time;
+	layoutstat->busy_start_time = now;
 	return ktime_sub(now, start);
 }
 
@@ -752,9 +753,9 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_ds_stripe *dss_info,
 	s64 report_interval = FF_LAYOUTSTATS_REPORT_INTERVAL;
 	ktime_t last_report;
 
-	lockdep_assert_held(&dss_info->lock);
+	lockdep_assert_held(&layoutstat->lock);
 
-	nfs4_ff_start_busy_timer(&layoutstat->busy_timer, now);
+	nfs4_ff_start_busy_timer(layoutstat, now);
 	layoutstat->io_stat.bytes_requested += requested;
 
 	nfs4_ff_layoutstat_set_start_time(dss_info, now);
@@ -784,13 +785,13 @@ nfs4_ff_layoutstat_end_io(struct nfs4_ff_layout_ds_stripe *dss_info,
 	ktime_t completion_time = ktime_sub(time_completed, time_started);
 	ktime_t timer;
 
-	lockdep_assert_held(&dss_info->lock);
+	lockdep_assert_held(&layoutstat->lock);
 
 	iostat->ops_completed++;
 	iostat->bytes_completed += completed;
 	iostat->bytes_not_delivered += requested - completed;
 
-	timer = nfs4_ff_end_busy_timer(&layoutstat->busy_timer, time_completed);
+	timer = nfs4_ff_end_busy_timer(layoutstat, time_completed);
 	iostat->total_busy_time =
 			ktime_add(iostat->total_busy_time, timer);
 	iostat->aggregate_completion_time =
@@ -807,10 +808,10 @@ nfs4_ff_layout_stat_io_start_read(struct inode *inode,
 	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
 	bool report;
 
-	spin_lock(&dss_info->lock);
+	spin_lock(&dss_info->read_stat.lock);
 	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->read_stat,
 					     requested, now);
-	spin_unlock(&dss_info->lock);
+	spin_unlock(&dss_info->read_stat.lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 
 	if (report)
@@ -826,11 +827,11 @@ nfs4_ff_layout_stat_io_end_read(struct rpc_task *task,
 {
 	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
 
-	spin_lock(&dss_info->lock);
+	spin_lock(&dss_info->read_stat.lock);
 	nfs4_ff_layoutstat_end_io(dss_info, &dss_info->read_stat,
 				  requested, completed,
 				  ktime_get(), task->tk_start);
-	spin_unlock(&dss_info->lock);
+	spin_unlock(&dss_info->read_stat.lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 }
 
@@ -843,10 +844,10 @@ nfs4_ff_layout_stat_io_start_write(struct inode *inode,
 	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
 	bool report;
 
-	spin_lock(&dss_info->lock);
+	spin_lock(&dss_info->write_stat.lock);
 	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->write_stat,
 					     requested, now);
-	spin_unlock(&dss_info->lock);
+	spin_unlock(&dss_info->write_stat.lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 
 	if (report)
@@ -866,11 +867,11 @@ nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
 	if (committed == NFS_UNSTABLE)
 		requested = completed = 0;
 
-	spin_lock(&dss_info->lock);
+	spin_lock(&dss_info->write_stat.lock);
 	nfs4_ff_layoutstat_end_io(dss_info, &dss_info->write_stat,
 				  requested, completed,
 				  ktime_get(), task->tk_start);
-	spin_unlock(&dss_info->lock);
+	spin_unlock(&dss_info->write_stat.lock);
 	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 }
 
@@ -3083,11 +3084,13 @@ ff_layout_encode_ff_layoutupdate(struct xdr_stream *xdr,
 	p = xdr_reserve_space(xdr, 4 + fh->size);
 	xdr_encode_opaque(p, fh->data, fh->size);
 	/* ff_io_latency4 read */
-	spin_lock(&dss_info->lock);
+	spin_lock(&dss_info->read_stat.lock);
 	ff_layout_encode_io_latency(xdr, &dss_info->read_stat);
+	spin_unlock(&dss_info->read_stat.lock);
 	/* ff_io_latency4 write */
+	spin_lock(&dss_info->write_stat.lock);
 	ff_layout_encode_io_latency(xdr, &dss_info->write_stat);
-	spin_unlock(&dss_info->lock);
+	spin_unlock(&dss_info->write_stat.lock);
 	/* nfstime4 */
 	ff_layout_encode_nfstime(xdr,
 				 ktime_sub(ktime_get(),
@@ -3163,16 +3166,18 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 			       NFS4_DEVICEID4_SIZE);
 			devinfo->offset = 0;
 			devinfo->length = NFS4_MAX_UINT64;
-			spin_lock(&dss_info->lock);
+			spin_lock(&dss_info->read_stat.lock);
 			devinfo->read_count =
 			    dss_info->read_stat.io_stat.ops_completed;
 			devinfo->read_bytes =
 			    dss_info->read_stat.io_stat.bytes_completed;
+			spin_unlock(&dss_info->read_stat.lock);
+			spin_lock(&dss_info->write_stat.lock);
 			devinfo->write_count =
 			    dss_info->write_stat.io_stat.ops_completed;
 			devinfo->write_bytes =
 			    dss_info->write_stat.io_stat.bytes_completed;
-			spin_unlock(&dss_info->lock);
+			spin_unlock(&dss_info->write_stat.lock);
 			devinfo->layout_type = LAYOUT_FLEX_FILES;
 			devinfo->ld_private.ops = &layoutstat_ops;
 			priv->dss_info = dss_info;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index a8f357a7000d0..76ff6575119d1 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -63,23 +63,21 @@ struct nfs4_ff_io_stat {
 	ktime_t				aggregate_completion_time;
 };
 
-/* Both members are protected by the enclosing stripe's lock */
-struct nfs4_ff_busy_timer {
-	ktime_t start_time;
-	int ops_in_flight;
-};
-
+/* One direction's statistics for one stripe, with the lock that guards them */
 struct nfs4_ff_layoutstat {
-	struct nfs4_ff_io_stat io_stat;
-	struct nfs4_ff_busy_timer busy_timer;
-};
+	/* Protects every other member of this structure */
+	spinlock_t			lock;
+	int				ops_in_flight;
+	struct nfs4_ff_io_stat		io_stat;
+	/* Start of the interval io_stat.total_busy_time accrues */
+	ktime_t				busy_start_time;
+} ____cacheline_aligned_in_smp;
 
 /* Caller must hold the lock guarding this nfs4_ff_layoutstat. */
 static inline __u64
 nfs4_ff_ops_requested(const struct nfs4_ff_layoutstat *layoutstat)
 {
-	return layoutstat->io_stat.ops_completed +
-	       layoutstat->busy_timer.ops_in_flight;
+	return layoutstat->io_stat.ops_completed + layoutstat->ops_in_flight;
 }
 
 struct nfs4_ff_layout_mirror;
@@ -95,12 +93,10 @@ struct nfs4_ff_layout_ds_stripe {
 	const struct cred __rcu		*ro_cred;
 	const struct cred __rcu		*rw_cred;
 	struct nfs_file_localio		nfl;
-	/* Protects read_stat and write_stat below */
-	spinlock_t			lock ____cacheline_aligned_in_smp;
-	struct nfs4_ff_layoutstat	read_stat;
-	struct nfs4_ff_layoutstat	write_stat;
 	/* Published once by the first I/O; see nfs4_ff_layoutstat_set_start_time() */
 	ktime_t				start_time;
+	struct nfs4_ff_layoutstat	read_stat;
+	struct nfs4_ff_layoutstat	write_stat;
 };
 
 struct nfs4_ff_layout_mirror {
-- 
2.52.0

[PATCH v2 09/12] NFSv4/flexfiles: drop NFS4_FF_MIRROR_STAT_AVAIL

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

Each DS RPC sets NFS4_FF_MIRROR_STAT_AVAIL twice, at start and at
completion.  These are two atomic operations per RPC on a cacheline that
all stripes of the mirror share.  The flag is read once per report
interval.

The flag also hides most stripes.  It is per mirror, but
ff_layout_mirror_prepare_stats() tests and clears it in the per-stripe
loop.  The first stripe clears it, so a LAYOUTSTATS report contains only
the lowest usable stripe of each mirror.

Remove the flag.  Record each stripe's completed ops when it is
reported, and skip a stripe whose count did not change.  A report then
contains each stripe that completed I/O since its last report, up to the
dev_limit of the caller.  The reported values do not change.
LAYOUTRETURN still reports every stripe that fits.

The new last_reported_ops is in an existing hole in the read-mostly part
of the stripe.  The I/O paths do not touch it, and the stripe stays 320
bytes.  Both callers of ff_layout_mirror_prepare_stats() hold the
inode's i_lock, which serializes it.

NFS4_FF_MIRROR_STAT_AVAIL was the only flag in
nfs4_ff_layout_mirror::flags, so remove that field too.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 192f0bd4918f9..91600086526ee 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -812,7 +812,6 @@ nfs4_ff_layout_stat_io_start_read(struct inode *inode,
 	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->read_stat,
 					     requested, now);
 	spin_unlock(&dss_info->read_stat.lock);
-	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 
 	if (report)
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
@@ -832,7 +831,6 @@ nfs4_ff_layout_stat_io_end_read(struct rpc_task *task,
 				  requested, completed,
 				  ktime_get(), task->tk_start);
 	spin_unlock(&dss_info->read_stat.lock);
-	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 }
 
 static void
@@ -848,7 +846,6 @@ nfs4_ff_layout_stat_io_start_write(struct inode *inode,
 	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->write_stat,
 					     requested, now);
 	spin_unlock(&dss_info->write_stat.lock);
-	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 
 	if (report)
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
@@ -872,7 +869,6 @@ nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
 				  requested, completed,
 				  ktime_get(), task->tk_start);
 	spin_unlock(&dss_info->write_stat.lock);
-	set_bit(NFS4_FF_MIRROR_STAT_AVAIL, &mirror->flags);
 }
 
 static struct nfs4_ff_layout_ds *
@@ -3139,6 +3135,7 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 	struct nfs4_ff_layout_mirror *mirror;
 	struct nfs4_ff_layout_ds_stripe *dss_info;
 	struct nfs4_ff_layout_ds *mirror_ds;
+	__u64 read_count, read_bytes, write_count, write_bytes, ops;
 	int i = 0, dss_id;
 
 	rcu_read_lock();
@@ -3150,13 +3147,26 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 			mirror_ds = rcu_dereference(dss_info->mirror_ds);
 			if (IS_ERR_OR_NULL(mirror_ds))
 				continue;
-			if (!test_and_clear_bit(NFS4_FF_MIRROR_STAT_AVAIL,
-						&mirror->flags) &&
+
+			spin_lock(&dss_info->read_stat.lock);
+			read_count = dss_info->read_stat.io_stat.ops_completed;
+			read_bytes = dss_info->read_stat.io_stat.bytes_completed;
+			spin_unlock(&dss_info->read_stat.lock);
+			spin_lock(&dss_info->write_stat.lock);
+			write_count = dss_info->write_stat.io_stat.ops_completed;
+			write_bytes = dss_info->write_stat.io_stat.bytes_completed;
+			spin_unlock(&dss_info->write_stat.lock);
+
+			/* Skip a stripe with no completions since its last report */
+			ops = read_count + write_count;
+			if (ops == dss_info->last_reported_ops &&
 			    type != NFS4_FF_OP_LAYOUTRETURN)
 				continue;
+
 			/* mirror refcount put in cleanup_layoutstats */
 			if (!refcount_inc_not_zero(&mirror->ref))
 				continue;
+			dss_info->last_reported_ops = ops;
 			/* The pin holds a reference; it is exchanged out only
 			 * under i_lock.  Put in ff_layout_free_layoutstats().
 			 */
@@ -3166,18 +3176,10 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 			       NFS4_DEVICEID4_SIZE);
 			devinfo->offset = 0;
 			devinfo->length = NFS4_MAX_UINT64;
-			spin_lock(&dss_info->read_stat.lock);
-			devinfo->read_count =
-			    dss_info->read_stat.io_stat.ops_completed;
-			devinfo->read_bytes =
-			    dss_info->read_stat.io_stat.bytes_completed;
-			spin_unlock(&dss_info->read_stat.lock);
-			spin_lock(&dss_info->write_stat.lock);
-			devinfo->write_count =
-			    dss_info->write_stat.io_stat.ops_completed;
-			devinfo->write_bytes =
-			    dss_info->write_stat.io_stat.bytes_completed;
-			spin_unlock(&dss_info->write_stat.lock);
+			devinfo->read_count = read_count;
+			devinfo->read_bytes = read_bytes;
+			devinfo->write_count = write_count;
+			devinfo->write_bytes = write_bytes;
 			devinfo->layout_type = LAYOUT_FLEX_FILES;
 			devinfo->ld_private.ops = &layoutstat_ops;
 			priv->dss_info = dss_info;
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 76ff6575119d1..df91165ba72ee 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -95,6 +95,8 @@ struct nfs4_ff_layout_ds_stripe {
 	struct nfs_file_localio		nfl;
 	/* Published once by the first I/O; see nfs4_ff_layoutstat_set_start_time() */
 	ktime_t				start_time;
+	/* Completed ops, both directions, as of this stripe's last report */
+	__u64				last_reported_ops;
 	struct nfs4_ff_layoutstat	read_stat;
 	struct nfs4_ff_layoutstat	write_stat;
 };
@@ -105,12 +107,9 @@ struct nfs4_ff_layout_mirror {
 	u32				dss_count;
 	struct nfs4_ff_layout_ds_stripe *dss;
 	refcount_t			ref;
-	unsigned long			flags;
 	u32				report_interval;
 };
 
-#define NFS4_FF_MIRROR_STAT_AVAIL	(0)
-
 struct nfs4_ff_layout_segment {
 	struct pnfs_layout_segment	generic_hdr;
 	u64				stripe_unit;
-- 
2.52.0

[PATCH v2 10/12] NFSv4/flexfiles: rotate LAYOUTSTATS reporting across a mirror's stripes

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

Both callers of ff_layout_mirror_prepare_stats() limit a report to four
ff_layoutupdate4 entries, and the scan of mirror->dss[] stops when the
limit is reached.  On a mirror with more than four busy stripes, every
report contains the same four stripes, and the others are reported only
at LAYOUTRETURN.

Record where each mirror's scan stopped, and start the next scan there.
The index moves past each stripe that gets a slot, and idle stripes do
not use a slot.  So every busy stripe is reported within
ceil(busy_stripes / 4) report intervals.  The values and the encoding
do not change.

The mirror loop gets the same dev_limit test as the stripe loop, so a
full report stops the scan.

[jonathan.flynn: read dss_report_index once per mirror, before the
 stripe loop.  Reading it again in the loop skipped stripes and could
 report one stripe twice.]
Signed-off-by: Jonathan Flynn <jonathan.flynn@hammerspace.com>
Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 15 ++++++++++++---
 fs/nfs/flexfilelayout/flexfilelayout.h |  1 +
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 91600086526ee..b4d4971816145 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -3136,14 +3136,21 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 	struct nfs4_ff_layout_ds_stripe *dss_info;
 	struct nfs4_ff_layout_ds *mirror_ds;
 	__u64 read_count, read_bytes, write_count, write_bytes, ops;
-	int i = 0, dss_id;
+	u32 dss_id, scanned, base;
+	int i = 0;
 
 	rcu_read_lock();
 	list_for_each_entry(mirror, &ff_layout->mirrors, mirrors) {
-		for (dss_id = 0; dss_id < mirror->dss_count; ++dss_id) {
-			dss_info = &mirror->dss[dss_id];
+		if (i >= dev_limit)
+			break;
+		base = mirror->dss_report_index;
+		for (scanned = 0; scanned < mirror->dss_count; ++scanned) {
 			if (i >= dev_limit)
 				break;
+			dss_id = base + scanned;
+			if (dss_id >= mirror->dss_count)
+				dss_id -= mirror->dss_count;
+			dss_info = &mirror->dss[dss_id];
 			mirror_ds = rcu_dereference(dss_info->mirror_ds);
 			if (IS_ERR_OR_NULL(mirror_ds))
 				continue;
@@ -3167,6 +3174,8 @@ ff_layout_mirror_prepare_stats(struct pnfs_layout_hdr *lo,
 			if (!refcount_inc_not_zero(&mirror->ref))
 				continue;
 			dss_info->last_reported_ops = ops;
+			mirror->dss_report_index =
+				dss_id + 1 < mirror->dss_count ? dss_id + 1 : 0;
 			/* The pin holds a reference; it is exchanged out only
 			 * under i_lock.  Put in ff_layout_free_layoutstats().
 			 */
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index df91165ba72ee..33ad85d1d07a2 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -108,6 +108,7 @@ struct nfs4_ff_layout_mirror {
 	struct nfs4_ff_layout_ds_stripe *dss;
 	refcount_t			ref;
 	u32				report_interval;
+	u32				dss_report_index;
 };
 
 struct nfs4_ff_layout_segment {
-- 
2.52.0

[PATCH v2 11/12] NFSv4/flexfiles: take the report decision out of the critical section

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

When an I/O to a data server starts, the stripe lock also covered the
decision to send a LAYOUTSTATS report.  That decision reads the mirror
and the layout header, divides in ktime_to_ms(), and does a cmpxchg on
ffl->last_report_time.  None of it is data that the lock protects.

Move the decision into nfs4_ff_layoutstat_report_due() and call it,
and nfs4_ff_layoutstat_set_start_time(), after the unlock.  The lock now
covers two writes to one cacheline.  The order of the updates and the
report does not change.

nfs4_ff_layoutstat_end_io() loses its unused dss_info argument.

Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 49 ++++++++++++++------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index b4d4971816145..f3f4573880d98 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -739,32 +739,36 @@ nfs4_ff_layoutstat_set_start_time(struct nfs4_ff_layout_ds_stripe *dss_info,
 		try_cmpxchg64(&dss_info->start_time, &unset, now);
 }
 
-/*
- * Account for an I/O about to be sent to this stripe, and return true if a
- * LAYOUTSTATS report is due.
- */
-static bool
-nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_ds_stripe *dss_info,
-			    struct nfs4_ff_layoutstat *layoutstat,
+/* Account for an I/O about to be sent to this stripe. */
+static void
+nfs4_ff_layoutstat_start_io(struct nfs4_ff_layoutstat *layoutstat,
 			    __u64 requested, ktime_t now)
+{
+	lockdep_assert_held(&layoutstat->lock);
+
+	nfs4_ff_start_busy_timer(layoutstat, now);
+	layoutstat->io_stat.bytes_requested += requested;
+}
+
+/*
+ * Is a LAYOUTSTATS report due?  Called outside the statistics locks, which
+ * protect none of this.  last_report_time is shared by every mirror and
+ * stripe of the layout: the cmpxchg winner reports.
+ */
+static bool
+nfs4_ff_layoutstat_report_due(struct nfs4_ff_layout_ds_stripe *dss_info,
+			      ktime_t now)
 {
 	struct nfs4_ff_layout_mirror *mirror = dss_info->mirror;
 	struct nfs4_flexfile_layout *ffl = FF_LAYOUT_FROM_HDR(mirror->layout);
 	s64 report_interval = FF_LAYOUTSTATS_REPORT_INTERVAL;
 	ktime_t last_report;
 
-	lockdep_assert_held(&layoutstat->lock);
-
-	nfs4_ff_start_busy_timer(layoutstat, now);
-	layoutstat->io_stat.bytes_requested += requested;
-
-	nfs4_ff_layoutstat_set_start_time(dss_info, now);
 	if (mirror->report_interval != 0)
 		report_interval = (s64)mirror->report_interval * 1000LL;
 	else if (layoutstats_timer != 0)
 		report_interval = (s64)layoutstats_timer * 1000LL;
 
-	/* Shared by every mirror and stripe: the cmpxchg winner reports. */
 	last_report = READ_ONCE(ffl->last_report_time);
 	if (ktime_to_ms(ktime_sub(now, last_report)) < report_interval)
 		return false;
@@ -774,8 +778,7 @@ nfs4_ff_layoutstat_start_io(struct nfs4_ff_layout_ds_stripe *dss_info,
 
 /* Account for an I/O that has completed on this stripe. */
 static void
-nfs4_ff_layoutstat_end_io(struct nfs4_ff_layout_ds_stripe *dss_info,
-			  struct nfs4_ff_layoutstat *layoutstat,
+nfs4_ff_layoutstat_end_io(struct nfs4_ff_layoutstat *layoutstat,
 			  __u64 requested,
 			  __u64 completed,
 			  ktime_t time_completed,
@@ -809,10 +812,11 @@ nfs4_ff_layout_stat_io_start_read(struct inode *inode,
 	bool report;
 
 	spin_lock(&dss_info->read_stat.lock);
-	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->read_stat,
-					     requested, now);
+	nfs4_ff_layoutstat_start_io(&dss_info->read_stat, requested, now);
 	spin_unlock(&dss_info->read_stat.lock);
 
+	nfs4_ff_layoutstat_set_start_time(dss_info, now);
+	report = nfs4_ff_layoutstat_report_due(dss_info, now);
 	if (report)
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
 }
@@ -827,7 +831,7 @@ nfs4_ff_layout_stat_io_end_read(struct rpc_task *task,
 	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
 
 	spin_lock(&dss_info->read_stat.lock);
-	nfs4_ff_layoutstat_end_io(dss_info, &dss_info->read_stat,
+	nfs4_ff_layoutstat_end_io(&dss_info->read_stat,
 				  requested, completed,
 				  ktime_get(), task->tk_start);
 	spin_unlock(&dss_info->read_stat.lock);
@@ -843,10 +847,11 @@ nfs4_ff_layout_stat_io_start_write(struct inode *inode,
 	bool report;
 
 	spin_lock(&dss_info->write_stat.lock);
-	report = nfs4_ff_layoutstat_start_io(dss_info, &dss_info->write_stat,
-					     requested, now);
+	nfs4_ff_layoutstat_start_io(&dss_info->write_stat, requested, now);
 	spin_unlock(&dss_info->write_stat.lock);
 
+	nfs4_ff_layoutstat_set_start_time(dss_info, now);
+	report = nfs4_ff_layoutstat_report_due(dss_info, now);
 	if (report)
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
 }
@@ -865,7 +870,7 @@ nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
 		requested = completed = 0;
 
 	spin_lock(&dss_info->write_stat.lock);
-	nfs4_ff_layoutstat_end_io(dss_info, &dss_info->write_stat,
+	nfs4_ff_layoutstat_end_io(&dss_info->write_stat,
 				  requested, completed,
 				  ktime_get(), task->tk_start);
 	spin_unlock(&dss_info->write_stat.lock);
-- 
2.52.0

[PATCH v2 12/12] NFSv4/flexfiles: share one helper for the start of a read and a write

[Mike Snitzer]

From: Mike Snitzer <snitzer@hammerspace.com>

nfs4_ff_layout_stat_io_start_read() and
nfs4_ff_layout_stat_io_start_write() differ only in the statistics they
update.  Replace them with nfs4_ff_layout_stat_io_start(), which takes
the direction of the I/O.

No functional change.

Suggested-by: Anna Schumaker <anna@kernel.org>
Signed-off-by: Mike Snitzer <snitzer@hammerspace.com>
---
 fs/nfs/flexfilelayout/flexfilelayout.c | 62 +++++++-------------------
 1 file changed, 17 insertions(+), 45 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index f3f4573880d98..a28224637d394 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -803,21 +803,21 @@ nfs4_ff_layoutstat_end_io(struct nfs4_ff_layoutstat *layoutstat,
 }
 
 static void
-nfs4_ff_layout_stat_io_start_read(struct inode *inode,
-		struct nfs4_ff_layout_mirror *mirror,
-		u32 dss_id,
-		__u64 requested, ktime_t now)
+nfs4_ff_layout_stat_io_start(struct inode *inode,
+			     struct nfs4_ff_layout_mirror *mirror,
+			     u32 dss_id, bool write,
+			     __u64 requested, ktime_t now)
 {
 	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
-	bool report;
+	struct nfs4_ff_layoutstat *layoutstat =
+		write ? &dss_info->write_stat : &dss_info->read_stat;
 
-	spin_lock(&dss_info->read_stat.lock);
-	nfs4_ff_layoutstat_start_io(&dss_info->read_stat, requested, now);
-	spin_unlock(&dss_info->read_stat.lock);
+	spin_lock(&layoutstat->lock);
+	nfs4_ff_layoutstat_start_io(layoutstat, requested, now);
+	spin_unlock(&layoutstat->lock);
 
 	nfs4_ff_layoutstat_set_start_time(dss_info, now);
-	report = nfs4_ff_layoutstat_report_due(dss_info, now);
-	if (report)
+	if (nfs4_ff_layoutstat_report_due(dss_info, now))
 		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
 }
 
@@ -837,25 +837,6 @@ nfs4_ff_layout_stat_io_end_read(struct rpc_task *task,
 	spin_unlock(&dss_info->read_stat.lock);
 }
 
-static void
-nfs4_ff_layout_stat_io_start_write(struct inode *inode,
-		struct nfs4_ff_layout_mirror *mirror,
-		u32 dss_id,
-		__u64 requested, ktime_t now)
-{
-	struct nfs4_ff_layout_ds_stripe *dss_info = &mirror->dss[dss_id];
-	bool report;
-
-	spin_lock(&dss_info->write_stat.lock);
-	nfs4_ff_layoutstat_start_io(&dss_info->write_stat, requested, now);
-	spin_unlock(&dss_info->write_stat.lock);
-
-	nfs4_ff_layoutstat_set_start_time(dss_info, now);
-	report = nfs4_ff_layoutstat_report_due(dss_info, now);
-	if (report)
-		pnfs_report_layoutstat(inode, nfs_io_gfp_mask());
-}
-
 static void
 nfs4_ff_layout_stat_io_end_write(struct rpc_task *task,
 		struct nfs4_ff_layout_mirror *mirror,
@@ -1772,12 +1753,8 @@ static void ff_layout_read_record_layoutstats_start(struct rpc_task *task,
 		mirror->dss_count,
 		hdr->args.offset);
 
-	nfs4_ff_layout_stat_io_start_read(
-		hdr->inode,
-		mirror,
-		dss_id,
-		hdr->args.count,
-		task->tk_start);
+	nfs4_ff_layout_stat_io_start(hdr->inode, mirror, dss_id, false,
+				     hdr->args.count, task->tk_start);
 }
 
 static void ff_layout_read_record_layoutstats_done(struct rpc_task *task,
@@ -2002,12 +1979,8 @@ static void ff_layout_write_record_layoutstats_start(struct rpc_task *task,
 		mirror->dss_count,
 		hdr->args.offset);
 
-	nfs4_ff_layout_stat_io_start_write(
-		hdr->inode,
-		mirror,
-		dss_id,
-		hdr->args.count,
-		task->tk_start);
+	nfs4_ff_layout_stat_io_start(hdr->inode, mirror, dss_id, true,
+				     hdr->args.count, task->tk_start);
 }
 
 static void ff_layout_write_record_layoutstats_done(struct rpc_task *task,
@@ -2121,10 +2094,9 @@ static void ff_layout_commit_record_layoutstats_start(struct rpc_task *task,
 
 	idx = calc_mirror_idx_from_commit(cdata->lseg, cdata->ds_commit_index);
 	dss_id = calc_dss_id_from_commit(cdata->lseg, cdata->ds_commit_index);
-	nfs4_ff_layout_stat_io_start_write(cdata->inode,
-			FF_LAYOUT_COMP(cdata->lseg, idx),
-			dss_id,
-			0, task->tk_start);
+	nfs4_ff_layout_stat_io_start(cdata->inode,
+				     FF_LAYOUT_COMP(cdata->lseg, idx),
+				     dss_id, true, 0, task->tk_start);
 }
 
 static void ff_layout_commit_record_layoutstats_done(struct rpc_task *task,
-- 
2.52.0

Re: [PATCH v2 01/12] NFSv4/flexfiles: move layoutstats accounting to a per-stripe lock

[kernel test robot]

Hi Mike,

kernel test robot noticed the following build errors:

[auto build test ERROR on 8cdff0c1549d3fce6bb652e6bc5dd7546e1cd1e3]

url:    https://github.com/intel-lab-lkp/linux/commits/Mike-Snitzer/NFSv4-flexfiles-move-layoutstats-accounting-to-a-per-stripe-lock/20261105-191238
base:   8cdff0c1549d3fce6bb652e6bc5dd7546e1cd1e3
patch link:    https://lore.kernel.org/r/20261106001251.73472-2-snitzer%40kernel.org
patch subject: [PATCH v2 01/12] NFSv4/flexfiles: move layoutstats accounting to a per-stripe lock
config: arm-mps2_defconfig (https://download.01.org/0day-ci/archive/20261106/202611062010.UcNJzsW9-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 812626a41399a1d464a0d69627ab45c83b8ebfb9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261106/202611062010.UcNJzsW9-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611062010.UcNJzsW9-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from fs/nfs/flexfilelayout/flexfilelayout.c:10:
   In file included from include/linux/nfs_fs.h:22:
   In file included from include/linux/in.h:19:
   In file included from include/uapi/linux/in.h:25:
   In file included from include/linux/socket.h:8:
   In file included from include/linux/uio.h:8:
   In file included from include/linux/kernel.h:23:
   In file included from include/linux/bitops.h:67:
   In file included from arch/arm/include/asm/bitops.h:245:
   In file included from include/asm-generic/bitops/lock.h:5:
   In file included from include/linux/atomic.h:7:
   In file included from arch/arm/include/asm/atomic.h:16:
>> arch/arm/include/asm/cmpxchg.h:260:1: error: instruction requires: !armv*m
     260 | "1:     ldrexd          %1, %H1, [%3]\n"
         | ^
   <inline asm>:1:5: note: instantiated into assembly here
       1 |         1:      ldrexd          r2, r3, [r0]
         |                 ^
   In file included from fs/nfs/flexfilelayout/flexfilelayout.c:10:
   In file included from include/linux/nfs_fs.h:22:
   In file included from include/linux/in.h:19:
   In file included from include/uapi/linux/in.h:25:
   In file included from include/linux/socket.h:8:
   In file included from include/linux/uio.h:8:
   In file included from include/linux/kernel.h:23:
   In file included from include/linux/bitops.h:67:
   In file included from arch/arm/include/asm/bitops.h:245:
   In file included from include/asm-generic/bitops/lock.h:5:
   In file included from include/linux/atomic.h:7:
   In file included from arch/arm/include/asm/atomic.h:16:
   arch/arm/include/asm/cmpxchg.h:264:2: error: instruction requires: !armv*m
     264 | "       strexd          %0, %5, %H5, [%3]\n"
         |  ^
   <inline asm>:5:2: note: instantiated into assembly here
       5 |         strexd          r1, r10, r11, [r0]
         |         ^
   2 errors generated.


vim +260 arch/arm/include/asm/cmpxchg.h

e001bbae7147b1 Russell King 2015-05-26  249  
2523c67bb6962f Will Deacon  2013-10-09  250  static inline unsigned long long __cmpxchg64(unsigned long long *ptr,
2523c67bb6962f Will Deacon  2013-10-09  251  					     unsigned long long old,
2523c67bb6962f Will Deacon  2013-10-09  252  					     unsigned long long new)
2523c67bb6962f Will Deacon  2013-10-09  253  {
2523c67bb6962f Will Deacon  2013-10-09  254  	unsigned long long oldval;
2523c67bb6962f Will Deacon  2013-10-09  255  	unsigned long res;
2523c67bb6962f Will Deacon  2013-10-09  256  
c32ffce0f66e5d Will Deacon  2014-02-21  257  	prefetchw(ptr);
c32ffce0f66e5d Will Deacon  2014-02-21  258  
2523c67bb6962f Will Deacon  2013-10-09  259  	__asm__ __volatile__(
2523c67bb6962f Will Deacon  2013-10-09 @260  "1:	ldrexd		%1, %H1, [%3]\n"
2523c67bb6962f Will Deacon  2013-10-09  261  "	teq		%1, %4\n"
2523c67bb6962f Will Deacon  2013-10-09  262  "	teqeq		%H1, %H4\n"
2523c67bb6962f Will Deacon  2013-10-09  263  "	bne		2f\n"
2523c67bb6962f Will Deacon  2013-10-09  264  "	strexd		%0, %5, %H5, [%3]\n"
2523c67bb6962f Will Deacon  2013-10-09  265  "	teq		%0, #0\n"
2523c67bb6962f Will Deacon  2013-10-09  266  "	bne		1b\n"
2523c67bb6962f Will Deacon  2013-10-09  267  "2:"
2523c67bb6962f Will Deacon  2013-10-09  268  	: "=&r" (res), "=&r" (oldval), "+Qo" (*ptr)
2523c67bb6962f Will Deacon  2013-10-09  269  	: "r" (ptr), "r" (old), "r" (new)
2523c67bb6962f Will Deacon  2013-10-09  270  	: "cc");
2523c67bb6962f Will Deacon  2013-10-09  271  
2523c67bb6962f Will Deacon  2013-10-09  272  	return oldval;
2523c67bb6962f Will Deacon  2013-10-09  273  }
2523c67bb6962f Will Deacon  2013-10-09  274  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki