diff mbox

[02/30] IB/core: Add kref to IB devices

Message ID 1b381c86-9f29-49a6-b7c5-9571d4490f5c@CMEXHTCAS2.ad.emulex.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Somnath Kotur Feb. 19, 2015, 10:02 p.m. UTC
From: Matan Barak <matanb@mellanox.com>

Previously. we used device_mutex lock in order to protect
the device's list. That means that in order to guarantee a
device isn't freed while we use it, we had to lock all
devices.

Adding a kref per IB device. Before an IB device
is unregistered, we wait before its not held anymore.

Signed-off-by: Matan Barak <matanb@mellanox.com>
Signed-off-by: Somnath Kotur <somnath.kotur@emulex.com>
---
 drivers/infiniband/core/device.c |   41 ++++++++++++++++++++++++++++++++++++++
 include/rdma/ib_verbs.h          |    6 +++++
 2 files changed, 47 insertions(+), 0 deletions(-)

Comments

Haggai Eran Feb. 19, 2015, 10:57 a.m. UTC | #1
On 20/02/2015 00:02, Somnath Kotur wrote:
> From: Matan Barak <matanb@mellanox.com>
> 
> Previously. we used device_mutex lock in order to protect
> the device's list. That means that in order to guarantee a
> device isn't freed while we use it, we had to lock all
> devices.
> 
> Adding a kref per IB device. Before an IB device
> is unregistered, we wait before its not held anymore.

Maybe I'm missing something, but IB device already has a struct device
embedded in it, which has a kobject with a kref. Wouldn't it be better
to simply move the operations that need to wait to the ib_device_release
function?

Regards,
Haggai
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Matan Barak Feb. 19, 2015, 12:34 p.m. UTC | #2
On Thu, Feb 19, 2015 at 12:57 PM, Haggai Eran <haggaie@mellanox.com> wrote:
> On 20/02/2015 00:02, Somnath Kotur wrote:
>> From: Matan Barak <matanb@mellanox.com>
>>
>> Previously. we used device_mutex lock in order to protect
>> the device's list. That means that in order to guarantee a
>> device isn't freed while we use it, we had to lock all
>> devices.
>>
>> Adding a kref per IB device. Before an IB device
>> is unregistered, we wait before its not held anymore.
>
> Maybe I'm missing something, but IB device already has a struct device
> embedded in it, which has a kobject with a kref. Wouldn't it be better
> to simply move the operations that need to wait to the ib_device_release
> function?

You're correct, but in ib_unregister_device we delete all client's related data.
We would like to ensure that all references were released before this
data is being deleted and the device is still functioning rather than
when the IB device's
memory is freed.
ib_device_get and ib_device_hold are APIs for the clients, similar to
dev_hold and dev_put.

Regards,
Matan

>
> Regards,
> Haggai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 18c1ece..8616a95 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -261,6 +261,39 @@  out:
 	return ret;
 }
 
+static void ib_device_complete_cb(struct kref *kref)
+{
+	struct ib_device *device = container_of(kref, struct ib_device,
+						refcount);
+
+	if (device->reg_state >= IB_DEV_UNREGISTERING)
+		complete(&device->free);
+}
+
+/**
+ * ib_device_hold - increase the reference count of device
+ * @device: ib device to prevent from being free'd
+ *
+ * Prevent the device from being free'd.
+ */
+void ib_device_hold(struct ib_device *device)
+{
+	kref_get(&device->refcount);
+}
+EXPORT_SYMBOL(ib_device_hold);
+
+/**
+ * ib_device_put - decrease the reference count of device
+ * @device: allows this device to be free'd
+ *
+ * Puts the ib_device and allows it to be free'd.
+ */
+int ib_device_put(struct ib_device *device)
+{
+	return kref_put(&device->refcount, ib_device_complete_cb);
+}
+EXPORT_SYMBOL(ib_device_put);
+
 /**
  * ib_register_device - Register an IB device with IB core
  * @device:Device to register
@@ -312,6 +345,9 @@  int ib_register_device(struct ib_device *device,
 
 	list_add_tail(&device->core_list, &device_list);
 
+	kref_init(&device->refcount);
+	init_completion(&device->free);
+
 	device->reg_state = IB_DEV_REGISTERED;
 
 	{
@@ -342,6 +378,8 @@  void ib_unregister_device(struct ib_device *device)
 
 	mutex_lock(&device_mutex);
 
+	device->reg_state = IB_DEV_UNREGISTERING;
+
 	list_for_each_entry_reverse(client, &client_list, list)
 		if (client->remove)
 			client->remove(device);
@@ -355,6 +393,9 @@  void ib_unregister_device(struct ib_device *device)
 
 	ib_device_unregister_sysfs(device);
 
+	ib_device_put(device);
+	wait_for_completion(&device->free);
+
 	spin_lock_irqsave(&device->client_data_lock, flags);
 	list_for_each_entry_safe(context, tmp, &device->client_data_list, list)
 		kfree(context);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 1866595..a7593b0 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -1716,6 +1716,7 @@  struct ib_device {
 	enum {
 		IB_DEV_UNINITIALIZED,
 		IB_DEV_REGISTERED,
+		IB_DEV_UNREGISTERING,
 		IB_DEV_UNREGISTERED
 	}                            reg_state;
 
@@ -1728,6 +1729,8 @@  struct ib_device {
 	u32			     local_dma_lkey;
 	u8                           node_type;
 	u8                           phys_port_cnt;
+	struct kref		     refcount;
+	struct completion	     free;
 };
 
 struct ib_client {
@@ -1741,6 +1744,9 @@  struct ib_client {
 struct ib_device *ib_alloc_device(size_t size);
 void ib_dealloc_device(struct ib_device *device);
 
+void ib_device_hold(struct ib_device *device);
+int ib_device_put(struct ib_device *device);
+
 int ib_register_device(struct ib_device *device,
 		       int (*port_callback)(struct ib_device *,
 					    u8, struct kobject *));