Re: [PATCH] xfs: don't use BMBT btree split workers for IO completion

[Dave Chinner <david@fromorbit.com>]

On Thu, Jan 19, 2023 at 10:33:26AM -0800, Christoph Hellwig wrote:
> On Thu, Jan 19, 2023 at 12:03:34PM +1100, Dave Chinner wrote:
> > The other place we can be called for a BMBT split without a
> > preceeding allocation is __xfs_bunmapi() when punching out the
> > center of an existing extent. We don't remove extents in the IO
> > path, so these operations don't tend to be called with a lot of
> > stack consumed. Hence we don't really need to ship the split off to
> > a worker thread in these cases, either.
> 
> So I agree with the fix.  But the t_firstblock seems a bit opaque.
> We do have a lot of comments, which is good but it still feels
> a little fragile to me.  Is there a good reason we can't do an
> explicit flag to document the intent better?

I don't see any point in adding flags to high level allocation APIs
that expose deeply internal btree modification implementation
details. Caller have no business knowing about this, have no idea
how much stack they might need or consume, what context a btree split
might occur in, etc. So I don't think there's any reason at all for
exposing this btree split offload implementation detail to any other
code.

As for the "documentation" aspect of the change, see the patchset I
just posted for "per-ag centric allocation". That fixes all the
existing problems with tp->t_firstblock and also converts it to
store an agno (i.e. tp->t_highest_agno) to make it clear that an AG
access restriction has been placed on this transaction.

That's exactly the situation that this deadlock avoidance needs to
be aware of, so I don't see any real need to duplicate this
information given the rework of the t-firstblock infrastructure that
is out for review.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com