[PATCH v2 0/2] xfs: restore nofs context unconditionally in xfs_trans_roll

[PATCH v2 0/2] xfs: restore nofs context unconditionally in xfs_trans_roll

[Yun Zhou]

xfs_trans_roll() loses the NOFS allocation context on the error path,
causing a circular lock dependency between xfs_nondir_ilock_class and
fs_reclaim reported by syzbot.  Fix this by transferring the nofs
context from the old transaction to the new one in xfs_trans_dup(),
so it remains active regardless of commit success or failure.

v2:
  - Instead of moving xfs_trans_set_context() before the error check,
    transfer the nofs context in xfs_trans_dup() via a new
    memalloc_flags_move() helper, as suggested by Darrick.
  - Change t_pflags from unsigned long to unsigned int to match mm API.

v1: https://lore.kernel.org/all/20260713035505.1635191-1-yun.zhou@windriver.com/T/

Yun Zhou (2):
  mm: introduce memalloc_flags_move() for transferring allocation scopes
  xfs: restore nofs context unconditionally in xfs_trans_roll

 fs/xfs/xfs_trans.c       |  9 +++------
 fs/xfs/xfs_trans.h       |  2 +-
 include/linux/sched/mm.h | 17 +++++++++++++++++
 3 files changed, 21 insertions(+), 7 deletions(-)

-- 
2.43.0

[PATCH v2 1/2] mm: introduce memalloc_flags_move() for transferring allocation scopes

[Yun Zhou]

Add memalloc_flags_move() to transfer a saved memalloc scope from one
tracking variable to another.  The source is zeroed so that a
subsequent memalloc_flags_restore() on it becomes a no-op, effectively
transferring ownership of the scope to the destination.

This is needed when a subsystem hands off an allocation context from
one structure to another (e.g. during transaction rolling in
filesystems), and needs to ensure the scope remains active without an
extra save/restore cycle.

Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 include/linux/sched/mm.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/include/linux/sched/mm.h b/include/linux/sched/mm.h
index 95d0040df584..a6de84b70c0a 100644
--- a/include/linux/sched/mm.h
+++ b/include/linux/sched/mm.h
@@ -342,6 +342,23 @@ static inline void memalloc_flags_restore(unsigned flags)
 	current->flags &= ~flags;
 }
 
+/**
+ * memalloc_flags_move - transfer a memalloc scope from one tracking
+ * variable to another.
+ * @old_flags: pointer to the source flags (will be zeroed)
+ *
+ * Returns the flags value to store in the destination.  The source is
+ * set to zero so that a subsequent memalloc_flags_restore() on it is
+ * a no-op.
+ */
+static inline unsigned int memalloc_flags_move(unsigned int *old_flags)
+{
+	unsigned int ret = *old_flags;
+
+	*old_flags = 0;
+	return ret;
+}
+
 /**
  * memalloc_noio_save - Marks implicit GFP_NOIO allocation scope.
  *
-- 
2.43.0

[PATCH v2 2/2] xfs: restore nofs context unconditionally in xfs_trans_roll

[Yun Zhou]

When __xfs_trans_commit() fails in xfs_trans_roll(), the NOFS context
is cleared but only restored in the success path.  This leaves the
error path without nofs protection, causing a circular lock dependency
between xfs_nondir_ilock_class and fs_reclaim:

       CPU0                    CPU1
       ----                    ----
  lock(&xfs_nondir_ilock_class);
                               lock(fs_reclaim);
                               lock(&xfs_nondir_ilock_class);
  lock(fs_reclaim);

Fix this by transferring the nofs context from the old transaction to
the new one in xfs_trans_dup() via memalloc_flags_move(), so it remains
active throughout the entire roll sequence regardless of commit success
or failure.

Reported-by: syzbot+59178abfeb0ea3f0ab20@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=59178abfeb0ea3f0ab20
Fixes: a1ca658d649a ("xfs: fix incorrect context handling in xfs_trans_roll")
Suggested-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/xfs/xfs_trans.c | 9 +++------
 fs/xfs/xfs_trans.h | 2 +-
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 7bfbd9f6f0df..89818c67b64f 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -127,6 +127,9 @@ xfs_trans_dup(
 	/* move deferred ops over to the new tp */
 	xfs_defer_move(ntp, tp);
 
+	/* move the nofs context to the new transaction */
+	ntp->t_pflags = memalloc_flags_move(&tp->t_pflags);
+
 	xfs_trans_dup_dqinfo(tp, ntp);
 	return ntp;
 }
@@ -1041,12 +1044,6 @@ xfs_trans_roll(
 	 * locked be logged in the prior and the next transactions.
 	 */
 	tp = *tpp;
-	/*
-	 * __xfs_trans_commit cleared the NOFS flag by calling into
-	 * xfs_trans_free.  Set it again here before doing memory
-	 * allocations.
-	 */
-	xfs_trans_set_context(tp);
 	error = xfs_log_regrant(tp->t_mountp, tp->t_ticket);
 	if (error)
 		return error;
diff --git a/fs/xfs/xfs_trans.h b/fs/xfs/xfs_trans.h
index eb83c5dac032..fd792584275a 100644
--- a/fs/xfs/xfs_trans.h
+++ b/fs/xfs/xfs_trans.h
@@ -152,7 +152,7 @@ typedef struct xfs_trans {
 	struct list_head	t_items;	/* log item descriptors */
 	struct list_head	t_busy;		/* list of busy extents */
 	struct list_head	t_dfops;	/* deferred operations */
-	unsigned long		t_pflags;	/* saved process flags state */
+	unsigned int		t_pflags;	/* saved process flags state */
 } xfs_trans_t;
 
 /*
-- 
2.43.0

Re: [PATCH v2 1/2] mm: introduce memalloc_flags_move() for transferring allocation scopes

[Andrew Morton]

On Sun, 19 Jul 2026 17:57:31 +0800 Yun Zhou <yun.zhou@windriver.com> wrote:

> Add memalloc_flags_move() to transfer a saved memalloc scope from one
> tracking variable to another.  The source is zeroed so that a
> subsequent memalloc_flags_restore() on it becomes a no-op, effectively
> transferring ownership of the scope to the destination.
> 
> This is needed when a subsystem hands off an allocation context from
> one structure to another (e.g. during transaction rolling in
> filesystems), and needs to ensure the scope remains active without an
> extra save/restore cycle.
> 
> ...
>

Thanks.  fyi, Sashiko might have found a pre-existing xfs issue:
	https://sashiko.dev/#/patchset/20260719095732.1813590-1-yun.zhou@windriver.com

>  include/linux/sched/mm.h | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)

This file isn't mentioned in MAINTAINERS.  Could someone please propose
a patch?

From the MM side, probably just place it in every record which mentions
include/linux/mm.h - close enough.

> --- a/include/linux/sched/mm.h
> +++ b/include/linux/sched/mm.h
> @@ -342,6 +342,23 @@ static inline void memalloc_flags_restore(unsigned flags)
>  	current->flags &= ~flags;
>  }
>  
> +/**
> + * memalloc_flags_move - transfer a memalloc scope from one tracking
> + * variable to another.
> + * @old_flags: pointer to the source flags (will be zeroed)
> + *
> + * Returns the flags value to store in the destination.  The source is
> + * set to zero so that a subsequent memalloc_flags_restore() on it is
> + * a no-op.
> + */
> +static inline unsigned int memalloc_flags_move(unsigned int *old_flags)
> +{
> +	unsigned int ret = *old_flags;
> +
> +	*old_flags = 0;
> +	return ret;
> +}
> +

I've added linux-mm to cc here.  Please include it in any future
versions of this patchset.

Re: [PATCH v2 1/2] mm: introduce memalloc_flags_move() for transferring allocation scopes

[Zhou, Yun]

On 7/20/26 04:57, Andrew Morton wrote:
> On Sun, 19 Jul 2026 17:57:31 +0800 Yun Zhou <yun.zhou@windriver.com> wrote:
> 
>> Add memalloc_flags_move() to transfer a saved memalloc scope from one
>> tracking variable to another.  The source is zeroed so that a
>> subsequent memalloc_flags_restore() on it becomes a no-op, effectively
>> transferring ownership of the scope to the destination.
>>
>> This is needed when a subsystem hands off an allocation context from
>> one structure to another (e.g. during transaction rolling in
>> filesystems), and needs to ensure the scope remains active without an
>> extra save/restore cycle.
>>
>> ...
>>
> 
> Thanks.  fyi, Sashiko might have found a pre-existing xfs issue:
>          https://sashiko.dev/#/patchset/20260719095732.1813590-1-yun.zhou@windriver.com
> 

Good catch. This is indeed a pre-existing issue, and needs a separate fix.

>>   include/linux/sched/mm.h | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
> 
> This file isn't mentioned in MAINTAINERS.  Could someone please propose
> a patch?
> 
>  From the MM side, probably just place it in every record which mentions
> include/linux/mm.h - close enough.
> 
>> --- a/include/linux/sched/mm.h
>> +++ b/include/linux/sched/mm.h
>> @@ -342,6 +342,23 @@ static inline void memalloc_flags_restore(unsigned flags)
>>        current->flags &= ~flags;
>>   }
>>
>> +/**
>> + * memalloc_flags_move - transfer a memalloc scope from one tracking
>> + * variable to another.
>> + * @old_flags: pointer to the source flags (will be zeroed)
>> + *
>> + * Returns the flags value to store in the destination.  The source is
>> + * set to zero so that a subsequent memalloc_flags_restore() on it is
>> + * a no-op.
>> + */
>> +static inline unsigned int memalloc_flags_move(unsigned int *old_flags)
>> +{
>> +     unsigned int ret = *old_flags;
>> +
>> +     *old_flags = 0;
>> +     return ret;
>> +}
>> +
> 
> I've added linux-mm to cc here.  Please include it in any future
> versions of this patchset.
> 

Well, got it.

Thanks,
Yun

Re: [PATCH v2 2/2] xfs: restore nofs context unconditionally in xfs_trans_roll

[Christoph Hellwig]

On Sun, Jul 19, 2026 at 05:57:32PM +0800, Yun Zhou wrote:
> When __xfs_trans_commit() fails in xfs_trans_roll(), the NOFS context
> is cleared but only restored in the success path.  This leaves the
> error path without nofs protection, causing a circular lock dependency
> between xfs_nondir_ilock_class and fs_reclaim:
> 
>        CPU0                    CPU1
>        ----                    ----
>   lock(&xfs_nondir_ilock_class);
>                                lock(fs_reclaim);
>                                lock(&xfs_nondir_ilock_class);
>   lock(fs_reclaim);
> 
> Fix this by transferring the nofs context from the old transaction to
> the new one in xfs_trans_dup() via memalloc_flags_move(), so it remains
> active throughout the entire roll sequence regardless of commit success
> or failure.

Good find, but we keep adding more magic here that will just lead
to further bugs.  Your previous version was much better, and could
be improved by also removing the hiding under xfs_trans_set_context as
a follow on. 

Having a maze of single or two caller magic macros that modify global
(or rather global-ish as it's per thread) state makes hairy bits like
this really error prone as seen by the bugs now found.