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

Darrick J. Wong
2023-10-03T15:46:47Z

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