* [PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents
2026-03-10 12:33 [PATCH 0/3] Decouple busy_extents from cil context cem
@ 2026-03-10 12:33 ` cem
2026-03-10 13:03 ` Christoph Hellwig
2026-03-10 12:33 ` [PATCH 2/3] xfs: convert busy_extents list to a pointer within cil context cem
2026-03-10 12:33 ` [PATCH 3/3] xfs: remove owner field from xfs_extent_busy cem
2 siblings, 1 reply; 7+ messages in thread
From: cem @ 2026-03-10 12:33 UTC (permalink / raw)
To: linux-xfs; +Cc: david, hch, djwong
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents
2026-03-10 12:33 ` [PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents cem
@ 2026-03-10 13:03 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-03-10 13:03 UTC (permalink / raw)
To: cem; +Cc: linux-xfs, david, hch, djwong
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/3] xfs: convert busy_extents list to a pointer within cil context
2026-03-10 12:33 [PATCH 0/3] Decouple busy_extents from cil context cem
2026-03-10 12:33 ` [PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents cem
@ 2026-03-10 12:33 ` cem
2026-03-10 13:04 ` Christoph Hellwig
2026-03-10 12:33 ` [PATCH 3/3] xfs: remove owner field from xfs_extent_busy cem
2 siblings, 1 reply; 7+ messages in thread
From: cem @ 2026-03-10 12:33 UTC (permalink / raw)
To: linux-xfs; +Cc: david, hch, djwong
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] xfs: remove owner field from xfs_extent_busy
2026-03-10 12:33 [PATCH 0/3] Decouple busy_extents from cil context cem
2026-03-10 12:33 ` [PATCH 1/3] xfs: add a couple helpers to alloc/free xfs_busy_extents cem
2026-03-10 12:33 ` [PATCH 2/3] xfs: convert busy_extents list to a pointer within cil context cem
@ 2026-03-10 12:33 ` cem
2026-03-10 13:06 ` Christoph Hellwig
2 siblings, 1 reply; 7+ messages in thread
From: cem @ 2026-03-10 12:33 UTC (permalink / raw)
To: linux-xfs; +Cc: david, hch, djwong
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 3/3] xfs: remove owner field from xfs_extent_busy
2026-03-10 12:33 ` [PATCH 3/3] xfs: remove owner field from xfs_extent_busy cem
@ 2026-03-10 13:06 ` Christoph Hellwig
0 siblings, 0 replies; 7+ messages in thread
From: Christoph Hellwig @ 2026-03-10 13:06 UTC (permalink / raw)
To: cem; +Cc: linux-xfs, david, hch, djwong
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.
^ permalink raw reply [flat|nested] 7+ messages in thread