Re: [PATCH 00/14] xfs: embed dfops in the transaction

[Christoph Hellwig <hch@infradead.org>]

On Thu, Jul 19, 2018 at 09:49:05AM -0400, Brian Foster wrote:
> return a clean transaction. Other things to consider might be to do away
> with support for external dfops and the ->t_dfops pointer indirection,
> or perhaps even consider going the other direction: allocate dfops from
> a separate zone to save some memory on non-permanent transactions (note
> that 16 of 28 transactions use a permanent log res. last I looked, so it
> may not be worth it atm).

The defer_ops aren't really that big, and allocations are relatively
costly, so I don't think a separate allocation is a good idea.  If we
really want to optimize the non-permanent transaction case we could do
something like:

struct xfs_trans {
	...
	struct xfs_defer_ops dfops[];
};

and then have two caches for the with an without dfops case.  But
I can't believe that would be worth it, especially in face of...


> I know Christoph also had thoughts around condensing some of the items
> joined to the dfops to those with the transaction.

... this.

> I have yet to think
> about that one, but I do have an RFC quality patch laying around that
> replaces the ->dop_low flag with a transaction flag (->t_flags),
> eliminating the need for that extra byte in xfs_defer_ops. The one quirk
> associated with that is the question of whether we want to preserve the
> behavior where low mode remains active across the series of transactions
> associated with the traditional (on-stack) dfops or is reset on
> transaction roll (a la firstblock). I'll post that RFC separately for a
> more proper discussion..

That sounds like a good enough start.  For now I'd keep the existing
behavior because it really is deep magic and needs a deep audit.  I
had started on that a long time ago but dropped the ball, but mixing
it with this work is probably not helpful.