Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

IB/core: Add rwsem to allow reading device list or client list

Currently the RDMA subsystem's device list and client list are protected by
a single mutex. This prevents adding user-facing APIs that iterate these
lists, since using them may cause a deadlock. The patch attempts to solve
this problem by adding a read-write semaphore to protect the lists. Readers
now don't need the mutex, and are safe just by read-locking the semaphore.

The ib_register_device, ib_register_client, ib_unregister_device, and
ib_unregister_client functions are modified to lock the semaphore for write
during their respective list modification. Also, in order to make sure
client callbacks are called only between add() and remove() calls, the code
is changed to only add items to the lists after the add() calls and remove
from the lists before the remove() calls.

This patch attempts to solve a similar need [1] that was seen in the RoCE
v2 patch series.

[1] http://www.spinics.net/lists/linux-rdma/msg24733.html

Reviewed-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
Cc: Matan Barak <matanb@mellanox.com>
Signed-off-by: Haggai Eran <haggaie@mellanox.com>
Signed-off-by: Doug Ledford <dledford@redhat.com>

authored by

Haggai Eran and committed by
Doug Ledford
5aa44bb9 3403051e

+28 -12
+28 -12
drivers/infiniband/core/device.c
··· 55 55 struct workqueue_struct *ib_wq; 56 56 EXPORT_SYMBOL_GPL(ib_wq); 57 57 58 + /* The device_list and client_list contain devices and clients after their 59 + * registration has completed, and the devices and clients are removed 60 + * during unregistration. */ 58 61 static LIST_HEAD(device_list); 59 62 static LIST_HEAD(client_list); 60 63 61 64 /* 62 - * device_mutex protects access to both device_list and client_list. 63 - * There's no real point to using multiple locks or something fancier 64 - * like an rwsem: we always access both lists, and we're always 65 - * modifying one list or the other list. In any case this is not a 66 - * hot path so there's no point in trying to optimize. 65 + * device_mutex and lists_rwsem protect access to both device_list and 66 + * client_list. device_mutex protects writer access by device and client 67 + * registration / de-registration. lists_rwsem protects reader access to 68 + * these lists. Iterators of these lists must lock it for read, while updates 69 + * to the lists must be done with a write lock. A special case is when the 70 + * device_mutex is locked. In this case locking the lists for read access is 71 + * not necessary as the device_mutex implies it. 67 72 */ 68 73 static DEFINE_MUTEX(device_mutex); 74 + static DECLARE_RWSEM(lists_rwsem); 75 + 69 76 70 77 static int ib_device_check_mandatory(struct ib_device *device) 71 78 { ··· 312 305 goto out; 313 306 } 314 307 315 - list_add_tail(&device->core_list, &device_list); 316 - 317 308 device->reg_state = IB_DEV_REGISTERED; 318 309 319 310 { ··· 322 317 client->add(device); 323 318 } 324 319 325 - out: 320 + down_write(&lists_rwsem); 321 + list_add_tail(&device->core_list, &device_list); 322 + up_write(&lists_rwsem); 323 + out: 326 324 mutex_unlock(&device_mutex); 327 325 return ret; 328 326 } ··· 345 337 346 338 mutex_lock(&device_mutex); 347 339 340 + down_write(&lists_rwsem); 341 + list_del(&device->core_list); 342 + up_write(&lists_rwsem); 343 + 348 344 list_for_each_entry_reverse(client, &client_list, list) 349 345 if (client->remove) 350 346 client->remove(device); 351 - 352 - list_del(&device->core_list); 353 347 354 348 mutex_unlock(&device_mutex); 355 349 ··· 385 375 386 376 mutex_lock(&device_mutex); 387 377 388 - list_add_tail(&client->list, &client_list); 389 378 list_for_each_entry(device, &device_list, core_list) 390 379 if (client->add && !add_client_context(device, client)) 391 380 client->add(device); 381 + 382 + down_write(&lists_rwsem); 383 + list_add_tail(&client->list, &client_list); 384 + up_write(&lists_rwsem); 392 385 393 386 mutex_unlock(&device_mutex); 394 387 ··· 415 402 416 403 mutex_lock(&device_mutex); 417 404 405 + down_write(&lists_rwsem); 406 + list_del(&client->list); 407 + up_write(&lists_rwsem); 408 + 418 409 list_for_each_entry(device, &device_list, core_list) { 419 410 if (client->remove) 420 411 client->remove(device); ··· 431 414 } 432 415 spin_unlock_irqrestore(&device->client_data_lock, flags); 433 416 } 434 - list_del(&client->list); 435 417 436 418 mutex_unlock(&device_mutex); 437 419 }