* Re: [f2fs-dev] [PATCHv2 0/5] direct-io file extended attributes
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
In-Reply-To: <20260710210646.3576365-1-kbusch@meta.com>
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
* Re: [f2fs-dev] [PATCHv2 0/5] direct-io file extended attributes
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
In-Reply-To: <20260710215328.GE1911@quark>
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
* Re: [PATCH] fs: report direct io constraints through file_getattr
From: Keith Busch @ 2026-07-10 23:11 UTC (permalink / raw)
To: Eric Biggers
Cc: Jan Kara, Christoph Hellwig, Keith Busch, linux-block, linux-ext4,
linux-f2fs-devel, linux-fsdevel, linux-xfs, axboe, brauner, cem,
jaegeuk, aalbersh, tytso
In-Reply-To: <20260710212058.GA1911@quark>
On Fri, Jul 10, 2026 at 05:20:58PM -0400, Eric Biggers wrote:
> So far I haven't seen the point. Yes, applications can benefit from the
> lower alignment in theory. But especially with encryption/decryption,
> it isn't at all easy to support. This has apparently been getting
> learned the hard way, as (for example) alignment was initially relaxed
> for dm-crypt without testing it, and it had to be reverted
> (https://lore.kernel.org/dm-devel/20221103152559.1909328-1-kbusch@meta.com/).
To be fair, that was most certainly tested in production, and it was
reverted for a bug not related to alignment. It was just a mishandled
early exit corner case accessing uninitialized fields; a trivial fixup.
^ permalink raw reply
* Re: [PATCH V4 7/9] null_blk: reject per-device queue resize for shared tag set
From: Zizhi Wo @ 2026-07-11 0:13 UTC (permalink / raw)
To: Nilay Shroff, Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn,
kbusch, bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1
In-Reply-To: <4dbcbb4a-852e-4ae8-b909-79d1aeeb0799@linux.ibm.com>
在 2026/7/10 20:34, Nilay Shroff 写道:
> On 7/9/26 3:34 PM, Zizhi Wo wrote:
>> From: Zizhi Wo <wozizhi@huawei.com>
>>
>> When shared_tags is enabled, null_setup_tagset() makes the device use the
>> global tag_set, whose driver_data stays NULL. null_map_queues() therefore
>> falls back to the module-wide g_submit_queues/g_poll_queues instead of
>> any
>> per-device value.
>>
>> Resizing submit_queues or poll_queues via configfs on such a device calls
>> blk_mq_update_nr_hw_queues() on the shared set, shrinking
>> set->nr_hw_queues. __blk_mq_realloc_hw_ctxs() only grows the
>> q->queue_hw_ctx[] allocation, so on shrink it merely exits and NULLs the
>> now-excess hctx slots. null_map_queues(), however, keeps mapping CPUs
>> with
>> the unchanged g_submit_queues/g_poll_queues, so mq_map[] ends up pointing
>> at those NULLed hctx slots. blk_mq_map_swqueue() then dereferences the
>> NULL
>> hctx (hctx->cpumask), crashing the kernel:
>>
>> [ 460.218374] KASAN: null-ptr-deref in range
>> [0x0000000000000098-0x000000000000009f]
>> [ 460.219003] CPU: 24 UID: 0 PID: 1492 Comm: sh Not tainted 7.2.0-
>> rc2+ #67 PREEMPT(full)
>> [ 460.219792] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
>> BIOS 1.17.0-4.fc41 04/01/2014
>> [ 460.220452] RIP: 0010:blk_mq_map_swqueue+0x4db/0x1430
>> ......
>> [ 460.228977] Call Trace:
>> [ 460.229175] <TASK>
>> [ 460.229354] blk_mq_update_nr_hw_queues+0xd49/0x11c0
>> [ 460.229779] ? __pfx_blk_mq_update_nr_hw_queues+0x10/0x10
>> [ 460.230200] nullb_update_nr_hw_queues+0x1a9/0x370 [null_blk]
>> [ 460.230694] nullb_device_submit_queues_store+0xd9/0x170 [null_blk]
>> [ 460.231190] ? __pfx_nullb_device_submit_queues_store+0x10/0x10
>> [null_blk]
>> [ 460.231776] ? configfs_write_iter+0x35c/0x4e0
>> [ 460.232122] configfs_write_iter+0x286/0x4e0
>> [ 460.232460] vfs_write+0x52d/0xd00
>> [ 460.232779] ? __x64_sys_openat+0x108/0x1d0
>> [ 460.233106] ? __pfx_vfs_write+0x10/0x10
>> [ 460.233413] ? fdget_pos+0x1cf/0x4c0
>> [ 460.233745] ? fput_close+0x133/0x190
>> [ 460.234038] ? __pfx_expand_files+0x10/0x10
>> [ 460.234368] ksys_write+0xfc/0x1d0
>>
>> Reproducer:
>> modprobe null_blk shared_tags=1 submit_queues=64 poll_queues=1
>> mkdir /sys/kernel/config/nullb/dev
>> echo 1 > /sys/kernel/config/nullb/dev/power
>> echo 1 > /sys/kernel/config/nullb/dev/submit_queues
>>
>> A per-device resize of a shared tag set is meaningless anyway, so
>> reject it
>> with -EINVAL in nullb_update_nr_hw_queues() when the device is bound
>> to the
>> global tag_set.
>>
>> Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after
>> an instance has been configured")
>> Signed-off-by: Zizhi Wo <wozizhi@huawei.com>
>> ---
>> drivers/block/null_blk/main.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/
>> main.c
>> index 6d30591abb28..340ecc0a331e 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -380,10 +380,19 @@ static int nullb_update_nr_hw_queues(struct
>> nullb_device *dev,
>> int ret, nr_hw_queues;
>> if (!dev->nullb)
>> return 0;
>> + /*
>> + * A shared tag_set is mapped via the module-wide queue counts, so a
>> + * per-device resize is meaningless. On shrink it would also leave
>> + * mq_map[] pointing at NULLed hctx slots, causing a NULL deref in
>> + * blk_mq_map_swqueue(). Reject it.
>> + */
>> + if (dev->nullb->tag_set == &tag_set)
>> + return -EINVAL;
>> +
>
> Wouldn't it be simpler to check dev->shared_tags here instead? Since the
> restriction
> is specifically for devices configured with shared tags, that seems a
> bit easier to
> reason about than checking whether dev->nullb->tag_set points to the
> global tag_set.
> Something like this:
>
> if (dev->shared_tags)
> return -EINVAL;
> Thanks,
> --Nilay
>
>
Thanks for the suggetsion. I'll switch to it in the next version.
Thanks,
Zizhi Wo
^ permalink raw reply
* Re: [PATCHv2 0/5] direct-io file extended attributes
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
In-Reply-To: <alF5BHEZ5czyJWaY@kbusch-mbp>
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
* Re: [PATCH V4 5/9] null_blk: free zones array on device power-off
From: Zizhi Wo @ 2026-07-11 0:52 UTC (permalink / raw)
To: Nilay Shroff, Zizhi Wo, axboe, dlemoal, kch, johannes.thumshirn,
kbusch, bvanassche, linux-block
Cc: linux-kernel, yangerkun, chengzhihao1
In-Reply-To: <c110c8bf-52eb-4f01-b01d-b042178a6002@linux.ibm.com>
Hi Nilay!
在 2026/7/10 22:24, Nilay Shroff 写道:
> On 7/9/26 3:34 PM, Zizhi Wo wrote:
>> null_init_zoned_dev() allocates dev->zones when a zoned device is powered
>> on, but null_del_dev() never frees it on power-off; dev->zones is only
>> freed later in null_free_dev(), when the configfs directory is
>> removed. If
>> the device is powered off and then on again, null_init_zoned_dev()
>> allocates a new array and overwrites the dev->zones pointer, leaking the
>> previous allocation each power cycle.
>>
>> Free dev->zones in null_del_dev() via null_free_zoned_dev() to solve it.
>> And calling null_del_dev() in null_free_dev() is no longer necessary
>> because every caller already invokes null_del_dev() first: via
>> nullb_group_drop_item() before nullb_device_release(), in the
>> null_add_dev() error path of null_create_dev(), and in
>> null_destroy_dev().
>> Remove the redundant call.
>
> Looks good to me.
>
> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com>
Thanks for the review!
However, I just checked Sashiko and found that modifying it this way
directly would introduce a problem:
the caller of zone_cond_store() is lock-free, and internally it checks
whether dev->zones exists. This would then create a race with my patch.
The reason there was no problem before is that only
nullb_device_release() or null_exit() frees the dev->zones, which
guarantees that subsequent users won't access the configfs interface.
I'll address this issue in the next version by adding locking to them.
Thanks,
Zizhi Wo
^ permalink raw reply
* Re: [PATCHv2 0/5] direct-io file extended attributes
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
In-Reply-To: <20260711002412.GG1911@quark>
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
page: | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox