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

[Mike Snitzer <snitzer@kernel.org>]

From: Mike Snitzer <snitzer@hammerspace.com>

Moving the layoutstats lock into struct nfs4_ff_layout_ds_stripe stopped
stripes serialising on one spinlock, but it left them sharing cachelines
with each other.

The lock and the statistics it guards sat at the tail of a struct whose
sizeof() was 296 and whose alignof() was only 8, so in mirror->dss[]
stripe i began at i * 296.  296 mod 64 = 40, so every stripe started 40
bytes further into a line than the last and no stripe's lock was line
aligned.  Acquiring stripe i's lock therefore invalidated the line
holding stripe i+1's lock and counters.  A RAID0-style layout -- one
mirror, many stripes -- is exactly the case the per-stripe lock was
introduced for, so this false sharing ate into the win.

Align the first member of the guarded set.  That does two things: the
lock now starts on a line boundary, so the guarded set occupies lines of
its own and shares none with the read-mostly members above; and it raises
the struct's alignment to 64, which forces sizeof() up to a multiple of
64 and makes the array stride line aligned.  The stride is what actually
separates stripe i from stripe i+1.

Verified with pahole on flexfilelayout.o built with DWARF5.  The layout
is identical on x86_64, arm64 4K pages and arm64 64K pages -- cachelines
are 64 bytes on both architectures, so page size does not enter into it
-- and was taken with neither CONFIG_DEBUG_SPINLOCK nor
CONFIG_PROVE_LOCKING, both of which fatten spinlock_t and change it:

  /* --- cacheline 3 boundary (192 bytes) --- */
  spinlock_t                 lock __aligned(64);  /*   192     4 */
  /* XXX 4 bytes hole */
  struct nfs4_ff_layoutstat  read_stat;           /*   200    72 */
  struct nfs4_ff_layoutstat  write_stat;          /*   272    72 */
  ktime_t                    start_time;          /*   344     8 */
  /* size: 384, cachelines: 6 */
  } __attribute__((__aligned__(64)));

A read RPC now dirties two lines of its own stripe and a write three, and
neither disturbs a neighbouring stripe.  Later patches give each
direction a line of its own.

Cost is 296 -> 384 bytes per stripe, i.e. 2816 bytes for a 32 stripe
layout, which is negligible beside the rest of the per-file layout state.
384 is also a multiple of 128, so the stride holds for implementations
with a 128 byte coherency granule.

Note that the alignment attribute only orders the stripes relative to the
start of mirror->dss[], not that array's own base.  The base is aligned
too, under either allocator kvmalloc() may pick for it:

  - From the slab.  kmalloc() documents a size based alignment guarantee
    for power-of-two sizes only, but the smallest possible request is now
    384 bytes, and every kmalloc bucket at or above that is a power of
    two whose objects SLUB lays out at multiples of the object size from
    a page aligned slab, so the base is at least 512 byte aligned.  The
    exceptions, kmalloc-96 and kmalloc-192, are both smaller than one
    stripe and cannot be selected.

  - From vmalloc.  A stripe array wide enough to need a high order
    allocation is served by the vmalloc() fallback instead, which returns
    page aligned memory.

Either is comfortably more than the 64 bytes this needs.

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

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 8fa2d8851166a..f48860450cf28 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"
 
@@ -86,8 +87,17 @@ 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;
+	/*
+	 * Protects read_stat, write_stat and start_time below.
+	 *
+	 * Cacheline aligned so that the layoutstats accounting done on
+	 * every I/O to this stripe does not share cachelines with the
+	 * read-mostly members above, nor with the neighbouring stripes
+	 * in mirror->dss[]: aligning the first member of the guarded
+	 * set also rounds sizeof() up to a multiple of the cacheline,
+	 * which is what makes the array stride line-aligned.
+	 */
+	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