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

[Zhou, Yun]

Hi all,

On 7/20/2026 4:39 PM, Christoph Hellwig wrote:
> 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.

It's been a few days since the last discussion. I'm planning to
respin as v3 following Christoph's suggestion (drop the helper,
re-save directly in xfs_trans_roll). Any other suggestions?

Thanks,
Yun