Re: [PATCH] rhashtable: give each instance its own lockdep class

[Christian Brauner <brauner@kernel.org>]

On Mon, Apr 27, 2026 at 07:29:58PM +0800, Herbert Xu wrote:
> On Mon, Apr 27, 2026 at 01:09:57PM +0200, Christian Brauner wrote:
> > syzbot reported a possible circular locking dependency between
> > &ht->mutex and fs_reclaim:
> > 
> >   CPU0 (kswapd0)                    CPU1 (kworker)
> >   --------------                    --------------
> >   fs_reclaim                        ht->mutex
> >     shmem_evict_inode                 rhashtable_rehash_alloc
> >       simple_xattrs_free                bucket_table_alloc(GFP_KERNEL)
> >         rhashtable_free_and_destroy       __kvmalloc_node
> >           mutex_lock(&ht->mutex)            might_alloc -> fs_reclaim
> > 
> > The two halves of the splat refer to two different events on
> > &ht->mutex.
> > 
> > The kswapd0 path is unambiguous: shmem_evict_inode at mm/shmem.c:1429
> > calls simple_xattrs_free(), which calls rhashtable_free_and_destroy()
> > on the per-inode simple_xattrs rhashtable being torn down with the
> > inode.
> > 
> > The previously-recorded ht->mutex -> fs_reclaim edge comes from
> > rht_deferred_worker -> rhashtable_rehash_alloc ->
> > bucket_table_alloc(GFP_KERNEL) -> __kvmalloc_node ->
> > might_alloc -> fs_reclaim. That stack stops at generic library code:
> > there is no subsystem-specific frame above rht_deferred_worker, so
> > the splat does not identify which rhashtable's worker recorded the
> > edge -- only that some rhashtable in the system did.
> > 
> > Whether or not that recording happened on the same simple_xattrs ht
> > that is now being destroyed, the predicted deadlock cannot occur:
> > rhashtable_free_and_destroy() does cancel_work_sync(&ht->run_work)
> > before taking ht->mutex, so the deferred worker cannot be running on
> > the instance being torn down. If the recording was on a different
> > rhashtable instance, the two ht->mutex acquisitions are on distinct
> > mutex objects and cannot deadlock either.
> > 
> > Lockdep flags a cycle regardless because mutex_init(&ht->mutex) lives
> > on a single source line in rhashtable_init_noprof(), so every
> > ht->mutex in the kernel shares one static lockdep class. Lockdep
> > matches by class, not by instance, and collapses all of these into
> > one node.
> > 
> > Lift the lockdep key out of rhashtable_init_noprof() and into the
> > caller. The user-visible rhashtable_init_noprof() /
> > rhltable_init_noprof() identifiers become macros that declare a
> > per-call-site static lock_class_key.
> > 
> > Reported-by: syzbot+5af806780f38a5fe691f@syzkaller.appspotmail.com
> > Closes: https://lore.kernel.org/69e798fe.050a0220.24bfd3.0032.GAE@google.com
> > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > ---
> >  include/linux/rhashtable-types.h | 22 ++++++++++++++++++----
> >  lib/rhashtable.c                 | 17 ++++++++++-------
> >  2 files changed, 28 insertions(+), 11 deletions(-)
> 
> Thanks for the patch.
> 
> But could you please try this patch and see if it also fixes
> your problem?
> 
> https://patchwork.kernel.org/project/linux-crypto/patch/20260422213349.1345098-2-mikhail.v.gavrilov@gmail.com/

Possibly, I don't have a way to easily reproduce this though.
Imho, the right thing would be to have both: actual useful keyed lockdep
annotation and - if safe - dropping the mutex.