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.
