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

[Dave Chinner <david@fromorbit.com>]

On Fri, Aug 23, 2024 at 08:34:43PM -0700, Christoph Hellwig wrote:
> On Fri, Aug 23, 2024 at 07:04:37PM +0800, Long Li wrote:
> > After pushing log items, the log item may have been freed, making it
> > unsafe to access in tracepoints. This commit introduces XFS_ITEM_UNSAFE
> > to indicate when an item might be freed during the item push operation.
> 
> So instead of this magic unsafe operation I think declaring a rule that
> the lip must never be accessed after the return is the much saner
> choice.

That may well be the case, but in the normal case the only way to
remove the item from the AIL is to run IO completion. We don't
actually submit IO for anything we've pushed until after we've
dropped out of the item push loop, so IO completion and potential
item AIL removal and freeing can't occur for anything we need to do
IO on.

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...)

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