fs/xfs/libxfs/xfs_btree.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
xfs_btree_split_worker() calls xfs_trans_set_context() on the shared
transaction, overwriting tp->t_pflags saved by the submitting thread.
This can cause the submitting thread's NOFS protection to be cleared
prematurely when the transaction is freed, risking deadlocks from
allocations recursing into fs_reclaim.
Thread A (NOFS set) Worker
------------------- ------
xfs_trans_alloc()
xfs_trans_set_context(tp)
tp->t_pflags = 0 (*)
...
xfs_btree_split()
queue_work(split_worker)
xfs_trans_set_context(tp)
tp->t_pflags = NOFS (clobbers!)
__xfs_btree_split()
xfs_trans_clear_context(tp)
wait_for_completion()
...
xfs_trans_free(tp)
memalloc_nofs_restore(NOFS)
-> clears Thread A's NOFS!
(*) returns 0 because NOFS was already set by outer scope
Fix by using a local memalloc_nofs_save()/restore() in the worker
instead of touching the shared transaction state.
Reported-by: sashiko <sashiko@sashiko.dev>
Fixes: 756b1c343333 ("xfs: use current->journal_info for detecting transaction recursion")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
fs/xfs/libxfs/xfs_btree.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 60ef7f08b1d3..1e4c43b7fa3d 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -3010,6 +3010,7 @@ xfs_btree_split_worker(
struct xfs_btree_split_args, work);
unsigned long pflags;
unsigned long new_pflags = 0;
+ unsigned int nofs_flags;
/*
* we are in a transaction context here, but may also be doing work
@@ -3021,12 +3022,12 @@ xfs_btree_split_worker(
new_pflags |= PF_MEMALLOC | PF_KSWAPD;
current_set_flags_nested(&pflags, new_pflags);
- xfs_trans_set_context(args->cur->bc_tp);
+ nofs_flags = memalloc_nofs_save();
args->result = __xfs_btree_split(args->cur, args->level, args->ptrp,
args->key, args->curp, args->stat);
- xfs_trans_clear_context(args->cur->bc_tp);
+ memalloc_nofs_restore(nofs_flags);
current_restore_flags_nested(&pflags, new_pflags);
/*
--
2.43.0
Looks good for now: Reviewed-by: Christoph Hellwig <hch@lst.de>
On Mon, Jul 20, 2026 at 01:05:22PM +0800, Yun Zhou wrote: > current_set_flags_nested(&pflags, new_pflags); > - xfs_trans_set_context(args->cur->bc_tp); > + nofs_flags = memalloc_nofs_save(); Note that the above is the only user of current_set_flags_nested. > args->result = __xfs_btree_split(args->cur, args->level, args->ptrp, > args->key, args->curp, args->stat); > > - xfs_trans_clear_context(args->cur->bc_tp); > + memalloc_nofs_restore(nofs_flags); > current_restore_flags_nested(&pflags, new_pflags); and this is the only caller of current_restore_flags_nested. Both of which modify the task flags just like memalloc_nofs_save. I think we'd be much better of just killing all these silly helpers and do direct current->flags manipulations, which will both clarify this code and fix the bug it caused. Similarly xfs_trans_set_context / xfs_trans_set_context need to go away as they were a part of this problem. And to make this coherent, it should be combined with your other flags series.
Hi Christoph, On 7/20/2026 4:35 PM, Christoph Hellwig wrote: > On Mon, Jul 20, 2026 at 01:05:22PM +0800, Yun Zhou wrote: >> current_set_flags_nested(&pflags, new_pflags); >> - xfs_trans_set_context(args->cur->bc_tp); >> + nofs_flags = memalloc_nofs_save(); > > Note that the above is the only user of current_set_flags_nested. > >> args->result = __xfs_btree_split(args->cur, args->level, args->ptrp, >> args->key, args->curp, args->stat); >> >> - xfs_trans_clear_context(args->cur->bc_tp); >> + memalloc_nofs_restore(nofs_flags); >> current_restore_flags_nested(&pflags, new_pflags); > > and this is the only caller of current_restore_flags_nested. Both > of which modify the task flags just like memalloc_nofs_save. > > I think we'd be much better of just killing all these silly helpers > and do direct current->flags manipulations, which will both clarify > this code and fix the bug it caused. > > Similarly xfs_trans_set_context / xfs_trans_set_context need to go > away as they were a part of this problem. And to make this coherent, > it should be combined with your other flags series. > Thanks a lot. I'd like to understand the scope you have in mind. Should the removal of xfs_trans_set/clear_context and current_set/restore_flags_nested be part of my nofs series, or is that something you'd prefer to handle separately? BR, Yun
On Fri, Jul 24, 2026 at 03:54:10PM +0800, Zhou, Yun wrote: > Thanks a lot. > I'd like to understand the scope you have in mind. Should the removal of > xfs_trans_set/clear_context and current_set/restore_flags_nested be part of > my nofs series, or is that something you'd prefer to handle separately? I don't care too strongly. Maybe we should a minimal fix for the bug in ASAP, and then we just need to agree who cleans up after that. Happy to do that myself if you don't have the time or aren't interested.
On 7/24/2026 4:06 PM, Christoph Hellwig wrote: > On Fri, Jul 24, 2026 at 03:54:10PM +0800, Zhou, Yun wrote: >> Thanks a lot. >> I'd like to understand the scope you have in mind. Should the removal of >> xfs_trans_set/clear_context and current_set/restore_flags_nested be part of >> my nofs series, or is that something you'd prefer to handle separately? > > I don't care too strongly. Maybe we should a minimal fix for the bug > in ASAP, and then we just need to agree who cleans up after that. > Happy to do that myself if you don't have the time or aren't interested. Thanks. The patch I sent is already a minimal fix - it just replaces xfs_trans_set/clear_context with memalloc_nofs_save/restore in the worker. Could you take another look? BR, Yun
© 2016 - 2026 Red Hat, Inc.