RE: [PATCH v4] svcrdma: Use contiguous pages for RDMA Read sink buffers

> -----Original Message-----
> From: Mike Snitzer <[email protected]>
> Sent: Thursday, June 4, 2026 2:51 PM
> To: Chuck Lever <[email protected]>; Jonathan Flynn
> <jonathan.flynn-F/[email protected]>
> Cc: Leon Romanovsky <[email protected]>; Christoph Hellwig <[email protected]>;
> NeilBrown <[email protected]>; Jeff Layton <[email protected]>; Olga
> Kornievskaia <[email protected]>; Dai Ngo <[email protected]>; Tom
> Talpey <[email protected]>; [email protected]; linux-
> [email protected]; Chuck Lever <[email protected]>
> Subject: Re: [PATCH v4] svcrdma: Use contiguous pages for RDMA Read sink
> buffers
>
> On Thu, Mar 19, 2026 at 09:36:10AM -0400, Chuck Lever wrote:
> > From: Chuck Lever <[email protected]>
> >
> > svc_rdma_build_read_segment() constructs RDMA Read sink buffers by
> > consuming pages one-at-a-time from rq_pages[] and building one bvec
> > per page. A 64KB NFS READ payload produces 16 separate bvecs, 16 DMA
> > mappings, and potentially multiple RDMA Read WRs (on platforms with
> > 4KB pages).
> >
> > A single higher-order allocation followed by split_page() yields
> > physically contiguous memory while preserving per-page refcounts. A
> > single bvec spanning the contiguous range causes
> > rdma_rw_ctx_init_bvec() to take the
> > rdma_rw_init_single_wr_bvec() fast path: one DMA mapping, one SGE, one
> > WR.
> >
> > The split sub-pages replace the original rq_pages[] entries, so all
> > downstream page tracking, completion handling, and xdr_buf assembly
> > remain unchanged.
> >
> > Allocation uses __GFP_NORETRY | __GFP_NOWARN and falls back through
> > decreasing orders. If even order-1 fails, the existing per-page path
> > handles the segment.
> >
> > When nr_pages is not a power of two, get_order() rounds up and the
> > allocation yields more pages than needed. The extra split pages
> > replace existing rq_pages[] entries (freed via
> > put_page() first), so there is no net increase in per- request page
> > consumption. Successive segments reuse the same padding slots,
> > preventing accumulation. The rq_maxpages guard rejects any allocation
> > that would overrun the array, falling back to the per-page path.
> > Under memory pressure, __GFP_NORETRY causes the higher- order
> > allocation to fail without stalling.
> >
> > The contiguous path is attempted when the segment starts page-aligned
> > (rc_pageoff == 0) and spans at least two pages. NFS WRITE segments
> > carry application-modified byte ranges of arbitrary length, so the
> > optimization is not restricted to power-of-two page counts.
> >
> > Signed-off-by: Chuck Lever <[email protected]>
> > ---
> > Changes since v3:
> > - Drop 1/3 - 3/3, they have already been reviewed and queued
> > - Incorporate hch's review comments
> > - Remove the #ifdef SZ_64K -- the logic works on those systems too
> >
> >
> >  net/sunrpc/xprtrdma/svc_rdma_rw.c | 213
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 213 insertions(+)
>
> This patch, which landed during the 7.1 merge window as commit
> 18755b8c2f241 ("svcrdma: Use contiguous pages for RDMA Read sink
> buffers"), severely hurts RDMA performance when testing on very fast
RDMA
> networking on x86_64.
>
> With this commit WRITE performance is "only" 24.2GB/s.
> Without this commit WRITE performance is 60.6GB/s.
>
> The cpu burn due to spinlock dominates the flamegraph that was
collected.
> Chuck, I'll send you the flamegraph off-list (and can send it to anyone
else who
> might be interested).  Jon Flynn did the testing and we can request more
info
> from him.
>
> We may have a window _now_ on the current Hammerspace testbed to try an
> incremental fix, but as of now simply reverting commit
> 18755b8c2f241 is as far as we got.
>
> Thanks,
> Mike

Chuck, All,

Here is link to a bundle containing the test environment documentation,
benchmark results, system characterization, and perf data collected while
investigating the performance regression associated with commit
18755b8c2f241 ("svcrdma: Use contiguous pages for RDMA Read sink
buffers").
https://1drv.ms/f/c/522bdd4fbd5c042b/IgBszn47Y5ShQZzEqo9yQQfVAdlNkI2XCJBUK
NA23ikzE2c?e=fqUquj

Summary
Testing was performed using a single NFS/RDMA client and server connected
via dual 400 Gb/s ConnectX-7 adapters. The server exports four XFS
filesystems backed by four independent 8-drive NVMe RAID0 arrays. In the
phase1 steady-state run, the module containing commit 18755b8c2f241
achieved 30.3 GiB/s (32.5 GB/s) of write throughput versus 73.9 GiB/s
(79.4 GB/s) with the commit reverted. Average server system CPU
utilization increased from 8.54% with the commit reverted to 76.35% with
the commit present. Standard perf profiling consistently showed
significant CPU time spent in the page allocation path associated with the
contiguous buffer allocation logic introduced by this change.

The best starting point is:
* 'test_environment_configuration.docx'

That document summarizes the server, client, networking, storage,
filesystem, NFS, and workload configuration.

**Bundle Download**
The bundle layout is:
- 'system/'
  * Server hardware/software characterization
  * CPU, memory, PCI topology, NICs, RDMA, NVMe, mdadm, XFS, and NFS
configuration

- 'client-system/'
  * Client hardware/software characterization
  * NICs, RDMA, NFS mount configuration, and per-mount mountstats

- 'regressed/'
  * Data collected with commit 18755b8c2f241 present

- 'reverted/'
  * Data collected with commit 18755b8c2f241 reverted

Each of 'regressed/' and 'reverted/' contains:
- 'phase1/'
  * fio output
  * Server CPU utilization captures:
    * 'mpstat.txt'
    * 'sar-cpu.txt'
    * 'vmstat.txt'
    * 'top-threads.txt'

- 'phase2/'
  * Standard perf profile:
    * 'perf.data'
    * 'perf-report.txt'
    * 'perf-report-children.txt'
    * 'perf-mm-lock-summary.txt'

- 'phase3/'
  * Raw lock contention tracepoint collection:
    * 'perf-contention.data'
    * 'perf-contention-report.txt'
    * 'perf-contention-script.txt'

Regarding collection methods:
- Standard 'perf record' profiling was collected successfully and produced
usable call graphs and allocator-focused summaries.
- 'perf lock' was attempted, but it did not produce useful contention
information on this kernel/configuration.
- 'lock_stat' could not be used because 'CONFIG_LOCK_STAT' is not enabled
in this kernel.
- As an alternative, raw lock contention tracepoints were collected using
'lock:contention_begin' and 'lock:contention_end'. These traces are
substantially larger than the standard perf captures due to the volume of
contention events observed during the regressed test runs and should be
considered supplemental to the standard perf profiles.

The primary comparison is between the regressed module containing commit
18755b8c2f241 and a locally rebuilt module with that commit reverted while
keeping the remainder of the kernel unchanged.

Please let me know if there are any additional traces, instrumentation, or
workload variations that would be useful.

Thanks,

Jon Flynn