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

Bart Van Assche
2022-05-31T17:56:02Z

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.

Thanks,

Bart.

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index fabca5e51e3d..d476c64cd84a 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -860,6 +860,8 @@ __rdma_create_id(struct net *net, rdma_cm_event_handler event_handler,
init_completion(&id_priv->comp);
refcount_set(&id_priv->refcount, 1);
mutex_init(&id_priv->handler_mutex);
+ lockdep_register_key(&id_priv->handler_key);
+ lockdep_set_class(&id_priv->handler_mutex, &id_priv->handler_key);
INIT_LIST_HEAD(&id_priv->device_item);
INIT_LIST_HEAD(&id_priv->listen_list);
INIT_LIST_HEAD(&id_priv->mc_list);
@@ -1899,12 +1901,16 @@ static void _destroy_id(struct rdma_id_private *id_priv,
cma_id_put(id_priv);
wait_for_completion(&id_priv->comp);

+ mutex_destroy(&id_priv->handler_mutex);
+ lockdep_unregister_key(&id_priv->handler_key);
+
if (id_priv->internal_id)
cma_id_put(id_priv->id.context);

kfree(id_priv->id.route.path_rec);

put_net(id_priv->id.route.addr.dev_addr.net);
+
kfree(id_priv);
}

diff --git a/drivers/infiniband/core/cma_priv.h b/drivers/infiniband/core/cma_priv.h
index 757a0ef79872..4affecd044eb 100644
--- a/drivers/infiniband/core/cma_priv.h
+++ b/drivers/infiniband/core/cma_priv.h
@@ -75,6 +75,7 @@ struct rdma_id_private {
struct completion comp;
refcount_t refcount;
struct mutex handler_mutex;
+ struct lock_class_key handler_key;

int backlog;
int timeout_ms;