Re: [PATCH 25/25] xfs: remove xfs_defer_init() firstblock param

[Brian Foster <bfoster@redhat.com>]

On Sun, Jul 08, 2018 at 08:37:08AM -0700, Christoph Hellwig wrote:
> On Tue, Jul 03, 2018 at 06:27:43PM -0700, Darrick J. Wong wrote:
> > Moving on to the question of whether or not to embed a struct
> > xfs_defer_ops into struct xfs_trans instead of just a pointer, it looks
> > to me like that should be a pretty straightforward conversion.  Most of
> > the defer_ops users keep it within the scope of a single
> > xfs_trans_{alloc,commit} pair so we can pass *tp instead of *dfops into
> > the helpers.
> 
> I'd like to see that happen eventually, probably rather sooner than
> later.
> 
> > The one big exception to that of course is the log item replay where we
> > don't want to finish any of the new defer_ops until we're done with
> > replay, for which we'll need to have a xfs_defer_move to transfer all
> > the items and [bi]join'd state from one dfops to another.  This is
> > probably the same mechanism that you'd have to use to preserve dfops
> > state in xfs_defer_trans_roll.
> 
> Yes.  As far as I can tell we can just list_split_init the two lists
> over, copy over the inodes and bufs arrays (at least if we are rolling
> the transaction) and mostly ignore dop_low and dop_commited.
> 

I have a series in progress that takes care of most of this. It has some
details left to work out and needs cleanup, but survived an initial
round of tests. Unfortunately I won't have much time to get to it this
week, but hopefully I'll have it cleaned up enough to post something
next week.

Brian

> Now the transaction itself actually already has a list of items joined
> to them, which must include dop_inodes and dop_bufs, and I suspect
> we should just look at the items and rejoin all inodes and bufs instead
> of separately keeping track of them for the normal roll case.  We'd
> just need some special outside storage for the log recovery case.
> 
> > The other area of trickiness I anticipate is Allison's reworking of the
> > xattr code's usage of defer_ops to eliminate the repeated creation and
> > finishing of defer ops.  Even if attribute operations can still allocate
> > and commit multiple transactions, we'll have to find a way to carry the
> > defer_ops state (the attr intent item and presumably a _defer_ijoin'd
> > inode) across every one of those transactions.  I'm not sure how far
> > she's gotten with that, but some coordination is needed.
> 
> In general attribute operations should be rolling transactions, and
> if they aren't we probably found a bug.  Once it is a rolling
> transaction we've already got all the state coverted by the
> xfs_defer_trans_roll equivalent case.