Commit 9cd58817 authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Doug Ledford

RDMA/devices: Remove the lock around remove_client_context

Due to the complexity of client->remove() callbacks it is desirable to not
hold any locks while calling them. Remove the last one by tracking only
the highest client ID and running backwards from there over the xarray.

Since the only purpose of that lock was to protect the linked list, we can
drop the lock.
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
Signed-off-by: default avatarLeon Romanovsky <leonro@mellanox.com>
Link: https://lore.kernel.org/r/20190731081841.32345-3-leon@kernel.orgSigned-off-by: default avatarDoug Ledford <dledford@redhat.com>
parent 621e55ff
...@@ -94,7 +94,7 @@ static DEFINE_XARRAY_FLAGS(devices, XA_FLAGS_ALLOC); ...@@ -94,7 +94,7 @@ static DEFINE_XARRAY_FLAGS(devices, XA_FLAGS_ALLOC);
static DECLARE_RWSEM(devices_rwsem); static DECLARE_RWSEM(devices_rwsem);
#define DEVICE_REGISTERED XA_MARK_1 #define DEVICE_REGISTERED XA_MARK_1
static LIST_HEAD(client_list); static u32 highest_client_id;
#define CLIENT_REGISTERED XA_MARK_1 #define CLIENT_REGISTERED XA_MARK_1
static DEFINE_XARRAY_FLAGS(clients, XA_FLAGS_ALLOC); static DEFINE_XARRAY_FLAGS(clients, XA_FLAGS_ALLOC);
static DECLARE_RWSEM(clients_rwsem); static DECLARE_RWSEM(clients_rwsem);
...@@ -1237,7 +1237,7 @@ static int setup_device(struct ib_device *device) ...@@ -1237,7 +1237,7 @@ static int setup_device(struct ib_device *device)
static void disable_device(struct ib_device *device) static void disable_device(struct ib_device *device)
{ {
struct ib_client *client; u32 cid;
WARN_ON(!refcount_read(&device->refcount)); WARN_ON(!refcount_read(&device->refcount));
...@@ -1245,10 +1245,19 @@ static void disable_device(struct ib_device *device) ...@@ -1245,10 +1245,19 @@ static void disable_device(struct ib_device *device)
xa_clear_mark(&devices, device->index, DEVICE_REGISTERED); xa_clear_mark(&devices, device->index, DEVICE_REGISTERED);
up_write(&devices_rwsem); up_write(&devices_rwsem);
/*
* Remove clients in LIFO order, see assign_client_id. This could be
* more efficient if xarray learns to reverse iterate. Since no new
* clients can be added to this ib_device past this point we only need
* the maximum possible client_id value here.
*/
down_read(&clients_rwsem); down_read(&clients_rwsem);
list_for_each_entry_reverse(client, &client_list, list) cid = highest_client_id;
remove_client_context(device, client->client_id);
up_read(&clients_rwsem); up_read(&clients_rwsem);
while (cid) {
cid--;
remove_client_context(device, cid);
}
/* Pairs with refcount_set in enable_device */ /* Pairs with refcount_set in enable_device */
ib_device_put(device); ib_device_put(device);
...@@ -1675,30 +1684,31 @@ static int assign_client_id(struct ib_client *client) ...@@ -1675,30 +1684,31 @@ static int assign_client_id(struct ib_client *client)
/* /*
* The add/remove callbacks must be called in FIFO/LIFO order. To * The add/remove callbacks must be called in FIFO/LIFO order. To
* achieve this we assign client_ids so they are sorted in * achieve this we assign client_ids so they are sorted in
* registration order, and retain a linked list we can reverse iterate * registration order.
* to get the LIFO order. The extra linked list can go away if xarray
* learns to reverse iterate.
*/ */
if (list_empty(&client_list)) { client->client_id = highest_client_id;
client->client_id = 0;
} else {
struct ib_client *last;
last = list_last_entry(&client_list, struct ib_client, list);
client->client_id = last->client_id + 1;
}
ret = xa_insert(&clients, client->client_id, client, GFP_KERNEL); ret = xa_insert(&clients, client->client_id, client, GFP_KERNEL);
if (ret) if (ret)
goto out; goto out;
highest_client_id++;
xa_set_mark(&clients, client->client_id, CLIENT_REGISTERED); xa_set_mark(&clients, client->client_id, CLIENT_REGISTERED);
list_add_tail(&client->list, &client_list);
out: out:
up_write(&clients_rwsem); up_write(&clients_rwsem);
return ret; return ret;
} }
static void remove_client_id(struct ib_client *client)
{
down_write(&clients_rwsem);
xa_erase(&clients, client->client_id);
for (; highest_client_id; highest_client_id--)
if (xa_load(&clients, highest_client_id - 1))
break;
up_write(&clients_rwsem);
}
/** /**
* ib_register_client - Register an IB client * ib_register_client - Register an IB client
* @client:Client to register * @client:Client to register
...@@ -1778,11 +1788,7 @@ void ib_unregister_client(struct ib_client *client) ...@@ -1778,11 +1788,7 @@ void ib_unregister_client(struct ib_client *client)
* removal is ongoing. Wait until all removals are completed. * removal is ongoing. Wait until all removals are completed.
*/ */
wait_for_completion(&client->uses_zero); wait_for_completion(&client->uses_zero);
remove_client_id(client);
down_write(&clients_rwsem);
list_del(&client->list);
xa_erase(&clients, client->client_id);
up_write(&clients_rwsem);
} }
EXPORT_SYMBOL(ib_unregister_client); EXPORT_SYMBOL(ib_unregister_client);
......
...@@ -2650,7 +2650,6 @@ struct ib_client { ...@@ -2650,7 +2650,6 @@ struct ib_client {
refcount_t uses; refcount_t uses;
struct completion uses_zero; struct completion uses_zero;
struct list_head list;
u32 client_id; u32 client_id;
/* kverbs are not required by the client */ /* kverbs are not required by the client */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment