Linux NFS development
 help / color / mirror / Atom feed
From: Anna Schumaker <anna.schumaker@oracle.com>
To: Mike Snitzer <snitzer@kernel.org>, Anna Schumaker <anna@kernel.org>
Cc: Trond Myklebust <trond.myklebust@hammerspace.com>,
	linux-nfs@vger.kernel.org
Subject: Re: [PATCH v10 6/7] nfs/localio: add tracepoints for misaligned DIO READ and WRITE support
Date: Thu, 18 Sep 2025 13:33:30 -0400	[thread overview]
Message-ID: <23118649-3dc6-443b-beb7-9360199e93e3@oracle.com> (raw)
In-Reply-To: <20250917181843.33865-7-snitzer@kernel.org>

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);


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
>  
> diff --git a/fs/nfs/nfstrace.h b/fs/nfs/nfstrace.h
> index d5949da8c2e5d..132c1b87fa3eb 100644
> --- a/fs/nfs/nfstrace.h
> +++ b/fs/nfs/nfstrace.h
> @@ -1599,6 +1599,76 @@ DEFINE_NFS_DIRECT_REQ_EVENT(nfs_direct_write_completion);
>  DEFINE_NFS_DIRECT_REQ_EVENT(nfs_direct_write_schedule_iovec);
>  DEFINE_NFS_DIRECT_REQ_EVENT(nfs_direct_write_reschedule_io);
>  
> +#if IS_ENABLED(CONFIG_NFS_LOCALIO)
> +
> +DECLARE_EVENT_CLASS(nfs_local_dio_class,
> +	TP_PROTO(
> +		const struct inode *inode,
> +		loff_t offset,
> +		ssize_t count,
> +		const struct nfs_local_dio *local_dio
> +	),
> +	TP_ARGS(inode, offset, count, local_dio),
> +	TP_STRUCT__entry(
> +		__field(dev_t, dev)
> +		__field(u64, fileid)
> +		__field(u32, fhandle)
> +		__field(loff_t, offset)
> +		__field(ssize_t, count)
> +		__field(u32, mem_align)
> +		__field(u32, offset_align)
> +		__field(loff_t, start)
> +		__field(ssize_t, start_len)
> +		__field(loff_t, middle)
> +		__field(ssize_t, middle_len)
> +		__field(loff_t, end)
> +		__field(ssize_t, end_len)
> +	),
> +	TP_fast_assign(
> +		const struct nfs_inode *nfsi = NFS_I(inode);
> +		const struct nfs_fh *fh = &nfsi->fh;
> +
> +		__entry->dev = inode->i_sb->s_dev;
> +		__entry->fileid = nfsi->fileid;
> +		__entry->fhandle = nfs_fhandle_hash(fh);
> +		__entry->offset = offset;
> +		__entry->count = count;
> +		__entry->mem_align = local_dio->mem_align;
> +		__entry->offset_align = local_dio->offset_align;
> +		__entry->start = offset;
> +		__entry->start_len = local_dio->start_len;
> +		__entry->middle = local_dio->middle_offset;
> +		__entry->middle_len = local_dio->middle_len;
> +		__entry->end = local_dio->end_offset;
> +		__entry->end_len = local_dio->end_len;
> +	),
> +	TP_printk("fileid=%02x:%02x:%llu fhandle=0x%08x "
> +		  "offset=%lld count=%zd "
> +		  "mem_align=%u offset_align=%u "
> +		  "start=%llu+%zd middle=%llu+%zd end=%llu+%zd",
> +		  MAJOR(__entry->dev), MINOR(__entry->dev),
> +		  (unsigned long long)__entry->fileid,
> +		  __entry->fhandle, __entry->offset, __entry->count,
> +		  __entry->mem_align, __entry->offset_align,
> +		  __entry->start, __entry->start_len,
> +		  __entry->middle, __entry->middle_len,
> +		  __entry->end, __entry->end_len)
> +)
> +
> +#define DEFINE_NFS_LOCAL_DIO_EVENT(name)		\
> +DEFINE_EVENT(nfs_local_dio_class, nfs_local_dio_##name,	\
> +	TP_PROTO(const struct inode *inode,		\
> +		 loff_t offset,				\
> +		 ssize_t count,				\
> +		 const struct nfs_local_dio *local_dio),\
> +	TP_ARGS(inode, offset, count, local_dio))
> +
> +DEFINE_NFS_LOCAL_DIO_EVENT(read);
> +DEFINE_NFS_LOCAL_DIO_EVENT(write);
> +DEFINE_NFS_LOCAL_DIO_EVENT(misaligned);
> +
> +#endif /* CONFIG_NFS_LOCALIO */
> +
>  TRACE_EVENT(nfs_fh_to_dentry,
>  		TP_PROTO(
>  			const struct super_block *sb,


  reply	other threads:[~2025-09-18 17:33 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-17 18:18 [PATCH v10 0/7] NFS Direct: align misaligned DIO for LOCALIO Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 1/7] nfs/localio: make trace_nfs_local_open_fh more useful Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 2/7] nfs/localio: avoid issuing misaligned IO using O_DIRECT Mike Snitzer
2025-09-18 17:15   ` Anna Schumaker
2025-09-18 17:31     ` Mike Snitzer
2025-09-18 18:55     ` Chuck Lever
2025-09-18 19:17       ` Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 3/7] nfs/localio: refactor iocb and iov_iter_bvec initialization Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 4/7] nfs/localio: refactor iocb initialization Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 5/7] nfs/localio: add proper O_DIRECT support for READ and WRITE Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 6/7] nfs/localio: add tracepoints for misaligned DIO READ and WRITE support Mike Snitzer
2025-09-18 17:33   ` Anna Schumaker [this message]
2025-09-18 17:46     ` Mike Snitzer
2025-09-18 17:55       ` Anna Schumaker
2025-09-18 19:21         ` Mike Snitzer
2025-09-18 19:55           ` Anna Schumaker
2025-09-18 20:18             ` Mike Snitzer
2025-09-18 21:03               ` Mike Snitzer
2025-09-18 21:06                 ` Anna Schumaker
2025-09-18 21:07                 ` Anna Schumaker
2025-09-18 21:41                   ` Mike Snitzer
2025-09-17 18:18 ` [PATCH v10 7/7] NFS: add basic STATX_DIOALIGN and STATX_DIO_READ_ALIGN support Mike Snitzer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23118649-3dc6-443b-beb7-9360199e93e3@oracle.com \
    --to=anna.schumaker@oracle.com \
    --cc=anna@kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=snitzer@kernel.org \
    --cc=trond.myklebust@hammerspace.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox