fs/xfs/xfs_log.c | 38 ++++++++++++++++++++++++++++++++++++++ fs/xfs/xfs_trans.h | 7 +++++++ fs/xfs/xfs_trans_ail.c | 5 +++-- fs/xfs/xfs_trans_priv.h | 2 +- 4 files changed, 49 insertions(+), 3 deletions(-)
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 fs/xfs/xfs_log.c | 38 ++++++++++++++++++++++++++++++++++++++ fs/xfs/xfs_trans.h | 7 +++++++ fs/xfs/xfs_trans_ail.c | 5 +++-- fs/xfs/xfs_trans_priv.h | 2 +- 4 files changed, 49 insertions(+), 3 deletions(-) -- 2.55.0
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. Cheers, Dave. -- Dave Chinner dgc@kernel.org
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. Thanks, Jeffin.
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
© 2016 - 2026 Red Hat, Inc.