[PATCH] xfs: restore nofs context unconditionally in xfs_trans_roll

[PATCH] 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")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 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] xfs: restore nofs context unconditionally in xfs_trans_roll

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 11:55:05AM +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")
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
> ---
>  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);

The tp assignment above now returns the incorrect transaction when
__xfs_trans_commit fails, so you can't do this.

Otherwise yes, this call should move up.  I don't really see how
it fixes the syzbot report, though.

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

[Zhou, Yun]

On 7/13/26 17:09, Christoph Hellwig wrote:
> 
> On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
>>
>> 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);
> 
> The tp assignment above now returns the incorrect transaction when
> __xfs_trans_commit fails, so you can't do this.
> 
> Otherwise yes, this call should move up.  I don't really see how
> it fixes the syzbot report, though.
> 

Thank you very much for your reply. The tp here is a local variable only 
used for convenience within the function. The caller always gets the new 
transaction through *tpp, which was set by xfs_trans_dup() before the 
commit call. Moving tp = *tpp before the error check doesn't change what 
the caller sees - *tpp still points to the new (dup'd) transaction 
regardless.

If I have overlooked any edge cases, please point them out.

BR,
Yun

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

[Christoph Hellwig]

On Mon, Jul 13, 2026 at 06:06:38PM +0800, Zhou, Yun wrote:
> 
> 
> On 7/13/26 17:09, Christoph Hellwig wrote:
> > 
> > On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
> > > 
> > > 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);
> > 
> > The tp assignment above now returns the incorrect transaction when
> > __xfs_trans_commit fails, so you can't do this.
> > 
> > Otherwise yes, this call should move up.  I don't really see how
> > it fixes the syzbot report, though.
> > 
> 
> Thank you very much for your reply. The tp here is a local variable only
> used for convenience within the function. The caller always gets the new
> transaction through *tpp, which was set by xfs_trans_dup() before the commit
> call. Moving tp = *tpp before the error check doesn't change what the caller
> sees - *tpp still points to the new (dup'd) transaction regardless.

Ah, right.  Tis should be fine:

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

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

[Darrick J. Wong]

On Mon, Jul 13, 2026 at 06:28:38AM -0700, Christoph Hellwig wrote:
> On Mon, Jul 13, 2026 at 06:06:38PM +0800, Zhou, Yun wrote:
> > 
> > 
> > On 7/13/26 17:09, Christoph Hellwig wrote:
> > > 
> > > On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
> > > > 
> > > > 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);
> > > 
> > > The tp assignment above now returns the incorrect transaction when
> > > __xfs_trans_commit fails, so you can't do this.
> > > 
> > > Otherwise yes, this call should move up.  I don't really see how
> > > it fixes the syzbot report, though.
> > > 
> > 
> > Thank you very much for your reply. The tp here is a local variable only
> > used for convenience within the function. The caller always gets the new
> > transaction through *tpp, which was set by xfs_trans_dup() before the commit
> > call. Moving tp = *tpp before the error check doesn't change what the caller
> > sees - *tpp still points to the new (dup'd) transaction regardless.
> 
> Ah, right.  Tis should be fine:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Why not move tp_pflags to the new transaction in xfs_trans_dup like we
do for the deferred item list:

	/* move deferred ops over to the new tp */
	xfs_defer_move(ntp, tp);

	ntp->t_pflags = tp->t_pflags;
	tp->t_pflags = 0;

--D

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

[Zhou, Yun]

On 7/14/26 07:04, Darrick J. Wong wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> On Mon, Jul 13, 2026 at 06:28:38AM -0700, Christoph Hellwig wrote:
>> On Mon, Jul 13, 2026 at 06:06:38PM +0800, Zhou, Yun wrote:
>>>
>>>
>>> On 7/13/26 17:09, Christoph Hellwig wrote:
>>>>
>>>> On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
>>>>>
>>>>> 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);
>>>>
>>>> The tp assignment above now returns the incorrect transaction when
>>>> __xfs_trans_commit fails, so you can't do this.
>>>>
>>>> Otherwise yes, this call should move up.  I don't really see how
>>>> it fixes the syzbot report, though.
>>>>
>>>
>>> Thank you very much for your reply. The tp here is a local variable only
>>> used for convenience within the function. The caller always gets the new
>>> transaction through *tpp, which was set by xfs_trans_dup() before the commit
>>> call. Moving tp = *tpp before the error check doesn't change what the caller
>>> sees - *tpp still points to the new (dup'd) transaction regardless.
>>
>> Ah, right.  Tis should be fine:
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Why not move tp_pflags to the new transaction in xfs_trans_dup like we
> do for the deferred item list:
> 
>          /* move deferred ops over to the new tp */
>          xfs_defer_move(ntp, tp);
> 
>          ntp->t_pflags = tp->t_pflags;
>          tp->t_pflags = 0;
> 

That's what the old xfs_trans_switch_context() did before a1ca658d649a 
removed it. The problem is that setting tp->t_pflags = 0 means 
xfs_trans_free() calls memalloc_nofs_restore(0), which relies on that 
being a  no-op — an mm implementation detail. A fresh 
memalloc_nofs_save() on the new tp keeps the save/restore pairing 
correct unconditionally.

BR,
Yun

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

[Darrick J. Wong]

[add linux-mm since we're talking about memalloc_nofs_save]

On Tue, Jul 14, 2026 at 10:15:53AM +0800, Zhou, Yun wrote:
> 
> 
> On 7/14/26 07:04, Darrick J. Wong wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > 
> > On Mon, Jul 13, 2026 at 06:28:38AM -0700, Christoph Hellwig wrote:
> > > On Mon, Jul 13, 2026 at 06:06:38PM +0800, Zhou, Yun wrote:
> > > > 
> > > > 
> > > > On 7/13/26 17:09, Christoph Hellwig wrote:
> > > > > 
> > > > > On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
> > > > > > 
> > > > > > 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);
> > > > > 
> > > > > The tp assignment above now returns the incorrect transaction when
> > > > > __xfs_trans_commit fails, so you can't do this.
> > > > > 
> > > > > Otherwise yes, this call should move up.  I don't really see how
> > > > > it fixes the syzbot report, though.
> > > > > 
> > > > 
> > > > Thank you very much for your reply. The tp here is a local variable only
> > > > used for convenience within the function. The caller always gets the new
> > > > transaction through *tpp, which was set by xfs_trans_dup() before the commit
> > > > call. Moving tp = *tpp before the error check doesn't change what the caller
> > > > sees - *tpp still points to the new (dup'd) transaction regardless.
> > > 
> > > Ah, right.  Tis should be fine:
> > > 
> > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > 
> > Why not move tp_pflags to the new transaction in xfs_trans_dup like we
> > do for the deferred item list:
> > 
> >          /* move deferred ops over to the new tp */
> >          xfs_defer_move(ntp, tp);
> > 
> >          ntp->t_pflags = tp->t_pflags;
> >          tp->t_pflags = 0;
> > 
> 
> That's what the old xfs_trans_switch_context() did before a1ca658d649a
> removed it. The problem is that setting tp->t_pflags = 0 means
> xfs_trans_free() calls memalloc_nofs_restore(0), which relies on that being
> a  no-op — an mm implementation detail. A fresh memalloc_nofs_save() on the
> new tp keeps the save/restore pairing correct unconditionally.

So add a new helper.

/**
 * memalloc_flags_take - move an implicit __GFP_MEMALLOC scope from one
 * tracking structure to another.
 */
static inline unsigned int
memalloc_flags_take(unsigned int *old_flags)
{
	unsigned int ret = *old_flags;

	*old_flags = 0;
	return ret;
}

and then:

	/* move deferred ops over to the new tp */
	xfs_defer_move(ntp, tp);

	ntp->t_pflags = memalloc_flags_take(&tp->t_pflags);

--D

> BR,
> Yun
> 

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

[Matthew Wilcox]

On Tue, Jul 14, 2026 at 10:55:28AM -0700, Darrick J. Wong wrote:
> [add linux-mm since we're talking about memalloc_nofs_save]

Thanks!

> On Tue, Jul 14, 2026 at 10:15:53AM +0800, Zhou, Yun wrote:
> > On 7/14/26 07:04, Darrick J. Wong wrote:
> > > On Mon, Jul 13, 2026 at 06:28:38AM -0700, Christoph Hellwig wrote:
> > > > On Mon, Jul 13, 2026 at 06:06:38PM +0800, Zhou, Yun wrote:
> > > > > On 7/13/26 17:09, Christoph Hellwig wrote:
> > > > > > On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
> > > > > > > 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);
> > > > > > 
> > > > > > The tp assignment above now returns the incorrect transaction when
> > > > > > __xfs_trans_commit fails, so you can't do this.
> > > > > > 
> > > > > > Otherwise yes, this call should move up.  I don't really see how
> > > > > > it fixes the syzbot report, though.
> > > > > 
> > > > > Thank you very much for your reply. The tp here is a local variable only
> > > > > used for convenience within the function. The caller always gets the new
> > > > > transaction through *tpp, which was set by xfs_trans_dup() before the commit
> > > > > call. Moving tp = *tpp before the error check doesn't change what the caller
> > > > > sees - *tpp still points to the new (dup'd) transaction regardless.
> > > > 
> > > > Ah, right.  Tis should be fine:
> > > > 
> > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > 
> > > Why not move tp_pflags to the new transaction in xfs_trans_dup like we
> > > do for the deferred item list:
> > > 
> > >          /* move deferred ops over to the new tp */
> > >          xfs_defer_move(ntp, tp);
> > > 
> > >          ntp->t_pflags = tp->t_pflags;
> > >          tp->t_pflags = 0;
> > 
> > That's what the old xfs_trans_switch_context() did before a1ca658d649a
> > removed it. The problem is that setting tp->t_pflags = 0 means
> > xfs_trans_free() calls memalloc_nofs_restore(0), which relies on that being
> > a  no-op — an mm implementation detail. A fresh memalloc_nofs_save() on the
> > new tp keeps the save/restore pairing correct unconditionally.
> 
> So add a new helper.
> 
> /**
>  * memalloc_flags_take - move an implicit __GFP_MEMALLOC scope from one
>  * tracking structure to another.
>  */
> static inline unsigned int
> memalloc_flags_take(unsigned int *old_flags)
> {
> 	unsigned int ret = *old_flags;
> 
> 	*old_flags = 0;
> 	return ret;
> }
> 
> and then:
> 
> 	/* move deferred ops over to the new tp */
> 	xfs_defer_move(ntp, tp);
> 
> 	ntp->t_pflags = memalloc_flags_take(&tp->t_pflags);

I might stick with the 'move' wording?  ie memalloc_flags_move().

And I think you should bury the call to memalloc_flags_move() inside
xfs_defer_move().  I don't think there's a case where you'd want to move
from one transaction to another without preserving the nofs state, is
there?  Bit hard to tell since there's only one caller of xfs_defer_move()
in the XFS code base.

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

[Darrick J. Wong]

On Tue, Jul 14, 2026 at 07:15:44PM +0100, Matthew Wilcox wrote:
> On Tue, Jul 14, 2026 at 10:55:28AM -0700, Darrick J. Wong wrote:
> > [add linux-mm since we're talking about memalloc_nofs_save]
> 
> Thanks!
> 
> > On Tue, Jul 14, 2026 at 10:15:53AM +0800, Zhou, Yun wrote:
> > > On 7/14/26 07:04, Darrick J. Wong wrote:
> > > > On Mon, Jul 13, 2026 at 06:28:38AM -0700, Christoph Hellwig wrote:
> > > > > On Mon, Jul 13, 2026 at 06:06:38PM +0800, Zhou, Yun wrote:
> > > > > > On 7/13/26 17:09, Christoph Hellwig wrote:
> > > > > > > On Mon, Jul 13, 2026 at 11:55:05AM +0800, Yun Zhou wrote:
> > > > > > > > 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);
> > > > > > > 
> > > > > > > The tp assignment above now returns the incorrect transaction when
> > > > > > > __xfs_trans_commit fails, so you can't do this.
> > > > > > > 
> > > > > > > Otherwise yes, this call should move up.  I don't really see how
> > > > > > > it fixes the syzbot report, though.
> > > > > > 
> > > > > > Thank you very much for your reply. The tp here is a local variable only
> > > > > > used for convenience within the function. The caller always gets the new
> > > > > > transaction through *tpp, which was set by xfs_trans_dup() before the commit
> > > > > > call. Moving tp = *tpp before the error check doesn't change what the caller
> > > > > > sees - *tpp still points to the new (dup'd) transaction regardless.
> > > > > 
> > > > > Ah, right.  Tis should be fine:
> > > > > 
> > > > > Reviewed-by: Christoph Hellwig <hch@lst.de>
> > > > 
> > > > Why not move tp_pflags to the new transaction in xfs_trans_dup like we
> > > > do for the deferred item list:
> > > > 
> > > >          /* move deferred ops over to the new tp */
> > > >          xfs_defer_move(ntp, tp);
> > > > 
> > > >          ntp->t_pflags = tp->t_pflags;
> > > >          tp->t_pflags = 0;
> > > 
> > > That's what the old xfs_trans_switch_context() did before a1ca658d649a
> > > removed it. The problem is that setting tp->t_pflags = 0 means
> > > xfs_trans_free() calls memalloc_nofs_restore(0), which relies on that being
> > > a  no-op — an mm implementation detail. A fresh memalloc_nofs_save() on the
> > > new tp keeps the save/restore pairing correct unconditionally.
> > 
> > So add a new helper.
> > 
> > /**
> >  * memalloc_flags_take - move an implicit __GFP_MEMALLOC scope from one
> >  * tracking structure to another.
> >  */
> > static inline unsigned int
> > memalloc_flags_take(unsigned int *old_flags)
> > {
> > 	unsigned int ret = *old_flags;
> > 
> > 	*old_flags = 0;
> > 	return ret;
> > }
> > 
> > and then:
> > 
> > 	/* move deferred ops over to the new tp */
> > 	xfs_defer_move(ntp, tp);
> > 
> > 	ntp->t_pflags = memalloc_flags_take(&tp->t_pflags);
> 
> I might stick with the 'move' wording?  ie memalloc_flags_move().

<nod>

> And I think you should bury the call to memalloc_flags_move() inside
> xfs_defer_move().  I don't think there's a case where you'd want to move
> from one transaction to another without preserving the nofs state, is
> there?  Bit hard to tell since there's only one caller of xfs_defer_move()
> in the XFS code base.

There's only ever going to be one caller, and it's the transaction
rolling mechanism.  I wouldn't put the memalloc_flags_move in
xfs_defer_move because userspace transactions don't have t_pflags
because NOFS is meaningless there.

--D