diff options
| author | Dexuan Cui <decui@microsoft.com> | 2018-12-02 19:54:35 -0500 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2018-12-03 02:01:01 -0500 |
| commit | 37c2578c0c40e286bc0d30bdc05290b2058cf66e (patch) | |
| tree | 2c5b97ccde72236e13a982772588d147e40a0bc2 | |
| parent | 2595646791c319cadfdbf271563aac97d0843dc7 (diff) | |
Drivers: hv: vmbus: Offload the handling of channels to two workqueues
vmbus_process_offer() mustn't call channel->sc_creation_callback()
directly for sub-channels, because sc_creation_callback() ->
vmbus_open() may never get the host's response to the
OPEN_CHANNEL message (the host may rescind a channel at any time,
e.g. in the case of hot removing a NIC), and vmbus_onoffer_rescind()
may not wake up the vmbus_open() as it's blocked due to a non-zero
vmbus_connection.offer_in_progress, and finally we have a deadlock.
The above is also true for primary channels, if the related device
drivers use sync probing mode by default.
And, usually the handling of primary channels and sub-channels can
depend on each other, so we should offload them to different
workqueues to avoid possible deadlock, e.g. in sync-probing mode,
NIC1's netvsc_subchan_work() can race with NIC2's netvsc_probe() ->
rtnl_lock(), and causes deadlock: the former gets the rtnl_lock
and waits for all the sub-channels to appear, but the latter
can't get the rtnl_lock and this blocks the handling of sub-channels.
The patch can fix the multiple-NIC deadlock described above for
v3.x kernels (e.g. RHEL 7.x) which don't support async-probing
of devices, and v4.4, v4.9, v4.14 and v4.18 which support async-probing
but don't enable async-probing for Hyper-V drivers (yet).
The patch can also fix the hang issue in sub-channel's handling described
above for all versions of kernels, including v4.19 and v4.20-rc4.
So actually the patch should be applied to all the existing kernels,
not only the kernels that have 8195b1396ec8.
Fixes: 8195b1396ec8 ("hv_netvsc: fix deadlock on hotplug")
Cc: stable@vger.kernel.org
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
| -rw-r--r-- | drivers/hv/channel_mgmt.c | 189 | ||||
| -rw-r--r-- | drivers/hv/connection.c | 24 | ||||
| -rw-r--r-- | drivers/hv/hyperv_vmbus.h | 7 | ||||
| -rw-r--r-- | include/linux/hyperv.h | 7 |
4 files changed, 161 insertions, 66 deletions
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 6277597d3d58..edd34c167a9b 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c | |||
| @@ -435,61 +435,16 @@ void vmbus_free_channels(void) | |||
| 435 | } | 435 | } |
| 436 | } | 436 | } |
| 437 | 437 | ||
| 438 | /* | 438 | /* Note: the function can run concurrently for primary/sub channels. */ |
| 439 | * vmbus_process_offer - Process the offer by creating a channel/device | 439 | static void vmbus_add_channel_work(struct work_struct *work) |
| 440 | * associated with this offer | ||
| 441 | */ | ||
| 442 | static void vmbus_process_offer(struct vmbus_channel *newchannel) | ||
| 443 | { | 440 | { |
| 444 | struct vmbus_channel *channel; | 441 | struct vmbus_channel *newchannel = |
| 445 | bool fnew = true; | 442 | container_of(work, struct vmbus_channel, add_channel_work); |
| 443 | struct vmbus_channel *primary_channel = newchannel->primary_channel; | ||
| 446 | unsigned long flags; | 444 | unsigned long flags; |
| 447 | u16 dev_type; | 445 | u16 dev_type; |
| 448 | int ret; | 446 | int ret; |
| 449 | 447 | ||
| 450 | /* Make sure this is a new offer */ | ||
| 451 | mutex_lock(&vmbus_connection.channel_mutex); | ||
| 452 | |||
| 453 | /* | ||
| 454 | * Now that we have acquired the channel_mutex, | ||
| 455 | * we can release the potentially racing rescind thread. | ||
| 456 | */ | ||
| 457 | atomic_dec(&vmbus_connection.offer_in_progress); | ||
| 458 | |||
| 459 | list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) { | ||
| 460 | if (!uuid_le_cmp(channel->offermsg.offer.if_type, | ||
| 461 | newchannel->offermsg.offer.if_type) && | ||
| 462 | !uuid_le_cmp(channel->offermsg.offer.if_instance, | ||
| 463 | newchannel->offermsg.offer.if_instance)) { | ||
| 464 | fnew = false; | ||
| 465 | break; | ||
| 466 | } | ||
| 467 | } | ||
| 468 | |||
| 469 | if (fnew) | ||
| 470 | list_add_tail(&newchannel->listentry, | ||
| 471 | &vmbus_connection.chn_list); | ||
| 472 | |||
| 473 | mutex_unlock(&vmbus_connection.channel_mutex); | ||
| 474 | |||
| 475 | if (!fnew) { | ||
| 476 | /* | ||
| 477 | * Check to see if this is a sub-channel. | ||
| 478 | */ | ||
| 479 | if (newchannel->offermsg.offer.sub_channel_index != 0) { | ||
| 480 | /* | ||
| 481 | * Process the sub-channel. | ||
| 482 | */ | ||
| 483 | newchannel->primary_channel = channel; | ||
| 484 | spin_lock_irqsave(&channel->lock, flags); | ||
| 485 | list_add_tail(&newchannel->sc_list, &channel->sc_list); | ||
| 486 | channel->num_sc++; | ||
| 487 | spin_unlock_irqrestore(&channel->lock, flags); | ||
| 488 | } else { | ||
| 489 | goto err_free_chan; | ||
| 490 | } | ||
| 491 | } | ||
| 492 | |||
| 493 | dev_type = hv_get_dev_type(newchannel); | 448 | dev_type = hv_get_dev_type(newchannel); |
| 494 | 449 | ||
| 495 | init_vp_index(newchannel, dev_type); | 450 | init_vp_index(newchannel, dev_type); |
| @@ -507,27 +462,26 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) | |||
| 507 | /* | 462 | /* |
| 508 | * This state is used to indicate a successful open | 463 | * This state is used to indicate a successful open |
| 509 | * so that when we do close the channel normally, we | 464 | * so that when we do close the channel normally, we |
| 510 | * can cleanup properly | 465 | * can cleanup properly. |
| 511 | */ | 466 | */ |
| 512 | newchannel->state = CHANNEL_OPEN_STATE; | 467 | newchannel->state = CHANNEL_OPEN_STATE; |
| 513 | 468 | ||
| 514 | if (!fnew) { | 469 | if (primary_channel != NULL) { |
| 515 | struct hv_device *dev | 470 | /* newchannel is a sub-channel. */ |
| 516 | = newchannel->primary_channel->device_obj; | 471 | struct hv_device *dev = primary_channel->device_obj; |
| 517 | 472 | ||
| 518 | if (vmbus_add_channel_kobj(dev, newchannel)) | 473 | if (vmbus_add_channel_kobj(dev, newchannel)) |
| 519 | goto err_free_chan; | 474 | goto err_deq_chan; |
| 475 | |||
| 476 | if (primary_channel->sc_creation_callback != NULL) | ||
| 477 | primary_channel->sc_creation_callback(newchannel); | ||
| 520 | 478 | ||
| 521 | if (channel->sc_creation_callback != NULL) | ||
| 522 | channel->sc_creation_callback(newchannel); | ||
| 523 | newchannel->probe_done = true; | 479 | newchannel->probe_done = true; |
| 524 | return; | 480 | return; |
| 525 | } | 481 | } |
| 526 | 482 | ||
| 527 | /* | 483 | /* |
| 528 | * Start the process of binding this offer to the driver | 484 | * Start the process of binding the primary channel to the driver |
| 529 | * We need to set the DeviceObject field before calling | ||
| 530 | * vmbus_child_dev_add() | ||
| 531 | */ | 485 | */ |
| 532 | newchannel->device_obj = vmbus_device_create( | 486 | newchannel->device_obj = vmbus_device_create( |
| 533 | &newchannel->offermsg.offer.if_type, | 487 | &newchannel->offermsg.offer.if_type, |
| @@ -556,13 +510,28 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) | |||
| 556 | 510 | ||
| 557 | err_deq_chan: | 511 | err_deq_chan: |
| 558 | mutex_lock(&vmbus_connection.channel_mutex); | 512 | mutex_lock(&vmbus_connection.channel_mutex); |
| 559 | list_del(&newchannel->listentry); | 513 | |
| 514 | /* | ||
| 515 | * We need to set the flag, otherwise | ||
| 516 | * vmbus_onoffer_rescind() can be blocked. | ||
| 517 | */ | ||
| 518 | newchannel->probe_done = true; | ||
| 519 | |||
| 520 | if (primary_channel == NULL) { | ||
| 521 | list_del(&newchannel->listentry); | ||
| 522 | } else { | ||
| 523 | spin_lock_irqsave(&primary_channel->lock, flags); | ||
| 524 | list_del(&newchannel->sc_list); | ||
| 525 | spin_unlock_irqrestore(&primary_channel->lock, flags); | ||
| 526 | } | ||
| 527 | |||
| 560 | mutex_unlock(&vmbus_connection.channel_mutex); | 528 | mutex_unlock(&vmbus_connection.channel_mutex); |
| 561 | 529 | ||
| 562 | if (newchannel->target_cpu != get_cpu()) { | 530 | if (newchannel->target_cpu != get_cpu()) { |
| 563 | put_cpu(); | 531 | put_cpu(); |
| 564 | smp_call_function_single(newchannel->target_cpu, | 532 | smp_call_function_single(newchannel->target_cpu, |
| 565 | percpu_channel_deq, newchannel, true); | 533 | percpu_channel_deq, |
| 534 | newchannel, true); | ||
| 566 | } else { | 535 | } else { |
| 567 | percpu_channel_deq(newchannel); | 536 | percpu_channel_deq(newchannel); |
| 568 | put_cpu(); | 537 | put_cpu(); |
| @@ -570,14 +539,104 @@ err_deq_chan: | |||
| 570 | 539 | ||
| 571 | vmbus_release_relid(newchannel->offermsg.child_relid); | 540 | vmbus_release_relid(newchannel->offermsg.child_relid); |
| 572 | 541 | ||
| 573 | err_free_chan: | ||
| 574 | free_channel(newchannel); | 542 | free_channel(newchannel); |
| 575 | } | 543 | } |
| 576 | 544 | ||
| 577 | /* | 545 | /* |
| 546 | * vmbus_process_offer - Process the offer by creating a channel/device | ||
| 547 | * associated with this offer | ||
| 548 | */ | ||
| 549 | static void vmbus_process_offer(struct vmbus_channel *newchannel) | ||
| 550 | { | ||
| 551 | struct vmbus_channel *channel; | ||
| 552 | struct workqueue_struct *wq; | ||
| 553 | unsigned long flags; | ||
| 554 | bool fnew = true; | ||
| 555 | |||
| 556 | mutex_lock(&vmbus_connection.channel_mutex); | ||
| 557 | |||
| 558 | /* | ||
| 559 | * Now that we have acquired the channel_mutex, | ||
| 560 | * we can release the potentially racing rescind thread. | ||
| 561 | */ | ||
| 562 | atomic_dec(&vmbus_connection.offer_in_progress); | ||
| 563 | |||
| 564 | list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) { | ||
| 565 | if (!uuid_le_cmp(channel->offermsg.offer.if_type, | ||
| 566 | newchannel->offermsg.offer.if_type) && | ||
| 567 | !uuid_le_cmp(channel->offermsg.offer.if_instance, | ||
| 568 | newchannel->offermsg.offer.if_instance)) { | ||
| 569 | fnew = false; | ||
| 570 | break; | ||
| 571 | } | ||
| 572 | } | ||
| 573 | |||
| 574 | if (fnew) | ||
| 575 | list_add_tail(&newchannel->listentry, | ||
| 576 | &vmbus_connection.chn_list); | ||
| 577 | else { | ||
| 578 | /* | ||
| 5 | |||
