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

[Christoph Hellwig <hch@infradead.org>]

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>