Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt

Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt

[Christoph Hellwig]

On Fri, Mar 21, 2025 at 10:20:21AM +0000, John Garry wrote:
> Coming back to what was discussed about not adding a new flag to fetch this 
> limit:
>
> > Does that actually work?  Can userspace assume all unknown statx
> > fields are padded to zero?
>
> In cp_statx, we do pre-zero the statx structure. As such, the rule "if 
> zero, just use hard limit unit max" seems to hold.

Ok, canwe document this somewhere?

Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt

[John Garry]

On 23/03/2025 06:40, Christoph Hellwig wrote:

I'm not happy with the name stx_atomic_write_unit_max_opt - it's vague 
and subjective.

So I am thinking one of these:
a. stx_atomic_write_unit_max_dev
b. stx_atomic_write_unit_max_bdev
c. stx_atomic_write_unit_max_align
d. stx_atomic_write_unit_max_hw

The terms dev (or device) and bdev are already used in the meaning of 
some members in struct statx, so not too bad. However, when we support 
large atomic writes for XFS rtvol, the bdev atomic write limit and 
rtextsize would influence this value (so just bdev might be a bit 
misleading in the name).

As for stx_atomic_write_unit_max_align, it would mean "max 
alignment/granularity" for possible HW offload. Not great.

stx_atomic_write_unit_max_hw would match the bdev request queue sysfs 
names, but that it a different concept to statx. And it has the same 
issue as bdev for rtvol, above.

Any further suggestions or comments?

> On Fri, Mar 21, 2025 at 10:20:21AM +0000, John Garry wrote:
>> Coming back to what was discussed about not adding a new flag to fetch this
>> limit:
>>
>>> Does that actually work?  Can userspace assume all unknown statx
>>> fields are padded to zero?
>>
>> In cp_statx, we do pre-zero the statx structure. As such, the rule "if
>> zero, just use hard limit unit max" seems to hold.
> 
> Ok, canwe document this somewhere?
> 

Sure, but I want to decide on the name first.. if using 
stx_atomic_write_unit_max_bdev/_dev/hw, then it would be odd that this 
value reports 0 for old kernels (as the bdev limit would never really be 0).

Then if we have rule "stx_atomic_write_unit_max_bdev=0 means that 
stx_atomic_write_unit_max_bdev = stx_atomic_write_unit_max", this breaks 
for when we solely rely on FS-based atomics, as 
stx_atomic_write_unit_max_bdev would be 0 there and that should really 
mean 0 (and not stx_atomic_write_unit_max).

So then we should have a new mask to fetch this field, which is not 
ideal, but ok.

Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt

[Christoph Hellwig]

On Thu, Apr 03, 2025 at 04:07:04PM +0100, John Garry wrote:
> So I am thinking one of these:
> a. stx_atomic_write_unit_max_dev
> b. stx_atomic_write_unit_max_bdev
> c. stx_atomic_write_unit_max_align
> d. stx_atomic_write_unit_max_hw
>
> The terms dev (or device) and bdev are already used in the meaning of some 
> members in struct statx, so not too bad. However, when we support large 
> atomic writes for XFS rtvol, the bdev atomic write limit and rtextsize 
> would influence this value (so just bdev might be a bit misleading in the 
> name).

Don't.  Especially when you have a natively out of write file system
that optimized case will not involve the usual hardware offload.

Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt

[John Garry]

On 04/04/2025 10:06, Christoph Hellwig wrote:
> On Thu, Apr 03, 2025 at 04:07:04PM +0100, John Garry wrote:
>> So I am thinking one of these:
>> a. stx_atomic_write_unit_max_dev
>> b. stx_atomic_write_unit_max_bdev
>> c. stx_atomic_write_unit_max_align
>> d. stx_atomic_write_unit_max_hw
>>
>> The terms dev (or device) and bdev are already used in the meaning of some
>> members in struct statx, so not too bad. However, when we support large
>> atomic writes for XFS rtvol, the bdev atomic write limit and rtextsize
>> would influence this value (so just bdev might be a bit misleading in the
>> name).
> 
> Don't.  Especially when you have a natively out of write file system
> that optimized case will not involve the usual hardware offload.
> 
> 
stx_atomic_write_unit_max_opt it is then.

Or stx_atomic_write_unit_max_optimal or stx_atomic_write_unit_max_fast. 
Or similar..

cheers,
John

Re: [PATCH RFC] statx.2: Add stx_atomic_write_unit_max_opt

[Christoph Hellwig]

On Fri, Apr 04, 2025 at 10:23:09AM +0100, John Garry wrote:
>> that optimized case will not involve the usual hardware offload.
>>
>>
> stx_atomic_write_unit_max_opt it is then.
>
> Or stx_atomic_write_unit_max_optimal or stx_atomic_write_unit_max_fast. Or 
> similar..

As we've used opt in various other ABIs I'd stick to that.