Skip to contentSkip to conversation

RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked

PATCH In review Leon Romanovsky 3 participants 3 messages Jul 4, 2019 – Jul 9, 2019 InfiniBand

lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

PATCH 1/1 RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked

From: Parav Pandit <[email protected]>

While compiled with CONFIG_DEBUG_MUTEXES, the kernel ensures that mutex is not held during destroy.
Hence add mutex_destroy() for mutexes used in RDMA modules.

Signed-off-by Parav Pandit <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/core/cache.c        | 1 +
 drivers/infiniband/core/cma_configfs.c | 1 +
 drivers/infiniband/core/device.c       | 3 +++
 drivers/infiniband/core/user_mad.c     | 2 +-
 drivers/infiniband/core/uverbs_main.c  | 2 ++
 drivers/infiniband/core/verbs.c        | 1 +
 6 files changed, 9 insertions(+), 1 deletion(-)
drivers/infiniband/core/cache.c +1 -0
index 18e476b3ced0..00fb3eacda19 100644
@@ -810,6 +810,7 @@ static void release_gid_table(struct ib_device *device,
 	if (leak)
 		return;
 
+	mutex_destroy(&table->lock);
 	kfree(table->data_vec);
 	kfree(table);
 }
drivers/infiniband/core/cma_configfs.c +1 -0
index 3ec2c415bb70..0a7b5eba2fc0 100644
@@ -350,4 +350,5 @@ int __init cma_configfs_init(void)
 void __exit cma_configfs_exit(void)
 {
 	configfs_unregister_subsystem(&cma_subsys);
+	mutex_destroy(&cma_subsys.su_mutex);
 }
drivers/infiniband/core/device.c +3 -0
index 7f4affe8a10d..adf8d93bb42d 100644
@@ -508,6 +508,9 @@ static void ib_device_release(struct device *device)
 			  rcu_head);
 	}
 
+	mutex_destroy(&dev->unregistration_lock);
+	mutex_destroy(&dev->compat_devs_mutex);
+
 	xa_destroy(&dev->compat_devs);
 	xa_destroy(&dev->client_data);
 	kfree_rcu(dev, rcu_head);
drivers/infiniband/core/user_mad.c +1 -1
index 9f8a48016b41..e0512aef033c 100644
@@ -1038,7 +1038,7 @@ static int ib_umad_close(struct inode *inode, struct file *filp)
 				ib_unregister_mad_agent(file->agent[i]);
 
 	mutex_unlock(&file->port->file_mutex);
-
+	mutex_destroy(&file->mutex);
 	kfree(file);
 	return 0;
 }
drivers/infiniband/core/uverbs_main.c +2 -0
index 11c13c1381cf..4827aa3415ff 100644
@@ -120,6 +120,8 @@ static void ib_uverbs_release_dev(struct device *device)
 
 	uverbs_destroy_api(dev->uapi);
 	cleanup_srcu_struct(&dev->disassociate_srcu);
+	mutex_destroy(&dev->lists_mutex);
+	mutex_destroy(&dev->xrcd_tree_mutex);
 	kfree(dev);
 }
drivers/infiniband/core/verbs.c +1 -0
index 92349bf37589..f974b6854224 100644
@@ -2259,6 +2259,7 @@ int ib_dealloc_xrcd(struct ib_xrcd *xrcd, struct ib_udata *udata)
 		if (ret)
 			return ret;
 	}
+	mutex_destroy(&xrcd->tgt_qp_mutex);
 
 	return xrcd->device->ops.dealloc_xrcd(xrcd, udata);
 }
Signature · 1 line
2.20.1
#1 lore raw
Quoted 31 lines · Leon Romanovsky

From: Parav Pandit <[email protected]>

While compiled with CONFIG_DEBUG_MUTEXES, the kernel ensures that mutex is not held during destroy.
Hence add mutex_destroy() for mutexes used in RDMA modules.

Signed-off-by Parav Pandit <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
 drivers/infiniband/core/cache.c        | 1 +
 drivers/infiniband/core/cma_configfs.c | 1 +
 drivers/infiniband/core/device.c       | 3 +++
 drivers/infiniband/core/user_mad.c     | 2 +-
 drivers/infiniband/core/uverbs_main.c  | 2 ++
 drivers/infiniband/core/verbs.c        | 1 +
 6 files changed, 9 insertions(+), 1 deletion(-)
drivers/infiniband/core/cache.c +2 -0
index 18e476b3ced0..00fb3eacda19 100644
+++ b/drivers/infiniband/core/cache.c

@@ -810,6 +810,7 @@ static void release_gid_table(struct ib_device *device,
 	if (leak)
 		return;
 
+	mutex_destroy(&table->lock);
 	kfree(table->data_vec);
 	kfree(table);
 }
drivers/infiniband/core/cma_configfs.c +1 -0
index 3ec2c415bb70..0a7b5eba2fc0 100644
+++ b/drivers/infiniband/core/cma_configfs.c

drivers/infiniband/core/cma_configfs.c int __init cma_configfs_init(void) · line 354

Quoted 5 lines
drivers/infiniband/core/cma_configfs.c +1 -0
@@ -350,4 +350,5 @@ int __init cma_configfs_init(void)
 void __exit cma_configfs_exit(void)
 {
 	configfs_unregister_subsystem(&cma_subsys);
+	mutex_destroy(&cma_subsys.su_mutex);
 }

There is a missing mutex_destroy in cma_configfs_init's error path.

Quoted 17 lines
drivers/infiniband/core/device.c +4 -0
index 7f4affe8a10d..adf8d93bb42d 100644
+++ b/drivers/infiniband/core/device.c

@@ -508,6 +508,9 @@ static void ib_device_release(struct device *device)
 			  rcu_head);
 	}
 
+	mutex_destroy(&dev->unregistration_lock);
+	mutex_destroy(&dev->compat_devs_mutex);
+
 	xa_destroy(&dev->compat_devs);
 	xa_destroy(&dev->client_data);
 	kfree_rcu(dev, rcu_head);
drivers/infiniband/core/user_mad.c +1 -0
index 9f8a48016b41..e0512aef033c 100644
+++ b/drivers/infiniband/core/user_mad.c

drivers/infiniband/core/user_mad.c static int ib_umad_close(struct inode *inode, struct file *filp) · line 1042

Quoted 6 lines
drivers/infiniband/core/user_mad.c +1 -1
@@ -1038,7 +1038,7 @@ static int ib_umad_close(struct inode *inode, struct file *filp)
 				ib_unregister_mad_agent(file->agent[i]);
 
 	mutex_unlock(&file->port->file_mutex);
-
+	mutex_destroy(&file->mutex);
 	kfree(file);

The file->port->file_mutex is missing a destroy in ib_umad_dev_free (bit tricky to do)

drivers/infiniband/core/uverbs_main.c static void ib_uverbs_release_dev(struct device *device) · line 124

Quoted 11 lines
drivers/infiniband/core/user_mad.c +0 -0
 	return 0;
 }
drivers/infiniband/core/uverbs_main.c +3 -0
index 11c13c1381cf..4827aa3415ff 100644
+++ b/drivers/infiniband/core/uverbs_main.c

@@ -120,6 +120,8 @@ static void ib_uverbs_release_dev(struct device *device)
 
 	uverbs_destroy_api(dev->uapi);
 	cleanup_srcu_struct(&dev->disassociate_srcu);
+	mutex_destroy(&dev->lists_mutex);
+	mutex_destroy(&dev->xrcd_tree_mutex);

This file also has ucontext_lock and umap_lock that are missing destroy

Jason

#2 lore raw
Quoted 38 lines

-----Original Message-----
From: Jason Gunthorpe <[email protected]>
Sent: Tuesday, July 9, 2019 1:31 AM
To: Leon Romanovsky <[email protected]>
Cc: Doug Ledford <[email protected]>; Parav Pandit <[email protected]>; RDMA mailing list <[email protected]>; Leon Romanovsky <[email protected]>
Subject: Re: [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked

Review static void release_gid_table(struct ib_device

Quoted 21 lines · Leon Romanovsky

From: Parav Pandit <[email protected]>

While compiled with CONFIG_DEBUG_MUTEXES, the kernel ensures that mutex is not held during destroy.
Hence add mutex_destroy() for mutexes used in RDMA modules.

Signed-off-by Parav Pandit <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
 drivers/infiniband/core/cache.c        | 1 +
 drivers/infiniband/core/cma_configfs.c | 1 +
 drivers/infiniband/core/device.c       | 3 +++
 drivers/infiniband/core/user_mad.c     | 2 +-
 drivers/infiniband/core/uverbs_main.c  | 2 ++
 drivers/infiniband/core/verbs.c        | 1 +
 6 files changed, 9 insertions(+), 1 deletion(-)
(quoted diff) +0 -0

b/drivers/infiniband/core/cache.c index 18e476b3ced0..00fb3eacda19 100644
+++ b/drivers/infiniband/core/cache.c

(quoted diff) +0 -0
@@ -810,6 +810,7 @@ static void release_gid_table(struct ib_device

*device,

Quoted 3 lines
drivers/infiniband/core/cache.c +0 -0
 	if (leak)
 		return;
Quoted 16 lines

drivers/infiniband/core/cma_configfs.c int __init cma_configfs_init(void) void __exit · line 353

Quoted 13 lines
drivers/infiniband/core/cache.c +1 -0
+	mutex_destroy(&table->lock);
 	kfree(table->data_vec);
 	kfree(table);
 }
drivers/infiniband/core/cma_configfs.c +2 -0
index 3ec2c415bb70..0a7b5eba2fc0 100644
+++ b/drivers/infiniband/core/cma_configfs.c

@@ -350,4 +350,5 @@ int __init cma_configfs_init(void)  void __exit
 cma_configfs_exit(void)  {
 	configfs_unregister_subsystem(&cma_subsys);
+	mutex_destroy(&cma_subsys.su_mutex);
 }

There is a missing mutex_destroy in cma_configfs_init's error path.

Ack. Adding it.

Quoted 22 lines

drivers/infiniband/core/user_mad.c static int ib_umad_close(struct inode *inode, struct

Quoted 19 lines
drivers/infiniband/core/device.c +4 -0
index 7f4affe8a10d..adf8d93bb42d 100644
+++ b/drivers/infiniband/core/device.c

@@ -508,6 +508,9 @@ static void ib_device_release(struct device *device)
 			  rcu_head);
 	}
 
+	mutex_destroy(&dev->unregistration_lock);
+	mutex_destroy(&dev->compat_devs_mutex);
+
 	xa_destroy(&dev->compat_devs);
 	xa_destroy(&dev->client_data);
 	kfree_rcu(dev, rcu_head);
drivers/infiniband/core/user_mad.c +1 -0
index 9f8a48016b41..e0512aef033c 100644
+++ b/drivers/infiniband/core/user_mad.c

@@ -1038,7 +1038,7 @@ static int ib_umad_close(struct inode *inode, struct

file *filp)

Quoted 2 lines
drivers/infiniband/core/user_mad.c +0 -0
 				ib_unregister_mad_agent(file->agent[i]);
Quoted 8 lines

drivers/infiniband/core/user_mad.c static int ib_umad_close(struct inode *inode, struct file *filp) · line 1042

Quoted 4 lines
drivers/infiniband/core/user_mad.c +1 -1
 	mutex_unlock(&file->port->file_mutex);
-
+	mutex_destroy(&file->mutex);
 	kfree(file);

The file->port->file_mutex is missing a destroy in ib_umad_dev_free (bit tricky to do)

I will leave this for now and address in second iteration.

Quoted 9 lines
Quoted 9 lines
drivers/infiniband/core/user_mad.c +0 -0
 	return 0;
 }
drivers/infiniband/core/uverbs_main.c +1 -0
index 11c13c1381cf..4827aa3415ff 100644
+++ b/drivers/infiniband/core/uverbs_main.c

@@ -120,6 +120,8 @@ static void ib_uverbs_release_dev(struct device
 *device)
Quoted 7 lines

drivers/infiniband/core/uverbs_main.c static void ib_uverbs_release_dev(struct device *device) · line 124

Quoted 4 lines
drivers/infiniband/core/uverbs_main.c +2 -0
 	uverbs_destroy_api(dev->uapi);
 	cleanup_srcu_struct(&dev->disassociate_srcu);
+	mutex_destroy(&dev->lists_mutex);
+	mutex_destroy(&dev->xrcd_tree_mutex);

This file also has ucontext_lock and umap_lock that are missing destroy

Ack. Adding it.