Re: [PATCH v10 6/7] nfs/localio: add tracepoints for misaligned DIO READ and WRITE support

[Mike Snitzer <snitzer@kernel.org>]

On Thu, Sep 18, 2025 at 04:18:49PM -0400, Mike Snitzer wrote:
> On Thu, Sep 18, 2025 at 03:55:32PM -0400, Anna Schumaker wrote:
> > 
> > 
> > On 9/18/25 3:21 PM, Mike Snitzer wrote:
> > > On Thu, Sep 18, 2025 at 01:55:03PM -0400, Anna Schumaker wrote:
> > >>
> > >>
> > >> On 9/18/25 1:46 PM, Mike Snitzer wrote:
> > >>> On Thu, Sep 18, 2025 at 01:33:30PM -0400, Anna Schumaker wrote:
> > >>>> Hi Mike,
> > >>>>
> > >>>> On 9/17/25 2:18 PM, Mike Snitzer wrote:
> > >>>>> Add nfs_local_dio_class and use it to create nfs_local_dio_read,
> > >>>>> nfs_local_dio_write and nfs_local_dio_misaligned trace events.
> > >>>>>
> > >>>>> These trace events show how NFS LOCALIO splits a given misaligned
> > >>>>> IO into a mix of misaligned head and/or tail extents and a DIO-aligned
> > >>>>> middle extent.  The misaligned head and/or tail extents are issued
> > >>>>> using buffered IO and the DIO-aligned middle is issued using O_DIRECT.
> > >>>>>
> > >>>>> This combination of trace events is useful for LOCALIO DIO READs:
> > >>>>>
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_local_dio_read/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_local_dio_misaligned/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_initiate_read/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_readpage_done/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/xfs/xfs_file_direct_read/enable
> > >>>>>
> > >>>>> This combination of trace events is useful for LOCALIO DIO WRITEs:
> > >>>>>
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_local_dio_write/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_local_dio_misaligned/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_initiate_write/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/nfs/nfs_writeback_done/enable
> > >>>>>   echo 1 > /sys/kernel/tracing/events/xfs/xfs_file_direct_write/enable
> > >>>>
> > >>>> I'm having a lot of trouble compiling this patch. I'm seeing errors like this:
> > >>>>
> > >>>>
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>>  1800 | DEFINE_NFS_LOCAL_DIO_EVENT(write);
> > >>>>       | ^
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: declaration of 'struct nfs_local_dio' will not be visible outside of this function [-Werror,-Wvisibility]
> > >>>> fs/nfs/nfstrace.h:1796:17: note: expanded from macro 'DEFINE_NFS_LOCAL_DIO_EVENT'
> > >>>>  1796 |                  const struct nfs_local_dio *local_dio),\
> > >>>>       |                               ^
> > >>>> fs/nfs/nfstrace.h:1800:1: error: incompatible pointer types passing 'const struct nfs_local_dio *' to parameter of type 'const struct nfs_local_dio *' [-Werror,-Wincompatible-pointer-types]
> > >>>>  1800 | DEFINE_NFS_LOCAL_DIO_EVENT(write);
> 
> BTW, this ^ last incompatible pointer type error doesn't make any
> sense.
> 
> I'm concerned this is very specific to your development environment.
> 
> (more reply below)
> 
> > >>>>
> > >>>>
> > >>>> Am I missing a patch somewhere along the line?
> > >>>>
> > >>>> Thanks,
> > >>>> Anna
> > >>>>
> > >>>>>
> > >>>>> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> > >>>>> ---
> > >>>>>  fs/nfs/internal.h | 10 +++++++
> > >>>>>  fs/nfs/localio.c  | 19 ++++++-------
> > >>>>>  fs/nfs/nfs3xdr.c  |  2 +-
> > >>>>>  fs/nfs/nfstrace.h | 70 +++++++++++++++++++++++++++++++++++++++++++++++
> > >>>>>  4 files changed, 89 insertions(+), 12 deletions(-)
> > >>>>>
> > >>>>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> > >>>>> index d44233cfd7949..3d380c45b5ef3 100644
> > >>>>> --- a/fs/nfs/internal.h
> > >>>>> +++ b/fs/nfs/internal.h
> > >>>>> @@ -456,6 +456,16 @@ extern int nfs_wait_bit_killable(struct wait_bit_key *key, int mode);
> > >>>>>  
> > >>>>>  #if IS_ENABLED(CONFIG_NFS_LOCALIO)
> > >>>>>  /* localio.c */
> > >>>>> +struct nfs_local_dio {
> > >>>>> +	u32 mem_align;
> > >>>>> +	u32 offset_align;
> > >>>>> +	loff_t middle_offset;
> > >>>>> +	loff_t end_offset;
> > >>>>> +	ssize_t	start_len;	/* Length for misaligned first extent */
> > >>>>> +	ssize_t	middle_len;	/* Length for DIO-aligned middle extent */
> > >>>>> +	ssize_t	end_len;	/* Length for misaligned last extent */
> > >>>>> +};
> > >>>>> +
> > >>>>>  extern void nfs_local_probe_async(struct nfs_client *);
> > >>>>>  extern void nfs_local_probe_async_work(struct work_struct *);
> > >>>>>  extern struct nfsd_file *nfs_local_open_fh(struct nfs_client *,
> > >>>>> diff --git a/fs/nfs/localio.c b/fs/nfs/localio.c
> > >>>>> index 768af570183af..cf1533759646e 100644
> > >>>>> --- a/fs/nfs/localio.c
> > >>>>> +++ b/fs/nfs/localio.c
> > >>>>> @@ -322,16 +322,6 @@ nfs_local_iocb_alloc(struct nfs_pgio_header *hdr,
> > >>>>>  	return iocb;
> > >>>>>  }
> > >>>>>  
> > >>>>> -struct nfs_local_dio {
> > >>>>> -	u32 mem_align;
> > >>>>> -	u32 offset_align;
> > >>>>> -	loff_t middle_offset;
> > >>>>> -	loff_t end_offset;
> > >>>>> -	ssize_t	start_len;	/* Length for misaligned first extent */
> > >>>>> -	ssize_t	middle_len;	/* Length for DIO-aligned middle extent */
> > >>>>> -	ssize_t	end_len;	/* Length for misaligned last extent */
> > >>>>> -};
> > >>>>> -
> > >>>>>  static bool
> > >>>>>  nfs_is_local_dio_possible(struct nfs_local_kiocb *iocb, int rw,
> > >>>>>  			  size_t len, struct nfs_local_dio *local_dio)
> > >>>>> @@ -367,6 +357,10 @@ nfs_is_local_dio_possible(struct nfs_local_kiocb *iocb, int rw,
> > >>>>>  	local_dio->middle_len = middle_end - start_end;
> > >>>>>  	local_dio->end_len = orig_end - middle_end;
> > >>>>>  
> > >>>>> +	if (rw == ITER_DEST)
> > >>>>> +		trace_nfs_local_dio_read(hdr->inode, offset, len, local_dio);
> > >>>>> +	else
> > >>>>> +		trace_nfs_local_dio_write(hdr->inode, offset, len, local_dio);
> > >>>>>  	return true;
> > >>>>>  }
> > >>>>>  
> > >>>>> @@ -446,8 +440,11 @@ nfs_local_iters_setup_dio(struct nfs_local_kiocb *iocb, int rw,
> > >>>>>  		nfs_iov_iter_aligned_bvec(&iters[n_iters],
> > >>>>>  			local_dio->mem_align-1, local_dio->offset_align-1);
> > >>>>>  
> > >>>>> -	if (unlikely(!iocb->iter_is_dio_aligned[n_iters]))
> > >>>>> +	if (unlikely(!iocb->iter_is_dio_aligned[n_iters])) {
> > >>>>> +		trace_nfs_local_dio_misaligned(iocb->hdr->inode,
> > >>>>> +			iocb->hdr->args.offset, len, local_dio);
> > >>>>>  		return 0; /* no DIO-aligned IO possible */
> > >>>>> +	}
> > >>>>>  	++n_iters;
> > >>>>>  
> > >>>>>  	iocb->n_iters = n_iters;
> > >>>>> diff --git a/fs/nfs/nfs3xdr.c b/fs/nfs/nfs3xdr.c
> > >>>>> index 4ae01c10b7e28..e17d729084125 100644
> > >>>>> --- a/fs/nfs/nfs3xdr.c
> > >>>>> +++ b/fs/nfs/nfs3xdr.c
> > >>>>> @@ -23,8 +23,8 @@
> > >>>>>  #include <linux/nfsacl.h>
> > >>>>>  #include <linux/nfs_common.h>
> > >>>>>  
> > >>>>> -#include "nfstrace.h"
> > >>>>>  #include "internal.h"
> > >>>>> +#include "nfstrace.h"
> > >>>>>  
> > >>>>>  #define NFSDBG_FACILITY		NFSDBG_XDR
> > >>>>>  
> > >>>
> > >>> This change ^ was critical for fixing unknown type issues for 'struct
> > >>> nfs_local_dio' issues on my build. But I haven't seen the issue you've
> > >>> reported above. I'll try applying my changes on very latest upstream
> > >>> tree.
> > >>>
> > >>> Which tree/branch are you using for your baseline?  Also, which
> > >>> version of gcc (which distro even) are you using?
> > >>
> > >> I'm using my linux-next branch from: git.linux-nfs.org/projects/anna/linux-nfs.git.
> > >> It's v6.17-rc6 plus the patches I'm planning for the next merge window.
> > > 
> > > Like I mentioned in another reply in this thread, I've pushed a tree
> > > that is based on your linux-next branch here:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/snitzer/linux.git/log/?h=anna-linux-next-6.18
> > > 
> > > I've verified that this builds fine for me when using either:
> > > EL8.10 with gcc 11.2.1-9
> > > EL9.5 with 11.5.0-5
> > > 
> > > Which distro and gcc version are you using?
> > 
> > This is with Archlinux and clang v 20.1.8, although I do
> > see similar errors with gcc 15.2.1.
> 
> OK, so way more bleeding edge than my 2 versions of gcc 11.x
> 
> I tried to see if disabling CONFIG_NFS_LOCALIO might explain it, but
> that compiles fine for me too.
> 
> Please feel free to share your .config with me off-list and I'll keep
> chasing.
> 

FYI, I'm not seeing any compiler issues with gcc 15.1.1-2 (I updated
my EL9 system to use gcc-toolset-15-15.0-9.el9.x86_64).

Please feel free to drop this nfstrace patch until we're able to
figure out what's happening on your build environment.

Thanks,
Mike