Re: [RFC PATCH 0/2] xfs: change xfs log item refcount design

[Dave Chinner <dgc@kernel.org>]

On Thu, Sep 17, 2026 at 09:21:04AM +0530, Jeffin Philip wrote:
> On Wed, September 16 2026 at 10:28 PM, Dave Chinner wrote:
> >On Fri, Sep 11, 2026 at 03:55:00PM +0530, Jeffin Philip wrote:
> >> This patch series is the first of the series where we attempt to convert
> >> the XFS Log item reference counting design to be more generic
> >> compared to the previous type-specific state to fix the "zero
> >> refcount but in AIL means alive" state. The patch series follows
> >> instructions outlined here[1].
> >> 
> >> Obviously, the wrappers will be integrated in type-specific functions
> >> later on.
> >> 
> >> No behavioral change as of yet.
> >> 
> >> [1]: https://lore.kernel.org/all/aqI6v-_dc2DDSbgB@dread/
> >> 
> >> Jeffin Philip (2):
> >>   xfs: add lockref and generic helpers for refcounting
> >>   xfs: change xfs_trans_ail_delete return type to bool
> >
> >It's great to see you starting on this, but there's not much point
> >in just posting wrappers that aren't used by anything. Until there
> >is code that uses them, we can't really say anythign useful about
> >the wrappers. e.g.
> >
> >Is the API optimal? Don't know.
> >
> >Is open coding the init correct for the generic case or is that just
> >projecting an antipattern from the BLI reference counting? Don't
> >know.
> >
> >And so on. Hence it's best to present the wrappers with the code
> >that then uses them so we have the necessary context with which to
> >review them.
> 
> The BLI changes have almost been finished and I am testing them. The
> reason I did not post it is because I wanted it to face more proper
> review and that is likely to be stricter when the patches are
> few. I could still write the entire implementation and post it as a
> giant patch series provided it does not fatigue the reviewers from
> reviewing several tens of patches at a time. Until then, I feel like
> it mould be much better to post 1 implementation step at a time in 1
> patchset. If you prefer otherwise, that can be done too.

I'm not sure it wise to only convert a single item type at a time,
as there are intricacies in different log item life cycles that may
expose difficult to solve issues further down the track. If we've
already converted and merged some of the code before we find that it
needs a fundamental rework, then that's worse that asking reviewers
to review a large patchset.

e.g. the AIL needs to take reference counts and they need to be
factored into log item life cycles, especially the shutdown
processing of items in the AIL.  Right now the AIL is not
responsible for removing items on shutdown because it does not hold
a reference to them. Changing the AIL to own a reference to the log
item means that there is a good chance that the shutdown behaviour
for items in the AIL may need to change...

So I'd be inclined to ensure at least have one of each group of item
types is fully converted (e.g. buf, inode, dquot, EFI, quotaoff) so
that most of the crouching tigers in the design have been flushed
out before asing for code review.

You can also push it to a git repo somewhere so people whoa re
interested can look at it and give early feedback before you are
ready to post it to the list for review...

-Dave.
-- 
Dave Chinner
dgc@kernel.org