Re: [PATCH v7 09/14] xfs: add large atomic writes checks in xfs_direct_write_iomap_begin()

[John Garry]

On 15/04/2025 18:34, Darrick J. Wong wrote:
> > +	/*
> > +	 * Spanning multiple extents would mean that multiple BIOs would be
> > +	 * issued, and so would lose atomicity required for REQ_ATOMIC-based
> > +	 * atomics.
> > +	 */
> > +	if (!imap_spans_range(imap, offset_fsb, end_fsb))
> > +		return false;
> > +
> > +	/*
> > +	 * The ->iomap_begin caller should ensure this, but check anyway.
> > +	 */
> > +	if (len > xfs_inode_buftarg(ip)->bt_bdev_awu_max)
> > +		return false;
> This needs to check len against bt_bdev_awu_min so that we don't submit
> too-short atomic writes to the hardware. 

Right, let me check this.

I think that we should only support sane HW which can write 1x FS block 
or more.

> Let's say that the hardware
> minimum is 32k and the fsblock size is 4k.  XFS can perform an out of
> place write for 4k-16k writes, but right now we'll just throw invalid
> commands at the bdev, and it'll return EINVAL.
>
> /me wonders if statx should grow a atomic_write_unit_min_opt field
> too, unless everyone in block layer land is convinced that awu_min will
> always match lbasize?  (I probably missed that conversation)

Nothing states that it should (match lbasize), but again HW which can 
only write >1 FS block is something which I don't want to support (yet).

Thanks,
John