On 03/05/2023 22:58, Dave Chinner wrote: Hi Dave,
+ /* Handle STATX_WRITE_ATOMIC for block devices */ + if (request_mask & STATX_WRITE_ATOMIC) { + struct inode *inode = d_backing_inode(path.dentry); + + if (S_ISBLK(inode->i_mode)) + bdev_statx_atomic(inode, stat); + }This duplicates STATX_DIOALIGN bdev handling. Really, the bdev attribute handling should be completely factored out of vfs_statx() - blockdevs are not the common fastpath for stat operations. Somthing like: /* * If this is a block device inode, override the filesystem * attributes with the block device specific parameters * that need to be obtained from the bdev backing inode. */ if (S_ISBLK(d_backing_inode(path.dentry)->i_mode)) bdev_statx(path.dentry, stat); And then all the overrides can go in the one function that doesn't need to repeatedly check S_ISBLK()....
ok, that looks like a reasonable idea. We'll look to make that change.
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h index 6b6f2992338c..19d33b2897b2 100644 --- a/include/linux/blkdev.h +++ b/include/linux/blkdev.h @@ -1527,6 +1527,7 @@ int sync_blockdev_range(struct block_device *bdev, loff_t lstart, loff_t lend); int sync_blockdev_nowait(struct block_device *bdev); void sync_bdevs(bool wait); void bdev_statx_dioalign(struct inode *inode, struct kstat *stat); +void bdev_statx_atomic(struct inode *inode, struct kstat *stat); void printk_all_partitions(void); #else static inline void invalidate_bdev(struct block_device *bdev) @@ -1546,6 +1547,9 @@ static inline void sync_bdevs(bool wait) static inline void bdev_statx_dioalign(struct inode *inode, struct kstat *stat) { } +static inline void bdev_statx_atomic(struct inode *inode, struct kstat *stat) +{ +} static inline void printk_all_partitions(void) { }That also gets rid of the need for all these fine grained exports out of the bdev code for statx....
Sure
diff --git a/include/linux/stat.h b/include/linux/stat.h index 52150570d37a..dfa69ecfaacf 100644 --- a/include/linux/stat.h +++ b/include/linux/stat.h @@ -53,6 +53,8 @@ struct kstat { u32 dio_mem_align; u32 dio_offset_align; u64 change_cookie; + u32 atomic_write_unit_max; + u32 atomic_write_unit_min; };/* These definitions are internal to the kernel for now. Mainly used by nfsd. */diff --git a/include/uapi/linux/stat.h b/include/uapi/linux/stat.h index 7cab2c65d3d7..c99d7cac2aa6 100644 --- a/include/uapi/linux/stat.h +++ b/include/uapi/linux/stat.h @@ -127,7 +127,10 @@ struct statx { __u32 stx_dio_mem_align; /* Memory buffer alignment for direct I/O */ __u32 stx_dio_offset_align; /* File offset alignment for direct I/O */ /* 0xa0 */ - __u64 __spare3[12]; /* Spare space for future expansion */ + __u32 stx_atomic_write_unit_max; + __u32 stx_atomic_write_unit_min; + /* 0xb0 */ + __u64 __spare3[11]; /* Spare space for future expansion */ /* 0x100 */ };No documentation on what units these are in.
It's in bytes, we're really just continuing the pattern of what we do for dio. I think that it would be reasonable to limit to u32.
Is there a statx() man page update for this addition?
No, not yet. Is it normally expected to provide a proposed man page update in parallel? Or somewhat later, when the kernel API change has some appreciable level of agreement?
Thanks, John
