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

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

[Jeffin Philip]

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

[RFC PATCH 1/2] xfs: add lockref and generic helpers for refcounting

[Jeffin Philip]

As part of fixing the UAF in xlog_cil_ail_insert() reported by
syzbot, add a generic lockref to xfs_log_item struct and
initialize it in xfs_log_item_init(). In addition, add generic
helpers(get()/put()/get_safe()) as part of the generic refcounting
infrastructure for xfs.

Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
 fs/xfs/xfs_log.c   | 38 ++++++++++++++++++++++++++++++++++++++
 fs/xfs/xfs_trans.h |  7 +++++++
 2 files changed, 45 insertions(+)

diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index f807f8f4f705..1489f8f20b3e 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -1033,12 +1033,50 @@ xfs_log_item_init(
 	item->li_ops = ops;
 	item->li_lv = NULL;
 
+	/*
+	 * Refrain from using lockref_init as BLI refcount should be
+	 * initialized to 0 and lockref_init initializes refcount to 1
+	 */
+	spin_lock_init(&item->li_ref.lock);
+	item->li_ref.count = 0;
 	INIT_LIST_HEAD(&item->li_ail);
 	INIT_LIST_HEAD(&item->li_cil);
 	INIT_LIST_HEAD(&item->li_bio_list);
 	INIT_LIST_HEAD(&item->li_trans);
 }
 
+/*
+ * Only called when the caller knows the object is alive
+ */
+void
+xfs_log_item_get(
+	struct xfs_log_item	*lip)
+{
+	lockref_get(&lip->li_ref);
+}
+
+/*
+ * Drop a log item reference when called. Returns true if last
+ * ref with lock held. Otherwise false.
+ */
+bool
+xfs_log_item_put(
+	struct xfs_log_item	*lip)
+{
+	return lockref_put_or_lock(&lip->li_ref);
+}
+
+/*
+ * Used to lookup if item may be dying. Returns true is the object
+ * is not dead, false otherwise.
+ */
+bool
+xfs_log_item_get_safe(
+	struct xfs_log_item	*lip)
+{
+	return lockref_get_not_dead(&lip->li_ref);
+}
+
 /*
  * Wake up processes waiting for log space after we have moved the log tail.
  */
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index eb83c5dac032..cd469e2e4e4d 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -6,6 +6,8 @@
 #ifndef	__XFS_TRANS_H__
 #define	__XFS_TRANS_H__
 
+#include <linux/lockref.h>
+
 /* kernel only transaction subsystem defines */
 
 struct xlog;
@@ -46,6 +48,8 @@ struct xfs_log_item {
 	struct xfs_log_vec		*li_lv_shadow;	/* standby vector */
 	xfs_csn_t			li_seq;		/* CIL commit seq */
 	uint32_t			li_order_id;	/* CIL commit order */
+
+	struct lockref			li_ref;		/* log item reference */
 };
 
 /*
@@ -110,6 +114,9 @@ xlog_item_is_intent_done(struct xfs_log_item *lip)
 
 void	xfs_log_item_init(struct xfs_mount *mp, struct xfs_log_item *item,
 			  int type, const struct xfs_item_ops *ops);
+void	xfs_log_item_get(struct xfs_log_item *lip);
+bool	xfs_log_item_put(struct xfs_log_item *lip);
+bool	xfs_log_item_get_safe(struct xfs_log_item *lip);
 
 /*
  * Return values for the iop_push() routines.
-- 
2.55.0

[RFC PATCH 2/2] xfs: change xfs_trans_ail_delete return type to bool

[Jeffin Philip]

Change xfs_trans_ail_delete's return type from void to
bool which returns true if the item was removed from
AIL and false in case of shutdown(item not in AIL).
Reflect the change in header file too.

Signed-off-by: Jeffin Philip <jeffinphilip14@gmail.com>
---
 fs/xfs/xfs_trans_ail.c  | 5 +++--
 fs/xfs/xfs_trans_priv.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
index 99a9bf3762b7..b17b677586a0 100644
--- a/fs/xfs/xfs_trans_ail.c
+++ b/fs/xfs/xfs_trans_ail.c
@@ -913,7 +913,7 @@ xfs_ail_delete_one(
 	return 0;
 }
 
-void
+bool
 xfs_trans_ail_delete(
 	struct xfs_log_item	*lip,
 	int			shutdown_type)
@@ -931,12 +931,13 @@ xfs_trans_ail_delete(
 					__func__);
 			xlog_force_shutdown(log, shutdown_type);
 		}
-		return;
+		return false;
 	}
 
 	clear_bit(XFS_LI_FAILED, &lip->li_flags);
 	tail_lsn = xfs_ail_delete_one(ailp, lip);
 	xfs_ail_update_finish(ailp, tail_lsn);	/* drops the AIL lock */
+	return true;
 }
 
 int
diff --git a/fs/xfs/xfs_trans_priv.h b/fs/xfs/xfs_trans_priv.h
index f945f0450b16..b00c803941c7 100644
--- a/fs/xfs/xfs_trans_priv.h
+++ b/fs/xfs/xfs_trans_priv.h
@@ -100,7 +100,7 @@ void xfs_trans_ail_insert(struct xfs_ail *ailp, struct xfs_log_item *lip,
 xfs_lsn_t xfs_ail_delete_one(struct xfs_ail *ailp, struct xfs_log_item *lip);
 void xfs_ail_update_finish(struct xfs_ail *ailp, xfs_lsn_t old_lsn)
 			__releases(ailp->ail_lock);
-void xfs_trans_ail_delete(struct xfs_log_item *lip, int shutdown_type);
+bool xfs_trans_ail_delete(struct xfs_log_item *lip, int shutdown_type);
 
 static inline void xfs_ail_push(struct xfs_ail *ailp)
 {
-- 
2.55.0

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

[Dave Chinner]

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

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

[Jeffin Philip]

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.