On Tue, Aug 05, 2025 at 10:55:06AM -0400, Chuck Lever wrote: > On 7/31/25 7:06 PM, Mike Snitzer wrote: > > If NFSD_IO_DIRECT is used, split any misaligned WRITE into a start, > > middle and end as needed. The large middle extent is DIO-aligned and > > the start and/or end are misaligned. Buffered IO is used for the > > misaligned extents and O_DIRECT is used for the middle DIO-aligned > > extent. > > > > The nfsd_analyze_write_dio trace event shows how NFSD splits a given > > misaligned WRITE into a mix of misaligned extent(s) and a DIO-aligned > > extent. > > > > This combination of trace events is useful: > > > > echo 1 > /sys/kernel/tracing/events/nfsd/nfsd_write_opened/enable > > echo 1 > /sys/kernel/tracing/events/nfsd/nfsd_analyze_write_dio/enable > > echo 1 > /sys/kernel/tracing/events/nfsd/nfsd_write_io_done/enable > > echo 1 > /sys/kernel/tracing/events/xfs/xfs_file_direct_write/enable > > > > Which for this dd command: > > > > dd if=/dev/zero of=/mnt/share1/test bs=47008 count=2 oflag=direct > > > > Results in: > > > > nfsd-55714 [043] ..... 79976.260851: nfsd_write_opened: xid=0x966c5d2d fh_hash=0x4d34e6c1 offset=0 len=47008 > > nfsd-55714 [043] ..... 79976.260852: nfsd_analyze_write_dio: xid=0x966c5d2d fh_hash=0x4d34e6c1 offset=0 len=47008 start=0+0 middle=0+45056 end=45056+1952 > > nfsd-55714 [043] ..... 79976.260857: xfs_file_direct_write: dev 259:12 ino 0x3e00008f disize 0x0 pos 0x0 bytecount 0xb000 > > nfsd-55714 [043] ..... 79976.260965: nfsd_write_io_done: xid=0x966c5d2d fh_hash=0x4d34e6c1 offset=0 len=47008 > > > > nfsd-55714 [043] ..... 79976.307762: nfsd_write_opened: xid=0x67e5ce6f fh_hash=0x4d34e6c1 offset=47008 len=47008 > > nfsd-55714 [043] ..... 79976.307762: nfsd_analyze_write_dio: xid=0x67e5ce6f fh_hash=0x4d34e6c1 offset=47008 len=47008 start=47008+2144 middle=49152+40960 end=90112+3904 > > nfsd-55714 [043] ..... 79976.307797: xfs_file_direct_write: dev 259:12 ino 0x3e00008f disize 0xc000 pos 0xc000 bytecount 0xa000 > > nfsd-55714 [043] ..... 79976.307866: nfsd_write_io_done: xid=0x67e5ce6f fh_hash=0x4d34e6c1 offset=47008 len=47008 > > > > Signed-off-by: Mike Snitzer <snitzer@xxxxxxxxxx> > > Reviewed-by: Jeff Layton <jlayton@xxxxxxxxxx> > > --- > > fs/nfsd/vfs.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 124 insertions(+), 11 deletions(-) > > The diffstat tells a worrying story. > > For the READ path, nfsd_iter_read() is the fallback -- the hot path > right now is splice reads. So adding instruction path length in > nfsd_iter_read() is a concern, but it's not an immediate problem. > > For the write side, there is only one path, which is ballooning with > this set of patches. This is bound to have an impact on page cache > writes, which are still the default write I/O mode. > > This is why I'm hesitant to apply this series as it is currently. > > If I might suggest a possible action/recourse: Perhaps the patches can > be restructured so that the default cached write mechanism retains a > short IPL. We can leave the de-duplication and other optimizations until > we have field results from our experiments. The default case has an extra memset() and some extra NULL pointer checks, but otherwise I don't see anything that is reason for concern. Can you be more specific? Have you quantified something in practice or this is purely from looking at the diffstat? Avoiding needless function calls (that should/could be inlined due to only one caller) is your concern? Or is wanting to somehow avoid any of this misaligned DIO support until/unless the user enabled NFSD_IO_DIRECT via debugfs? I've (ab)used jump_labels in the past to micro-optimize away code that should only be active if opt-in occurs. But it ends up being pretty gross, and I'm left unsure that jump_labels are sufficiently light-weight to be worth it. I've just posted v4... we can continue this discussion wherever you'd like. Thanks, Mike
