Re: [PATCH 3/5] xfs: add XFS_ITEM_UNSAFE for log item push return result

[Dave Chinner <david@fromorbit.com>]

On Tue, Aug 27, 2024 at 05:30:49AM -0700, Christoph Hellwig wrote:
> On Tue, Aug 27, 2024 at 08:00:05PM +1000, Dave Chinner wrote:
> > Hence the only cases where the item might have been already removed
> > from the AIL by the ->iop_push() are those where the push itself
> > removes the item from the AIL. This only occurs in shutdown
> > situations, so it's not the common case.
> > 
> > In which case, returning XFS_ITEM_FREED to tell the push code that
> > it was freed and should not reference it at all is fine. We don't
> > really even need tracing for this case because if the items can't be
> > removed from the AIL, they will leave some other AIL trace when
> > pushe (i.e.  they will be stuck locked, pinned or flushing and those
> > will leave traces...)
> 
> So XFS_ITEM_FREED is definitively a better name, but it still feels
> a bit fragile that any of these shutdown paths need special handling
> inside ->iop_push.

Agreed, but I don't see an easy way to fix that right now because
the shutdown behaviour is both item type and item state specific.

I suspect that we'd do better to have explicit shutdown processing
of log items in the AIL (i.e. a ->iop_shutdown method) that is
called instead of ->iop_push when the AIL detects that the
filesystem has shut down. We can then define the exact behaviour we
want in this case and processing does not have to be non-blocking
for performance and latency reasons.

If we go down that route, I think we'd want to add a
XFS_ITEM_SHUTDOWN return value after the push code calls
xfs_force_shutdown(). The push code does not error out the item or
remove it from the AIL, just shuts down the fs and returns
XFS_ITEM_SHUTDOWN.

The AIL then breaks out of the push loop and submits the delwri
buffers which will error them all out and remove them from the AIL
because the fs is shut down. It then starts a new walk from the tail
calling ->iop_shutdown on everything remaining in the AIL until the
AIL is empty....

-Dave.
-- 
Dave Chinner
david@fromorbit.com