On Tue, Feb 25, 2025 at 06:09:35PM +0800, Alan Huang wrote: > This fixes two deadlocks: > > 1.pcpu_alloc_mutex involved one as pointed by syzbot[1] > 2.recursion deadlock. > > The root cause is that we hold the bc lock during alloc_percpu, fix it > by following the pattern used by __btree_node_mem_alloc(). > > [1] https://lore.kernel.org/all/[email protected]/T/ > > Reported-by: [email protected] > Tested-by: [email protected] > Signed-off-by: Alan Huang <[email protected]> > --- > > syzbot test: > https://lore.kernel.org/all/[email protected]/T/#m92500622db70f4e4974576c390a51697e9e46314 > > fs/bcachefs/btree_cache.c | 7 ++++--- > fs/bcachefs/six.c | 8 +++++++- > fs/bcachefs/six.h | 1 + > 3 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/fs/bcachefs/btree_cache.c b/fs/bcachefs/btree_cache.c > index ca755e8d1a37..87be42fc889c 100644 > --- a/fs/bcachefs/btree_cache.c > +++ b/fs/bcachefs/btree_cache.c > @@ -795,17 +795,18 @@ struct btree *bch2_btree_node_mem_alloc(struct > btree_trans *trans, bool pcpu_rea > } > > b = __btree_node_mem_alloc(c, GFP_NOWAIT|__GFP_NOWARN); > - if (!b) { > + if (b) { > + bch2_btree_lock_init(&b->c, pcpu_read_locks ? > SIX_LOCK_INIT_PCPU|SIX_LOCK_INIT_NOWAIT : 0); > + } else { > mutex_unlock(&bc->lock); > bch2_trans_unlock(trans); > b = __btree_node_mem_alloc(c, GFP_KERNEL); > if (!b) > goto err; > + bch2_btree_lock_init(&b->c, pcpu_read_locks ? > SIX_LOCK_INIT_PCPU : 0); > mutex_lock(&bc->lock); > } > > - bch2_btree_lock_init(&b->c, pcpu_read_locks ? SIX_LOCK_INIT_PCPU : 0); > - > BUG_ON(!six_trylock_intent(&b->c.lock)); > BUG_ON(!six_trylock_write(&b->c.lock)); > > diff --git a/fs/bcachefs/six.c b/fs/bcachefs/six.c > index 7e7c66a1e1a6..81c832061331 100644 > --- a/fs/bcachefs/six.c > +++ b/fs/bcachefs/six.c > @@ -865,6 +865,9 @@ void __six_lock_init(struct six_lock *lock, const char > *name, > * userspace: > */ > #ifdef __KERNEL__ > + EBUG_ON(!(flags & SIX_LOCK_INIT_PCPU) && > + (flags & SIX_LOCK_INIT_NOWAIT)); > + > if (flags & SIX_LOCK_INIT_PCPU) { > /* > * We don't return an error here on memory allocation failure > @@ -873,7 +876,10 @@ void __six_lock_init(struct six_lock *lock, const char > *name, > * failure if they wish by checking lock->readers, but generally > * will not want to treat it as an error. > */ > - lock->readers = alloc_percpu(unsigned); > + if (flags & SIX_LOCK_INIT_NOWAIT) > + lock->readers = alloc_percpu_gfp(unsigned, GFP_NOWAIT); > + else > + lock->readers = alloc_percpu(unsigned); > } > #endif > } > diff --git a/fs/bcachefs/six.h b/fs/bcachefs/six.h > index c142e06b7a3a..d188f699339f 100644 > --- a/fs/bcachefs/six.h > +++ b/fs/bcachefs/six.h > @@ -161,6 +161,7 @@ void six_lock_exit(struct six_lock *lock); > > enum six_lock_init_flags { > SIX_LOCK_INIT_PCPU = 1U << 0, > + SIX_LOCK_INIT_NOWAIT = 1U << 1,
Don't do this, just plumb the gfp flags param through... It makes it harder to see the data flow and make sure a param is threaded correctly when you're converting the format
