[bug report] WARNING: possible circular locking at: rdma_destroy_id+0x17/0x20 [rdma_cm] triggered by blktests nvmeof-mp/002

[Jason Gunthorpe]

On Wed, Jun 01, 2022 at 09:26:52AM -0700, Bart Van Assche wrote:
> On 6/1/22 05:45, Jason Gunthorpe wrote:
> > On Tue, May 31, 2022 at 10:55:46AM -0700, Bart Van Assche wrote:
> > > On 5/31/22 05:35, Jason Gunthorpe wrote:
> > > > On Sat, May 28, 2022 at 09:00:16PM +0200, Bart Van Assche wrote:
> > > > > On 5/27/22 14:52, Jason Gunthorpe wrote:
> > > > > > That only works if you can detect actual different lock classes during
> > > > > > lock creation. It doesn't seem applicable in this case.
> > > > > 
> > > > > Why doesn't it seem applicable in this case? The default behavior of
> > > > > mutex_init() and related initialization functions is to create one lock
> > > > > class per synchronization object initialization caller.
> > > > > lockdep_register_key() can be used to create one lock class per
> > > > > synchronization object instance. I introduced lockdep_register_key() myself
> > > > > a few years ago.
> > > > 
> > > > I don't think this should be used to create one key per instance of
> > > > the object which would be required here. The overhead would be very
> > > > high.
> > > 
> > > Are we perhaps referring to different code changes? I'm referring to the
> > > code change below. The runtime and memory overhead of the patch below
> > > should be minimal.
> > 
> > This is not minimal, the lockdep graph will expand now with a node per
> > created CM ID ever created and with all the additional locking
> > arcs. This is an expensive operation.
> > 
> > AFIAK keys should not be created per-object like this but based on
> > object classes known when the object is created - eg a CM listening ID
> > vs a connceting ID as an example
> > 
> > This might be a suitable hack if the # of objects was small???
> 
> Lockdep uses hashing when looking up a lock object so the lookup time
> shouldn't increase significantly if the number of hash collisions stays low.
> I think this is likely since the number of hash entries is identical to the
> maximum number of synchronization objects divided by two. See also the
> definition of the lock_keys_hash[] array in kernel/locking/lockdep.c.

That is just the keys, not the graph arcs. lockdep records an arc
between every key that establishes a locking relationship and
minimizing the number of keys also de-duplicates those arcs.

Jason