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