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

[Bart Van Assche]

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.

Thanks,

Bart.