Linux RDMA and InfiniBand development
 help / color / mirror / Atom feed
* [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked
@ 2019-07-04 13:00 Leon Romanovsky
  2019-07-08 20:01 ` Jason Gunthorpe
  0 siblings, 1 reply; 3+ messages in thread
From: Leon Romanovsky @ 2019-07-04 13:00 UTC (permalink / raw)
  To: Doug Ledford, Jason Gunthorpe
  Cc: Parav Pandit, RDMA mailing list, Leon Romanovsky

From: Parav Pandit <parav@mellanox.com>

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 <parav@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 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(-)

diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
index 18e476b3ced0..00fb3eacda19 100644
--- a/drivers/infiniband/core/cache.c
+++ 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);
 }
diff --git a/drivers/infiniband/core/cma_configfs.c b/drivers/infiniband/core/cma_configfs.c
index 3ec2c415bb70..0a7b5eba2fc0 100644
--- a/drivers/infiniband/core/cma_configfs.c
+++ 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);
 }
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 7f4affe8a10d..adf8d93bb42d 100644
--- a/drivers/infiniband/core/device.c
+++ 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);
diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
index 9f8a48016b41..e0512aef033c 100644
--- a/drivers/infiniband/core/user_mad.c
+++ b/drivers/infiniband/core/user_mad.c
@@ -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;
 }
diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
index 11c13c1381cf..4827aa3415ff 100644
--- a/drivers/infiniband/core/uverbs_main.c
+++ 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);
 	kfree(dev);
 }
 
diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index 92349bf37589..f974b6854224 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -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);
 }
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked
  2019-07-04 13:00 [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked Leon Romanovsky
@ 2019-07-08 20:01 ` Jason Gunthorpe
  2019-07-09  5:25   ` Parav Pandit
  0 siblings, 1 reply; 3+ messages in thread
From: Jason Gunthorpe @ 2019-07-08 20:01 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Doug Ledford, Parav Pandit, RDMA mailing list, Leon Romanovsky

On Thu, Jul 04, 2019 at 04:00:12PM +0300, Leon Romanovsky wrote:
> From: Parav Pandit <parav@mellanox.com>
> 
> 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 <parav@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  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(-)
> 
> diff --git a/drivers/infiniband/core/cache.c b/drivers/infiniband/core/cache.c
> 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);
>  }
> diff --git a/drivers/infiniband/core/cma_configfs.c b/drivers/infiniband/core/cma_configfs.c
> 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.

> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> 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);
> diff --git a/drivers/infiniband/core/user_mad.c b/drivers/infiniband/core/user_mad.c
> 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)
>  				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)

>  	return 0;
>  }
> diff --git a/drivers/infiniband/core/uverbs_main.c b/drivers/infiniband/core/uverbs_main.c
> 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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked
  2019-07-08 20:01 ` Jason Gunthorpe
@ 2019-07-09  5:25   ` Parav Pandit
  0 siblings, 0 replies; 3+ messages in thread
From: Parav Pandit @ 2019-07-09  5:25 UTC (permalink / raw)
  To: Jason Gunthorpe, Leon Romanovsky
  Cc: Doug Ledford, RDMA mailing list, Leon Romanovsky



> -----Original Message-----
> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Tuesday, July 9, 2019 1:31 AM
> To: Leon Romanovsky <leon@kernel.org>
> Cc: Doug Ledford <dledford@redhat.com>; Parav Pandit
> <parav@mellanox.com>; RDMA mailing list <linux-rdma@vger.kernel.org>;
> Leon Romanovsky <leonro@mellanox.com>
> Subject: Re: [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to
> ensure that it is released as unlocked
> 
> On Thu, Jul 04, 2019 at 04:00:12PM +0300, Leon Romanovsky wrote:
> > From: Parav Pandit <parav@mellanox.com>
> >
> > 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 <parav@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >  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(-)
> >
> > diff --git a/drivers/infiniband/core/cache.c
> > b/drivers/infiniband/core/cache.c 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);
> >  }
> > diff --git a/drivers/infiniband/core/cma_configfs.c
> > b/drivers/infiniband/core/cma_configfs.c
> > 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.

> > diff --git a/drivers/infiniband/core/device.c
> > b/drivers/infiniband/core/device.c
> > 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);
> > diff --git a/drivers/infiniband/core/user_mad.c
> > b/drivers/infiniband/core/user_mad.c
> > 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)
> >  				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)
> 
I will leave this for now and address in second iteration.

> >  	return 0;
> >  }
> > diff --git a/drivers/infiniband/core/uverbs_main.c
> > b/drivers/infiniband/core/uverbs_main.c
> > 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
> 
Ack. Adding it.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2019-07-09  5:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-04 13:00 [PATCH rdma-next] RDMA/core: Annotate destroy of mutex to ensure that it is released as unlocked Leon Romanovsky
2019-07-08 20:01 ` Jason Gunthorpe
2019-07-09  5:25   ` Parav Pandit

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox