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

Dave Chinner
2024-08-29T10:16:43Z

On Tue, Aug 27, 2024 at 09:23:33PM -0700, Christoph Hellwig wrote:
> On Wed, Aug 28, 2024 at 07:52:48AM +1000, Dave Chinner wrote:
> > 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.
>
> Yes, that seems even better. But it would probably be a fair amount
> of work.

I don't think so. Only the log items that implement ->iop_push would
need to implement ->iop_shutdown, and there are only 3 items that
implement iop_push. i.e. inode, dquot and buffer items.

The buffer item shutdown method would simply be:

{
xfs_buf_lock(bp);
bp->b_flags |= XBF_ASYNC;
xfs_buf_ioend_fail(bp);
return XFS_ITEM_FREED;
}

The inode item would use the same setup as xfs_inode_item_push()
to lock the inode cluster buffer, then run the same code as above
for the buffer item shutdown.

I think that dquots end up the same - do the same setup work to get
the locked dquot buffer, then fail it directly without doing IO.

It doesn't seem like it's all that complex....

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