On Wed, May 01, 2024 at 12:36:02PM +0100, John Garry wrote:
> On 01/05/2024 02:32, Dave Chinner wrote:
> > On Mon, Apr 29, 2024 at 05:47:40PM +0000, John Garry wrote:
> > > Set iomap->extent_size when sub-extent zeroing is required.
> > >
> > > We treat a sub-extent write same as an unaligned write, so we can leverage
> > > the existing sub-FSblock unaligned write support, i.e. try a shared lock
> > > with IOMAP_DIO_OVERWRITE_ONLY flag, if this fails then try the exclusive
> > > lock.
> > >
> > > In xfs_iomap_write_unwritten(), FSB calcs are now based on the extsize.
> >
> > If forcedalign is set, should we just reject unaligned DIOs?
>
> Why would we? That's very restrictive. Indeed, we got to the point of adding
> the sub-extent zeroing just for supporting that.
> > > @@ -646,9 +647,9 @@ xfs_file_dio_write_unaligned(
> > > ssize_t ret;
> > > /*
> > > - * Extending writes need exclusivity because of the sub-block zeroing
> > > - * that the DIO code always does for partial tail blocks beyond EOF, so
> > > - * don't even bother trying the fast path in this case.
> > > + * Extending writes need exclusivity because of the sub-block/extent
> > > + * zeroing that the DIO code always does for partial tail blocks
> > > + * beyond EOF, so don't even bother trying the fast path in this case.
> > > */
> > > if (iocb->ki_pos > isize || iocb->ki_pos + count >= isize) {
> > > if (iocb->ki_flags & IOCB_NOWAIT)
> > > @@ -714,11 +715,19 @@ xfs_file_dio_write(
> > > struct xfs_inode *ip = XFS_I(file_inode(iocb->ki_filp));
> > > struct xfs_buftarg *target = xfs_inode_buftarg(ip);
> > > size_t count = iov_iter_count(from);
> > > + struct xfs_mount *mp = ip->i_mount;
> > > + unsigned int blockmask;
> > > /* direct I/O must be aligned to device logical sector size */
> > > if ((iocb->ki_pos | count) & target->bt_logical_sectormask)
> > > return -EINVAL;
> > > - if ((iocb->ki_pos | count) & ip->i_mount->m_blockmask)
> > > +
> > > + if (xfs_inode_has_forcealign(ip) && ip->i_extsize > 1)
> > > + blockmask = XFS_FSB_TO_B(mp, ip->i_extsize) - 1;
> > > + else
> > > + blockmask = mp->m_blockmask;
> >
> > alignmask = XFS_FSB_TO_B(mp, xfs_inode_alignment(ip)) - 1;
>
> Do you mean xfs_extent_alignment() instead of xfs_inode_alignment()?
Yes, I was.
I probably should have named it xfs_inode_extent_alignment() because
clearly I kept thinking of it as "inode alignment"... :)
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
