Linux EXT4 FS development
 help / color / mirror / Atom feed
* Re: [f2fs-dev] [PATCHv2 0/5] direct-io file extended attributes
       [not found] <20260710210646.3576365-1-kbusch@meta.com>
@ 2026-07-10 21:53 ` Eric Biggers
  2026-07-10 22:58   ` Keith Busch
       [not found] ` <20260710210646.3576365-2-kbusch@meta.com>
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2026-07-10 21:53 UTC (permalink / raw)
  To: Keith Busch
  Cc: linux-block, linux-ext4, linux-f2fs-devel, linux-fsdevel,
	linux-xfs, axboe, brauner, aalbersh, jack, tytso, Keith Busch,
	jaegeuk, cem

On Fri, Jul 10, 2026 at 02:06:41PM -0700, Keith Busch via Linux-f2fs-devel wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> The attributes reported through statx are incomplete for applications to
> fully know exactly how IO construction is valid or not. The statx call
> can report minimum memory alignment and total granularity, but it
> doesn't show the underlying gap boundary requirements or max segments
> per granule.
> 
> This series adds the minimum to the extended file attributes through
> file_getattr. I hear this is the preferred interface for reporting such
> things over adding more fields to statx. In order to get everything
> under a single syscall, some of the attributes are duplicated from
> statx.

Okay, in v2 we at least now know that the existing statx UAPI was
considered.  Could you give a specific real-world example (with the
actual values of each parameter) where it's not sufficient?  Without
that there isn't really any way to evaluate this proposal.

- Eric

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [f2fs-dev] [PATCHv2 0/5] direct-io file extended attributes
  2026-07-10 21:53 ` [f2fs-dev] [PATCHv2 0/5] direct-io file extended attributes Eric Biggers
@ 2026-07-10 22:58   ` Keith Busch
  2026-07-11  0:24     ` Eric Biggers
  0 siblings, 1 reply; 7+ messages in thread
From: Keith Busch @ 2026-07-10 22:58 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Keith Busch, linux-block, linux-ext4, linux-f2fs-devel,
	linux-fsdevel, linux-xfs, axboe, brauner, aalbersh, jack, tytso,
	jaegeuk, cem

On Fri, Jul 10, 2026 at 05:53:28PM -0400, Eric Biggers wrote:
> On Fri, Jul 10, 2026 at 02:06:41PM -0700, Keith Busch via Linux-f2fs-devel wrote:
> > From: Keith Busch <kbusch@kernel.org>
> > 
> > The attributes reported through statx are incomplete for applications to
> > fully know exactly how IO construction is valid or not. The statx call
> > can report minimum memory alignment and total granularity, but it
> > doesn't show the underlying gap boundary requirements or max segments
> > per granule.
> > 
> > This series adds the minimum to the extended file attributes through
> > file_getattr. I hear this is the preferred interface for reporting such
> > things over adding more fields to statx. In order to get everything
> > under a single syscall, some of the attributes are duplicated from
> > statx.
> 
> Okay, in v2 we at least now know that the existing statx UAPI was
> considered.  Could you give a specific real-world example (with the
> actual values of each parameter) where it's not sufficient?  Without
> that there isn't really any way to evaluate this proposal.

Yes, we can consider nvme. This protocol supports two different transfer
modes called PRP and SGL. PRP requires 4k aligned segments, though you
can have an arbitrary 4-byte aligned offset at the start. SGL on the
other hand allows completely arbitrary size and alignments for each
segment.

statx reports information sufficient to know that you can have dword
aligned page offsets for a virtually contiguous buffer, but it doesn't
report PRP's boundary gap requirement, so applications can't tell if the
file follows PRP or SGL rules for direct-io.

And if you have a device using SGL, statx doesn't report the max number
of sub-sector segments you can submit in a single command.

This series provides both limits so user space has the complete picture.

A typical nvme that supports only PRP has a DMA alignment of 4 bytes, a
dio offset alignment of 4k, and a virtual boundary of 4k.

If SGL were supported, there would be no virtual boundary gap, and max
segments is 256.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 0/5] direct-io file extended attributes
  2026-07-10 22:58   ` Keith Busch
@ 2026-07-11  0:24     ` Eric Biggers
  2026-07-11  1:06       ` Keith Busch
  0 siblings, 1 reply; 7+ messages in thread
From: Eric Biggers @ 2026-07-11  0:24 UTC (permalink / raw)
  To: Keith Busch
  Cc: Keith Busch, linux-block, linux-ext4, linux-f2fs-devel,
	linux-fsdevel, linux-xfs, axboe, brauner, aalbersh, jack, tytso,
	jaegeuk, cem

On Fri, Jul 10, 2026 at 04:58:12PM -0600, Keith Busch wrote:
> On Fri, Jul 10, 2026 at 05:53:28PM -0400, Eric Biggers wrote:
> > On Fri, Jul 10, 2026 at 02:06:41PM -0700, Keith Busch via Linux-f2fs-devel wrote:
> > > From: Keith Busch <kbusch@kernel.org>
> > > 
> > > The attributes reported through statx are incomplete for applications to
> > > fully know exactly how IO construction is valid or not. The statx call
> > > can report minimum memory alignment and total granularity, but it
> > > doesn't show the underlying gap boundary requirements or max segments
> > > per granule.
> > > 
> > > This series adds the minimum to the extended file attributes through
> > > file_getattr. I hear this is the preferred interface for reporting such
> > > things over adding more fields to statx. In order to get everything
> > > under a single syscall, some of the attributes are duplicated from
> > > statx.
> > 
> > Okay, in v2 we at least now know that the existing statx UAPI was
> > considered.  Could you give a specific real-world example (with the
> > actual values of each parameter) where it's not sufficient?  Without
> > that there isn't really any way to evaluate this proposal.
> 
> Yes, we can consider nvme. This protocol supports two different transfer
> modes called PRP and SGL. PRP requires 4k aligned segments, though you
> can have an arbitrary 4-byte aligned offset at the start. SGL on the
> other hand allows completely arbitrary size and alignments for each
> segment.
> 
> statx reports information sufficient to know that you can have dword
> aligned page offsets for a virtually contiguous buffer, but it doesn't
> report PRP's boundary gap requirement, so applications can't tell if the
> file follows PRP or SGL rules for direct-io.
> 
> And if you have a device using SGL, statx doesn't report the max number
> of sub-sector segments you can submit in a single command.
> 
> This series provides both limits so user space has the complete picture.
> 
> A typical nvme that supports only PRP has a DMA alignment of 4 bytes, a
> dio offset alignment of 4k, and a virtual boundary of 4k.

So each segment's length has to be a multiple of 4k, *and* it has to end
on a 4k aligned memory address?  That implies the segment begins at a 4k
aligned memory address as well, which is just stx_dio_mem_align=4k.

What am I missing?

What is a specific example of an I/O request that you'd like to be able
to submit that the existing UAPI can't declare support for?

> If SGL were supported, there would be no virtual boundary gap, and max
> segments is 256.

Can you elaborate on why DIO users need to know max_segments?

I'm worried about the UAPI duplication, as well as it going to be very
difficult for userspace to correctly use this information.  With just
the two alignments there's at least a chance of them getting it right.
If we throw virt_boundary_mask and max_segments into the mix, I don't
think there's much chance.

- Eric

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 0/5] direct-io file extended attributes
  2026-07-11  0:24     ` Eric Biggers
@ 2026-07-11  1:06       ` Keith Busch
  0 siblings, 0 replies; 7+ messages in thread
From: Keith Busch @ 2026-07-11  1:06 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Keith Busch, linux-block, linux-ext4, linux-f2fs-devel,
	linux-fsdevel, linux-xfs, axboe, brauner, aalbersh, jack, tytso,
	jaegeuk, cem

On Fri, Jul 10, 2026 at 08:24:12PM -0400, Eric Biggers wrote:
> On Fri, Jul 10, 2026 at 04:58:12PM -0600, Keith Busch wrote:
> > dio offset alignment of 4k, and a virtual boundary of 4k.
> 
> So each segment's length has to be a multiple of 4k, *and* it has to end
> on a 4k aligned memory address?  That implies the segment begins at a 4k
> aligned memory address as well, which is just stx_dio_mem_align=4k.
> 
> What am I missing?

I'm apparently poorly explaining PRPs, but I hear this is a common
experience.

Simply put, any virtually contiguous buffer that starts at a dword
aligned address is a valid io vector, no matter how many pages it spans.
It doesn't matter where it starts or where it ends, but every page in
the middle obviously starts and ends on their page boundary.

Simple case: pread/pwrite. You can consult statx to know you can provide
any dword aligned buffer with an aligned length, and that's a valid
direct IO.

What I'm trying to enable here is the vectored preadv/pwritev type paths
for hardware that don't need to subscribe to PRP constraints.
 
> What is a specific example of an I/O request that you'd like to be able
> to submit that the existing UAPI can't declare support for?

I want to support NVMe SGL. This allows virtually *discontiguous*
segments that we currently can't distinguish with what statx reports.
I'm trying to report limits that let applications know what constraints
they're dealing with.

> > If SGL were supported, there would be no virtual boundary gap, and max
> > segments is 256.
> 
> Can you elaborate on why DIO users need to know max_segments?

 * Logical block is 4k.

 * DMA granule is 4 bytes.

 * Max segments is 256.

That's very typical NVMe device contraints under SGL capabilities with
the linux driver.

You can provide 4 byte vectors as needed, but you'll hit the max
segments limit before you have a valid IO if they're all that small.
The average size needs to be larger, so we need to communicate that
somehow.

To be clear, I'm not interested in trying to enable applications
dispatching thousands of 4-byte vectors to do an IO. That's a stupid
application. The applications I'm trying to enable have unpredictable
offsets such that a tiny fraction of vectors are indeed that small, but
that's not a typical vector for the payload. But I can't enable just
some without generically enabling all.

> I'm worried about the UAPI duplication, as well as it going to be very
> difficult for userspace to correctly use this information.  With just
> the two alignments there's at least a chance of them getting it right.
> If we throw virt_boundary_mask and max_segments into the mix, I don't
> think there's much chance.

The blktests framework test case "block/043" does this with great
success with these exact parameters, but it takes these paramters from
the sysfs attributes. The same test works with filesystems too, but I
haven't gotten around to porting it to fstests because it's gating on
having this series.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 1/5] fs: add direct io attributes to file_getattr
       [not found] ` <20260710210646.3576365-2-kbusch@meta.com>
@ 2026-07-13 10:14   ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-07-13 10:14 UTC (permalink / raw)
  To: Keith Busch
  Cc: linux-block, linux-ext4, linux-f2fs-devel, linux-fsdevel,
	linux-xfs, axboe, jack, brauner, cem, jaegeuk, aalbersh, tytso,
	Keith Busch

On Fri, Jul 10, 2026 at 02:06:42PM -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Direct I/O imposes alignment and layout constraints that come from both
> the filesystem and its backing storage. statx() reports some of these,
> but not all, and it can no longer be extended.

Well, it could be extended.  Although it would require a new flag,
and there's been a lot of pushback to adding non-fastpath attributes
to it.   So maybe update this blurb ad bit and also mention which
attributes are duplicated?

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 2/5] block: report direct io attributes through file_getattr
       [not found] ` <20260710210646.3576365-3-kbusch@meta.com>
@ 2026-07-13 11:57   ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-07-13 11:57 UTC (permalink / raw)
  To: Keith Busch
  Cc: linux-block, linux-ext4, linux-f2fs-devel, linux-fsdevel,
	linux-xfs, axboe, jack, brauner, cem, jaegeuk, aalbersh, tytso,
	Keith Busch

>  #include "internal.h"
>  
> @@ -88,13 +89,16 @@ int vfs_fileattr_get(struct dentry *dentry, struct file_kattr *fa)
>  	struct inode *inode = d_inode(dentry);
>  	int error;
>  
> -	if (!inode->i_op->fileattr_get)
> +	if (!inode->i_op->fileattr_get && !S_ISBLK(inode->i_mode))
>  		return -ENOIOCTLCMD;
>  
>  	error = security_inode_file_getattr(dentry, fa);
>  	if (error)
>  		return error;
>  
> +	if (!inode->i_op->fileattr_get)
> +		return bdev_fileattr(inode, fa);
> +
>  	return inode->i_op->fileattr_get(dentry, fa);

While this is correct, it reads a bit odd as the check above
required the S_ISBLK above.  I'd be tempted to move the
-ENOIOCTLCMD below the seurity check to mae it more obvious:

  	error = security_inode_file_getattr(dentry, fa);
  	if (error)
  		return error;

	if (!inode->i_op->fileattr_get) {
		if (!S_ISBLK(inode->i_mode))
			return -ENOIOCTLCMD;
		return bdev_fileattr(inode, fa);
	}

...


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCHv2 3/5] xfs: report direct io attributes through file_getattr
       [not found] ` <20260710210646.3576365-4-kbusch@meta.com>
@ 2026-07-13 12:00   ` Christoph Hellwig
  0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-07-13 12:00 UTC (permalink / raw)
  To: Keith Busch
  Cc: linux-block, linux-ext4, linux-f2fs-devel, linux-fsdevel,
	linux-xfs, axboe, jack, brauner, cem, jaegeuk, aalbersh, tytso,
	Keith Busch

On Fri, Jul 10, 2026 at 02:06:44PM -0700, Keith Busch wrote:
> +static void
> +xfs_fill_fsxattr_dio(
> +	struct xfs_inode	*ip,
> +	int			whichfork,
> +	struct file_kattr	*fa)
> +{
> +	struct xfs_buftarg *target;

Same alignment for the variables as for the arguments.

> +
> +	if (whichfork != XFS_DATA_FORK || !S_ISREG(VFS_I(ip)->i_mode))
> +		return;

Move this check to th caller?  Although this and similar check in
the other file system patches miss block device nodes, for which we'd
need a special case, or one in the caller to override the attrs.

Or maybe using the file attr for this isn't actually a good idea,
and we should do an ioctl instead which automatically gets routed to
the block device fops?

> +	target = xfs_inode_buftarg(ip);

.. and initialize the variable at declaration time?


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-13 12:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260710210646.3576365-1-kbusch@meta.com>
2026-07-10 21:53 ` [f2fs-dev] [PATCHv2 0/5] direct-io file extended attributes Eric Biggers
2026-07-10 22:58   ` Keith Busch
2026-07-11  0:24     ` Eric Biggers
2026-07-11  1:06       ` Keith Busch
     [not found] ` <20260710210646.3576365-2-kbusch@meta.com>
2026-07-13 10:14   ` [PATCHv2 1/5] fs: add direct io attributes to file_getattr Christoph Hellwig
     [not found] ` <20260710210646.3576365-3-kbusch@meta.com>
2026-07-13 11:57   ` [PATCHv2 2/5] block: report direct io attributes through file_getattr Christoph Hellwig
     [not found] ` <20260710210646.3576365-4-kbusch@meta.com>
2026-07-13 12:00   ` [PATCHv2 3/5] xfs: " Christoph Hellwig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox