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

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