Re: [PATCH 03/21] fs/bdev: Add atomic write support info to statx

[Darrick J. Wong]

On Tue, Oct 03, 2023 at 08:23:26AM +0100, John Garry wrote:
> On 03/10/2023 03:57, Darrick J. Wong wrote:
> > > > > > +#define STATX_ATTR_WRITE_ATOMIC        0x00400000 /* File
> > > > > > supports atomic write operations */
> > > > > How would this differ from stx_atomic_write_unit_min != 0?
> > > Yeah, I suppose that we can just not set this for the case of
> > > stx_atomic_write_unit_min == 0.
> > Please use the STATX_ATTR_WRITE_ATOMIC flag to indicate that the
> > filesystem, file and underlying device support atomic writes when
> > the values are non-zero. The whole point of the attribute mask is
> > that the caller can check the mask for supported functionality
> > without having to read every field in the statx structure to
> > determine if the functionality it wants is present.
> 
> Sure, but again that would be just checking atomic_write_unit_min_bytes or
> another atomic write block setting as that is the only way to tell from the
> block layer (if atomic writes are supported), so it will be something like:
> 
> if (request_mask & STATX_WRITE_ATOMIC &&
> queue_atomic_write_unit_min_bytes(bdev->bd_queue)) {
>     stat->atomic_write_unit_min =
>       queue_atomic_write_unit_min_bytes(bdev->bd_queue);
>     stat->atomic_write_unit_max =
>       queue_atomic_write_unit_max_bytes(bdev->bd_queue);
>     stat->attributes |= STATX_ATTR_WRITE_ATOMIC;
>     stat->attributes_mask |= STATX_ATTR_WRITE_ATOMIC;
>     stat->result_mask |= STATX_WRITE_ATOMIC;

The result_mask (which becomes the statx stx_mask) needs to have
STATX_WRITE_ATOMIC set any time a filesystem responds to
STATX_WRITE_ATOMIC being set in the request_mask, even if the response
is "not supported".

The attributes_mask also needs to have STATX_ATTR_WRITE_ATOMIC set if
the filesystem+file can support the flag, even if it's not currently set
for that file.  This should get turned into a generic vfs helper for the
next fs that wants to support atomic write units:

static void generic_fill_statx_atomic_writes(struct kstat *stat,
		struct block_device *bdev)
{
	u64 min_bytes;

	/* Confirm that the fs driver knows about this statx request */
	stat->result_mask |= STATX_WRITE_ATOMIC;

	/* Confirm that the file attribute is known to the fs. */
	stat->attributes_mask |= STATX_ATTR_WRITE_ATOMIC;

	/* Fill out the rest of the atomic write fields if supported */
	min_bytes = queue_atomic_write_unit_min_bytes(bdev->bd_queue);
	if (min_bytes == 0)
		return;

	stat->atomic_write_unit_min = min_bytes;
	stat->atomic_write_unit_max =
			queue_atomic_write_unit_max_bytes(bdev->bd_queue);

	/* Atomic writes actually supported on this file. */
	stat->attributes |= STATX_ATTR_WRITE_ATOMIC;
}

and then:

	if (request_mask & STATX_WRITE_ATOMIC)
		generic_fill_statx_atomic_writes(stat, bdev);


> }
> 
> Thanks,
> John