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@xxxxxx>

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