[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230503215846.GE3223426@dread.disaster.area>
Date: Thu, 4 May 2023 07:58:46 +1000
From: Dave Chinner <david@...morbit.com>
To: John Garry <john.g.garry@...cle.com>
Cc: axboe@...nel.dk, kbusch@...nel.org, hch@....de, sagi@...mberg.me,
martin.petersen@...cle.com, djwong@...nel.org,
viro@...iv.linux.org.uk, brauner@...nel.org, dchinner@...hat.com,
jejb@...ux.ibm.com, linux-block@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org,
linux-scsi@...r.kernel.org, linux-xfs@...r.kernel.org,
linux-fsdevel@...r.kernel.org,
linux-security-module@...r.kernel.org, paul@...l-moore.com,
jmorris@...ei.org, serge@...lyn.com,
Prasad Singamsetty <prasad.singamsetty@...cle.com>
Subject: Re: [PATCH RFC 02/16] fs/bdev: Add atomic write support info to statx
On Wed, May 03, 2023 at 06:38:07PM +0000, John Garry wrote:
> From: Prasad Singamsetty <prasad.singamsetty@...cle.com>
>
> Extend statx system call to return additional info for atomic write support
> support if the specified file is a block device.
>
> Add initial support for a block device.
>
> Signed-off-by: Prasad Singamsetty <prasad.singamsetty@...cle.com>
> Signed-off-by: John Garry <john.g.garry@...cle.com>
> ---
> block/bdev.c | 21 +++++++++++++++++++++
> fs/stat.c | 10 ++++++++++
> include/linux/blkdev.h | 4 ++++
> include/linux/stat.h | 2 ++
> include/uapi/linux/stat.h | 7 ++++++-
> 5 files changed, 43 insertions(+), 1 deletion(-)
>
> diff --git a/block/bdev.c b/block/bdev.c
> index 1795c7d4b99e..6a5fd5abaadc 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -1014,3 +1014,24 @@ void bdev_statx_dioalign(struct inode *inode, struct kstat *stat)
>
> blkdev_put_no_open(bdev);
> }
> +
> +/*
> + * Handle statx for block devices to get properties of WRITE ATOMIC
> + * feature support.
> + */
> +void bdev_statx_atomic(struct inode *inode, struct kstat *stat)
> +{
> + struct block_device *bdev;
> +
> + bdev = blkdev_get_no_open(inode->i_rdev);
> + if (!bdev)
> + return;
> +
> + stat->atomic_write_unit_min = queue_atomic_write_unit_min(bdev->bd_queue);
> + stat->atomic_write_unit_max = queue_atomic_write_unit_max(bdev->bd_queue);
> + stat->attributes |= STATX_ATTR_WRITE_ATOMIC;
> + stat->attributes_mask |= STATX_ATTR_WRITE_ATOMIC;
> + stat->result_mask |= STATX_WRITE_ATOMIC;
> +
> + blkdev_put_no_open(bdev);
> +}
> diff --git a/fs/stat.c b/fs/stat.c
> index 7c238da22ef0..d20334a0e9ae 100644
> --- a/fs/stat.c
> +++ b/fs/stat.c
> @@ -256,6 +256,14 @@ static int vfs_statx(int dfd, struct filename *filename, int flags,
> bdev_statx_dioalign(inode, stat);
> }
>
> + /* 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()....
> 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....
> 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. Is there a statx() man
page update for this addition?
Cheers,
Dave.
--
Dave Chinner
david@...morbit.com
Powered by blists - more mailing lists