[PATCH 0/3] Decouple busy_extents from cil context

[PATCH 0/3] Decouple busy_extents from cil context

[cem]

From: Carlos Maiolino <cem@kernel.org>

This series belongs to some work I am doing on AIL aiming to improve its
writeback performance.

I need the cil context to be treated as a single unit during AIL
processing, and having busy_extents associated with it gets in the way.

This has passed the same tests I run during patch integration without
regressions.

Instead of keeping rebasing it on top of newer versions, I'd appreciate
I could get reviews and upstream this part now, to reduce the amount of
changes I need to rebase.

I should get the next part ready soon once I fix a few issues on it.

Carlos Maiolino (3):
  xfs: add a couple helpers to alloc/free xfs_busy_extents
  xfs: convert busy_extents list to a pointer within cil context
  xfs: remove owner field from xfs_extent_busy

 fs/xfs/xfs_discard.c     | 17 ++++++-----------
 fs/xfs/xfs_extent_busy.h | 24 +++++++++++++++++-------
 fs/xfs/xfs_log_cil.c     | 18 ++++++++----------
 fs/xfs/xfs_log_priv.h    |  2 +-
 4 files changed, 32 insertions(+), 29 deletions(-)

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
-- 
2.53.0

[PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents

[cem]

From: Carlos Maiolino <cem@kernel.org>

There are a couple locations which repeats the same code pattern
to alloc/free the xfs_busy_extents list

These helpers will come in handy when decoupling busy_extents
list from the cil context.

Notice though commit bf4afc53b77a removed GFP_KERNEL from the kzalloc_obj()
calls due to the new default argument.
On future usage we'll also need to pass NOFAIL here, so we need to be able
to pass some flags as argument.

The avoid calls like xfs_busy_extents_alloc(0) explicitly pass GFP_KERNEL
as argument. It looks better to me.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_discard.c     | 12 +++++-------
 fs/xfs/xfs_extent_busy.h | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index 906221ffe9ba..f393be78dc46 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -349,18 +349,17 @@ xfs_trim_perag_extents(
 	do {
 		struct xfs_busy_extents	*extents;
 
-		extents = kzalloc_obj(*extents);
+		extents = xfs_busy_extents_alloc(GFP_KERNEL);
 		if (!extents) {
 			error = -ENOMEM;
 			break;
 		}
 
 		extents->owner = extents;
-		INIT_LIST_HEAD(&extents->extent_list);
 
 		error = xfs_trim_gather_extents(pag, &tcur, extents);
 		if (error) {
-			kfree(extents);
+			xfs_busy_extents_free(extents);
 			break;
 		}
 
@@ -689,7 +688,7 @@ xfs_trim_rtgroup_extents(
 	 * trims the extents returned.
 	 */
 	do {
-		tr.extents = kzalloc_obj(*tr.extents);
+		tr.extents = xfs_busy_extents_alloc(GFP_KERNEL);
 		if (!tr.extents) {
 			error = -ENOMEM;
 			break;
@@ -698,7 +697,6 @@ xfs_trim_rtgroup_extents(
 		tr.queued = 0;
 		tr.batch = XFS_DISCARD_MAX_EXAMINE;
 		tr.extents->owner = tr.extents;
-		INIT_LIST_HEAD(&tr.extents->extent_list);
 
 		xfs_rtgroup_lock(rtg, XFS_RTGLOCK_BITMAP_SHARED);
 		error = xfs_rtalloc_query_range(rtg, tp, low, high,
@@ -707,12 +705,12 @@ xfs_trim_rtgroup_extents(
 		if (error == -ECANCELED)
 			error = 0;
 		if (error) {
-			kfree(tr.extents);
+			xfs_busy_extents_free(tr.extents);
 			break;
 		}
 
 		if (!tr.queued) {
-			kfree(tr.extents);
+			xfs_busy_extents_free(tr.extents);
 			break;
 		}
 
diff --git a/fs/xfs/xfs_extent_busy.h b/fs/xfs/xfs_extent_busy.h
index 3e6e019b6146..699f97a53530 100644
--- a/fs/xfs/xfs_extent_busy.h
+++ b/fs/xfs/xfs_extent_busy.h
@@ -43,6 +43,23 @@ struct xfs_busy_extents {
 	void			*owner;
 };
 
+static inline struct xfs_busy_extents *
+xfs_busy_extents_alloc(gfp_t flags)
+{
+	struct xfs_busy_extents *e;
+	e = kzalloc_obj(*e, flags);
+
+	if (e)
+		INIT_LIST_HEAD(&e->extent_list);
+	return e;
+}
+
+static inline void xfs_busy_extents_free(
+	struct xfs_busy_extents *e)
+{
+	kfree(e);
+}
+
 void xfs_extent_busy_insert(struct xfs_trans *tp, struct xfs_group *xg,
 		xfs_agblock_t bno, xfs_extlen_t len, unsigned int flags);
 void xfs_extent_busy_insert_discard(struct xfs_group *xg, xfs_agblock_t bno,
-- 
2.53.0

[PATCH 2/3] xfs: convert busy_extents list to a pointer within cil context

[cem]

From: Carlos Maiolino <cem@kernel.org>

Decouple the busy extents list from the cil context.

By having the object independently allocated, we can transfer
its ownership to the discard code.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_discard.c  |  1 +
 fs/xfs/xfs_log_cil.c  | 17 ++++++++++-------
 fs/xfs/xfs_log_priv.h |  2 +-
 3 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index f393be78dc46..2db0ea6eedc6 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -86,6 +86,7 @@ xfs_discard_endio_work(
 
 	xfs_extent_busy_clear(&extents->extent_list, false);
 	kfree(extents->owner);
+	xfs_busy_extents_free(extents);
 }
 
 /*
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index edc368938f30..bb71b5ad6b55 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -101,8 +101,8 @@ xlog_cil_ctx_alloc(void)
 	struct xfs_cil_ctx	*ctx;
 
 	ctx = kzalloc_obj(*ctx, GFP_KERNEL | __GFP_NOFAIL);
+	ctx->busy_extents = xfs_busy_extents_alloc(GFP_KERNEL | __GFP_NOFAIL);
 	INIT_LIST_HEAD(&ctx->committing);
-	INIT_LIST_HEAD(&ctx->busy_extents.extent_list);
 	INIT_LIST_HEAD(&ctx->log_items);
 	INIT_LIST_HEAD(&ctx->lv_chain);
 	INIT_WORK(&ctx->push_work, xlog_cil_push_work);
@@ -131,7 +131,7 @@ xlog_cil_push_pcp_aggregate(
 
 		if (!list_empty(&cilpcp->busy_extents)) {
 			list_splice_init(&cilpcp->busy_extents,
-					&ctx->busy_extents.extent_list);
+					&ctx->busy_extents->extent_list);
 		}
 		if (!list_empty(&cilpcp->log_items))
 			list_splice_init(&cilpcp->log_items, &ctx->log_items);
@@ -994,8 +994,8 @@ xlog_cil_committed(
 
 	xlog_cil_ail_insert(ctx, abort);
 
-	xfs_extent_busy_sort(&ctx->busy_extents.extent_list);
-	xfs_extent_busy_clear(&ctx->busy_extents.extent_list,
+	xfs_extent_busy_sort(&ctx->busy_extents->extent_list);
+	xfs_extent_busy_clear(&ctx->busy_extents->extent_list,
 			      xfs_has_discard(mp) && !abort);
 
 	spin_lock(&ctx->cil->xc_push_lock);
@@ -1004,12 +1004,13 @@ xlog_cil_committed(
 
 	xlog_cil_free_logvec(&ctx->lv_chain);
 
-	if (!list_empty(&ctx->busy_extents.extent_list)) {
-		ctx->busy_extents.owner = ctx;
-		xfs_discard_extents(mp, &ctx->busy_extents);
+	if (!list_empty(&ctx->busy_extents->extent_list)) {
+		ctx->busy_extents->owner = ctx;
+		xfs_discard_extents(mp, ctx->busy_extents);
 		return;
 	}
 
+	xfs_busy_extents_free(ctx->busy_extents);
 	kfree(ctx);
 }
 
@@ -1595,6 +1596,7 @@ xlog_cil_push_work(
 out_skip:
 	up_write(&cil->xc_ctx_lock);
 	xfs_log_ticket_put(new_ctx->ticket);
+	xfs_busy_extents_free(new_ctx->busy_extents);
 	kfree(new_ctx);
 	memalloc_nofs_restore(nofs_flags);
 	return;
@@ -2058,6 +2060,7 @@ xlog_cil_destroy(
 	if (cil->xc_ctx) {
 		if (cil->xc_ctx->ticket)
 			xfs_log_ticket_put(cil->xc_ctx->ticket);
+		xfs_busy_extents_free(cil->xc_ctx->busy_extents);
 		kfree(cil->xc_ctx);
 	}
 
diff --git a/fs/xfs/xfs_log_priv.h b/fs/xfs/xfs_log_priv.h
index cf1e4ce61a8c..b0f9923f30cc 100644
--- a/fs/xfs/xfs_log_priv.h
+++ b/fs/xfs/xfs_log_priv.h
@@ -240,7 +240,7 @@ struct xfs_cil_ctx {
 	struct xlog_in_core	*commit_iclog;
 	struct xlog_ticket	*ticket;	/* chkpt ticket */
 	atomic_t		space_used;	/* aggregate size of regions */
-	struct xfs_busy_extents	busy_extents;
+	struct xfs_busy_extents	*busy_extents;
 	struct list_head	log_items;	/* log items in chkpt */
 	struct list_head	lv_chain;	/* logvecs being pushed */
 	struct list_head	iclog_entry;
-- 
2.53.0

[PATCH 3/3] xfs: remove owner field from xfs_extent_busy

[cem]

From: Carlos Maiolino <cem@kernel.org>

Now that xfs_busy_extents is not embedded into a cil context, we can
get rid of the owner field.

The struct will be freed upon a call to xfs_discard_extents(), whether
at IO completion or by direct calling xfs_discard_endio_work().

What reamains is xlog_cil_commited now being responsible for freeing
the context itself.

Worth mentioning that the busy extents still need to be manually freed
during filesystem shutdown via xlog_cil_destroy() if there is a
remaining context there.

Also, xlog_cil_committed() now unconditionally calls xfs_discard_extents.
This seems safe because it can handle an empty list, and as a bonus,
it will call xfs_discard_endio_work() for such case, without any extra
wiring needed to free the busy extents list in case it is empty.
xfs_discard_endio_work() is also able to properly handle the empty list.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 fs/xfs/xfs_discard.c     |  4 ----
 fs/xfs/xfs_extent_busy.h |  7 -------
 fs/xfs/xfs_log_cil.c     | 13 ++++---------
 3 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
index 2db0ea6eedc6..be4bc16dc85c 100644
--- a/fs/xfs/xfs_discard.c
+++ b/fs/xfs/xfs_discard.c
@@ -85,7 +85,6 @@ xfs_discard_endio_work(
 		container_of(work, struct xfs_busy_extents, endio_work);
 
 	xfs_extent_busy_clear(&extents->extent_list, false);
-	kfree(extents->owner);
 	xfs_busy_extents_free(extents);
 }
 
@@ -356,8 +355,6 @@ xfs_trim_perag_extents(
 			break;
 		}
 
-		extents->owner = extents;
-
 		error = xfs_trim_gather_extents(pag, &tcur, extents);
 		if (error) {
 			xfs_busy_extents_free(extents);
@@ -697,7 +694,6 @@ xfs_trim_rtgroup_extents(
 
 		tr.queued = 0;
 		tr.batch = XFS_DISCARD_MAX_EXAMINE;
-		tr.extents->owner = tr.extents;
 
 		xfs_rtgroup_lock(rtg, XFS_RTGLOCK_BITMAP_SHARED);
 		error = xfs_rtalloc_query_range(rtg, tp, low, high,
diff --git a/fs/xfs/xfs_extent_busy.h b/fs/xfs/xfs_extent_busy.h
index 699f97a53530..5deaa1fbe378 100644
--- a/fs/xfs/xfs_extent_busy.h
+++ b/fs/xfs/xfs_extent_busy.h
@@ -34,13 +34,6 @@ struct xfs_extent_busy {
 struct xfs_busy_extents {
 	struct list_head	extent_list;
 	struct work_struct	endio_work;
-
-	/*
-	 * Owner is the object containing the struct xfs_busy_extents to free
-	 * once the busy extents have been processed. If only the
-	 * xfs_busy_extents object needs freeing, then point this at itself.
-	 */
-	void			*owner;
 };
 
 static inline struct xfs_busy_extents *
diff --git a/fs/xfs/xfs_log_cil.c b/fs/xfs/xfs_log_cil.c
index bb71b5ad6b55..befc2691f053 100644
--- a/fs/xfs/xfs_log_cil.c
+++ b/fs/xfs/xfs_log_cil.c
@@ -976,6 +976,7 @@ xlog_cil_committed(
 	struct xfs_cil_ctx	*ctx)
 {
 	struct xfs_mount	*mp = ctx->cil->xc_log->l_mp;
+	struct xfs_busy_extents	*busy_extents = ctx->busy_extents;
 	bool			abort = xlog_is_shutdown(ctx->cil->xc_log);
 
 	/*
@@ -994,8 +995,8 @@ xlog_cil_committed(
 
 	xlog_cil_ail_insert(ctx, abort);
 
-	xfs_extent_busy_sort(&ctx->busy_extents->extent_list);
-	xfs_extent_busy_clear(&ctx->busy_extents->extent_list,
+	xfs_extent_busy_sort(&busy_extents->extent_list);
+	xfs_extent_busy_clear(&busy_extents->extent_list,
 			      xfs_has_discard(mp) && !abort);
 
 	spin_lock(&ctx->cil->xc_push_lock);
@@ -1004,13 +1005,7 @@ xlog_cil_committed(
 
 	xlog_cil_free_logvec(&ctx->lv_chain);
 
-	if (!list_empty(&ctx->busy_extents->extent_list)) {
-		ctx->busy_extents->owner = ctx;
-		xfs_discard_extents(mp, ctx->busy_extents);
-		return;
-	}
-
-	xfs_busy_extents_free(ctx->busy_extents);
+	xfs_discard_extents(mp, busy_extents);
 	kfree(ctx);
 }
 
-- 
2.53.0

Re: [PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 01:33:14PM +0100, cem@kernel.org wrote:
> The avoid calls like xfs_busy_extents_alloc(0) explicitly pass GFP_KERNEL
> as argument. It looks better to me.

You can't really pass 0 anyway, so I think this part of the commit
log doesn't make much sense.

> +static inline struct xfs_busy_extents *
> +xfs_busy_extents_alloc(gfp_t flags)
> +{
> +	struct xfs_busy_extents *e;
> +	e = kzalloc_obj(*e, flags);

Please keep an empty line between declarations and code.  And in this
case you could probably move the initialization to the declaration line
as well.
>
> +	if (e)
> +		INIT_LIST_HEAD(&e->extent_list);
> +	return e;
> +}
> +
> +static inline void xfs_busy_extents_free(
> +	struct xfs_busy_extents *e)

This is inconsistent with the xfs_busy_extents_alloc prototype above,
which uses the mormal kernel style, which we sometimes do for inlines,
but also doesn't use the XFS style with the return type on a separate
line and the argument names tab indented.

Re: [PATCH 2/3] xfs: convert busy_extents list to a pointer within cil context

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 01:33:15PM +0100, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> Decouple the busy extents list from the cil context.
> 
> By having the object independently allocated, we can transfer
> its ownership to the discard code.

This looks fine, but I don't really understand the rationale here.

Re: [PATCH 3/3] xfs: remove owner field from xfs_extent_busy

[Christoph Hellwig]

On Tue, Mar 10, 2026 at 01:33:16PM +0100, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> Now that xfs_busy_extents is not embedded into a cil context, we can
> get rid of the owner field.
> 
> The struct will be freed upon a call to xfs_discard_extents(), whether
> at IO completion or by direct calling xfs_discard_endio_work().
> 
> What reamains is xlog_cil_commited now being responsible for freeing
> the context itself.
> 
> Worth mentioning that the busy extents still need to be manually freed
> during filesystem shutdown via xlog_cil_destroy() if there is a
> remaining context there.
> 
> Also, xlog_cil_committed() now unconditionally calls xfs_discard_extents.
> This seems safe because it can handle an empty list, and as a bonus,
> it will call xfs_discard_endio_work() for such case, without any extra
> wiring needed to free the busy extents list in case it is empty.
> xfs_discard_endio_work() is also able to properly handle the empty list.

Looking at this series, it feels almost like this should be mostly
merged with the previous patch, with a small caveat:

>  
>  	xlog_cil_free_logvec(&ctx->lv_chain);
>  
> -	if (!list_empty(&ctx->busy_extents->extent_list)) {
> -		ctx->busy_extents->owner = ctx;
> -		xfs_discard_extents(mp, ctx->busy_extents);
> -		return;
> -	}
> -
> -	xfs_busy_extents_free(ctx->busy_extents);
> +	xfs_discard_extents(mp, busy_extents);

I'd probably split this part out as it really neeѕs an explanation
and seems to be something taking advantage of the new dynamic
allocation scheme.