[PATCH v3] xfs: restore nofs context unconditionally in xfs_trans_roll

[PATCH v3] 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 moving xfs_trans_set_context() before the error check so
that nofs context is always restored on the new transaction.

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")
Cc: stable@vger.kernel.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
v3:
  - Change back to v1

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/

 fs/xfs/xfs_trans.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 7bfbd9f6f0df..1b36cf12d4e3 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -1029,6 +1029,15 @@ xfs_trans_roll(
 	 * duplicate transaction that gets returned.
 	 */
 	error = __xfs_trans_commit(tp, true);
+
+	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);
+
 	if (error)
 		return error;
 
@@ -1040,13 +1049,6 @@ xfs_trans_roll(
 	 * either nothing be locked across this call, or that anything that is
 	 * 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;
-- 
2.43.0

Re: [PATCH v3] xfs: restore nofs context unconditionally in xfs_trans_roll

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH v3] xfs: restore nofs context unconditionally in xfs_trans_roll

[Darrick J. Wong]

On Thu, Jul 30, 2026 at 05:30:02PM +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 moving xfs_trans_set_context() before the error check so
> that nofs context is always restored on the new transaction.
> 
> 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")
> Cc: stable@vger.kernel.org
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>

Looks good to me,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
> v3:
>   - Change back to v1
> 
> 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/
> 
>  fs/xfs/xfs_trans.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
> index 7bfbd9f6f0df..1b36cf12d4e3 100644
> --- a/fs/xfs/xfs_trans.c
> +++ b/fs/xfs/xfs_trans.c
> @@ -1029,6 +1029,15 @@ xfs_trans_roll(
>  	 * duplicate transaction that gets returned.
>  	 */
>  	error = __xfs_trans_commit(tp, true);
> +
> +	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);
> +
>  	if (error)
>  		return error;
>  
> @@ -1040,13 +1049,6 @@ xfs_trans_roll(
>  	 * either nothing be locked across this call, or that anything that is
>  	 * 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;
> -- 
> 2.43.0
> 
> 

Re: [PATCH v3] xfs: restore nofs context unconditionally in xfs_trans_roll

[Carlos Maiolino]

On Thu, 30 Jul 2026 17:30:02 +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);
> 
> [...]

Applied to for-next, thanks!

[1/1] xfs: restore nofs context unconditionally in xfs_trans_roll
      commit: 0241ea5fb0fe86d2a673163b2f5815111aadc7f7

Best regards,
-- 
Carlos Maiolino <cem@kernel.org>