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

[Mike Snitzer <snitzer@kernel.org>]

From: Mike Snitzer <snitzer@hammerspace.com>

start_time was written under the stripe lock in
nfs4_ff_layoutstat_start_io() and read outside it in
ff_layout_encode_ff_layoutupdate().  Pre-existing -- mirror->lock had the
same shape -- and benign, since it is a single naturally aligned 8 byte
value, but it is an unannotated race either way.

The lock is the wrong tool for it.  ffl_duration is measured from this
stripe's first I/O, so the field is written once and read-only for the
rest of the stripe's life.  Say that instead: publish it with
try_cmpxchg64() and let the first I/O win, and read it with READ_ONCE().
Only the first I/O to a stripe pays the read-modify-write; every one
after it short circuits on the READ_ONCE().

ffl->last_report_time two lines away is already published this way, so
the pattern is not new to this file.

This also stops start_time depending on there being exactly one lock per
stripe, which a later patch changes.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 7fc1369df8439..7c328f2ec900d 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -729,6 +729,22 @@ nfs4_ff_end_busy_timer(struct nfs4_ff_busy_timer *timer, ktime_t now)
 	return ktime_sub(now, start);
 }
 
+/*
+ * ffl_duration is measured from this stripe's first I/O, so start_time is
+ * written once and read-only for the rest of the stripe's life.  Publish it
+ * with a cmpxchg and let the first I/O win, rather than depending on a lock
+ * that every reader would then have to take.
+ */
+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 +756,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)
@@ -3087,7 +3102,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 f48860450cf28..658940979883a 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -88,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.
+	 * Protects read_stat and write_stat below.
 	 *
 	 * Cacheline aligned so that the layoutstats accounting done on
 	 * every I/O to this stripe does not share cachelines with the
@@ -100,6 +100,7 @@ struct nfs4_ff_layout_ds_stripe {
 	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