Re: [PATCH v4 06/11] block: Add atomic write support for statx

[John Garry]

On 20/02/2024 08:29, Christoph Hellwig wrote:
> > +#define BDEV_STATX_SUPPORTED_MASK (STATX_DIOALIGN | STATX_WRITE_ATOMIC)
>
> > +	if (!(request_mask & BDEV_STATX_SUPPORTED_MASK))
> > +		return;
>
> BDEV_STATX_SUPPORTED_MASK is misleading here.  bdevs support a lot more
> fields, these are just the ones needing special attention.  I'd do away
> with the extra define and just open code it.

ok, fine

> > +	/* 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
> > +	 */
>
> This is not the normal kernel multi-line comment format.

will fix

> > +	if (S_ISBLK(d_backing_inode(path.dentry)->i_mode))
> > +		bdev_statx(path.dentry, stat, request_mask);
>
> I know I touched this last, but does anyone remember why we have
> various random fixups in vfs_statx and not in vfs_getattr_nosec, where
> they we have more of them and also the inode at hand?