Linux NFS development
 help / color / mirror / Atom feed
From: Anna Schumaker <anna.schumaker@oracle.com>
To: Mike Snitzer <snitzer@kernel.org>
Cc: Anna Schumaker <anna@kernel.org>,
	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 15:55:32 -0400	[thread overview]
Message-ID: <34a15201-e8bd-4269-9f18-e74394c63dba@oracle.com> (raw)
In-Reply-To: <aMxbtqugI2dhwsF6@kernel.org>



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

Anna

> 
> Thanks,
> Mike


  reply	other threads:[~2025-09-18 19:55 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
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 [this message]
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=34a15201-e8bd-4269-9f18-e74394c63dba@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