Re: [PATCH 1/6] xfs: check if_bytes under the ilock in xfs_reflink_end_cow_extent

[Christoph Hellwig <hch@lst.de>]

On Fri, Mar 29, 2024 at 09:14:29AM -0700, Darrick J. Wong wrote:
> This unlocked access was supposed to short-circuit the case where
> there's absolutely nothing in the cow fork at all, so that we don't have
> to wait for a transaction and the ILOCK.  Is the unlocked access
> causing problems?

I've not observeved problems.  But I can't see how this access can
be race free.  Note that this case can only happen if we have racy
direct I/O writes, so I'm not sure trying to optimize performance for
it makes much sense.

> > +	/* No COW extents?  That's easy! */
> > +	if (ifp->if_bytes == 0) {
> > +		*offset_fsb = end_fsb;
> > +		goto out_cancel;
> > +	}
> 
> This is already taken care of by the clause that comes below
> the end of this diff:


> Since xfs_iext_lookup_extent will return false if the cow fork tree is
> empty.
> That said, I think the xfs_iext_count_may_overflow stuff is misplaced --
> we should be querying the cow fork extent and bouncing out early before
> we bother with checking/upgrading the nextents width.  If
> xfs_iext_count_upgrade dirtied the transaction, the early bailout will
> cause a shutdown.
> 
> (The iext upgrade only needs to happen after the bmapi_read.)

Yes, I guess the right thing is to move the upgrade later and then
just let xfs_iext_lookup_extent handle the no extents case.