diff options
| author | Dexuan Cui <decui@microsoft.com> | 2017-03-04 20:13:58 -0500 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2017-03-16 03:42:00 -0400 |
| commit | dad72a1d28442b03aac86836a42de2d00a1014ab (patch) | |
| tree | b7db945d6c53180471cacfbf814b51a865797340 | |
| parent | 8200f2085abe7f29a016381f3122000cc7b2a760 (diff) | |
vmbus: remove hv_event_tasklet_disable/enable
With the recent introduction of per-channel tasklet, we need to update
the way we handle the 3 concurrency issues:
1. hv_process_channel_removal -> percpu_channel_deq vs.
vmbus_chan_sched -> list_for_each_entry(..., percpu_list);
2. vmbus_process_offer -> percpu_channel_enq/deq vs. vmbus_chan_sched.
3. vmbus_close_internal vs. the per-channel tasklet vmbus_on_event;
The first 2 issues can be handled by Stephen's recent patch
"vmbus: use rcu for per-cpu channel list", and the third issue
can be handled by calling tasklet_disable in vmbus_close_internal here.
We don't need the original hv_event_tasklet_disable/enable since we
now use per-channel tasklet instead of the previous per-CPU tasklet,
and actually we must remove them due to the side effect now:
vmbus_process_offer -> hv_event_tasklet_enable -> tasklet_schedule will
start the per-channel callback prematurely, cauing NULL dereferencing
(the channel may haven't been properly configured to run the callback yet).
Fixes: 631e63a9f346 ("vmbus: change to per channel tasklet")
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Tested-by: Vitaly Kuznetsov <vkuznets@redhat.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.c | 12 | ||||
| -rw-r--r-- | drivers/hv/channel_mgmt.c | 19 | ||||
| -rw-r--r-- | include/linux/hyperv.h | 3 |
3 files changed, 4 insertions, 30 deletions
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index bd0d1988feb2..57b2958205c7 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c | |||
| @@ -530,15 +530,13 @@ static int vmbus_close_internal(struct vmbus_channel *channel) | |||
| 530 | int ret; | 530 | int ret; |
| 531 | 531 | ||
| 532 | /* | 532 | /* |
| 533 | * vmbus_on_event(), running in the tasklet, can race | 533 | * vmbus_on_event(), running in the per-channel tasklet, can race |
| 534 | * with vmbus_close_internal() in the case of SMP guest, e.g., when | 534 | * with vmbus_close_internal() in the case of SMP guest, e.g., when |
| 535 | * the former is accessing channel->inbound.ring_buffer, the latter | 535 | * the former is accessing channel->inbound.ring_buffer, the latter |
| 536 | * could be freeing the ring_buffer pages. | 536 | * could be freeing the ring_buffer pages, so here we must stop it |
| 537 | * | 537 | * first. |
| 538 | * To resolve the race, we can serialize them by disabling the | ||
| 539 | * tasklet when the latter is running here. | ||
| 540 | */ | 538 | */ |
| 541 | hv_event_tasklet_disable(channel); | 539 | tasklet_disable(&channel->callback_event); |
| 542 | 540 | ||
| 543 | /* | 541 | /* |
| 544 | * In case a device driver's probe() fails (e.g., | 542 | * In case a device driver's probe() fails (e.g., |
| @@ -605,8 +603,6 @@ static int vmbus_close_internal(struct vmbus_channel *channel) | |||
| 605 | get_order(channel->ringbuffer_pagecount * PAGE_SIZE)); | 603 | get_order(channel->ringbuffer_pagecount * PAGE_SIZE)); |
| 606 | 604 | ||
| 607 | out: | 605 | out: |
| 608 | hv_event_tasklet_enable(channel); | ||
| 609 | |||
| 610 | return ret; | 606 | return ret; |
| 611 | } | 607 | } |
| 612 | 608 | ||
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index d2cfa3eb71a2..bf846d078d85 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c | |||
| @@ -382,19 +382,6 @@ static void vmbus_release_relid(u32 relid) | |||
| 382 | true); | 382 | true); |
| 383 | } | 383 | } |
| 384 | 384 | ||
| 385 | void hv_event_tasklet_disable(struct vmbus_channel *channel) | ||
| 386 | { | ||
| 387 | tasklet_disable(&channel->callback_event); | ||
| 388 | } | ||
| 389 | |||
| 390 | void hv_event_tasklet_enable(struct vmbus_channel *channel) | ||
| 391 | { | ||
| 392 | tasklet_enable(&channel->callback_event); | ||
| 393 | |||
| 394 | /* In case there is any pending event */ | ||
| 395 | tasklet_schedule(&channel->callback_event); | ||
| 396 | } | ||
| 397 | |||
| 398 | void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid) | 385 | void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid) |
| 399 | { | 386 | { |
| 400 | unsigned long flags; | 387 | unsigned long flags; |
| @@ -403,7 +390,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid) | |||
| 403 | BUG_ON(!channel->rescind); | 390 | BUG_ON(!channel->rescind); |
| 404 | BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex)); | 391 | BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex)); |
| 405 | 392 | ||
| 406 | hv_event_tasklet_disable(channel); | ||
| 407 | if (channel->target_cpu != get_cpu()) { | 393 | if (channel->target_cpu != get_cpu()) { |
| 408 | put_cpu(); | 394 | put_cpu(); |
| 409 | smp_call_function_single(channel->target_cpu, | 395 | smp_call_function_single(channel->target_cpu, |
| @@ -412,7 +398,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid) | |||
| 412 | percpu_channel_deq(channel); | 398 | percpu_channel_deq(channel); |
| 413 | put_cpu(); | 399 | put_cpu(); |
| 414 | } | 400 | } |
| 415 | hv_event_tasklet_enable(channel); | ||
| 416 | 401 | ||
| 417 | if (channel->primary_channel == NULL) { | 402 | if (channel->primary_channel == NULL) { |
| 418 | list_del(&channel->listentry); | 403 | list_del(&channel->listentry); |
| @@ -506,7 +491,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) | |||
| 506 | 491 | ||
| 507 | init_vp_index(newchannel, dev_type); | 492 | init_vp_index(newchannel, dev_type); |
| 508 | 493 | ||
| 509 | hv_event_tasklet_disable(newchannel); | ||
| 510 | if (newchannel->target_cpu != get_cpu()) { | 494 | if (newchannel->target_cpu != get_cpu()) { |
| 511 | put_cpu(); | 495 | put_cpu(); |
| 512 | smp_call_function_single(newchannel->target_cpu, | 496 | smp_call_function_single(newchannel->target_cpu, |
| @@ -516,7 +500,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) | |||
| 516 | percpu_channel_enq(newchannel); | 500 | percpu_channel_enq(newchannel); |
| 517 | put_cpu(); | 501 | put_cpu(); |
| 518 | } | 502 | } |
| 519 | hv_event_tasklet_enable(newchannel); | ||
| 520 | 503 | ||
| 521 | /* | 504 | /* |
| 522 | * This state is used to indicate a successful open | 505 | * This state is used to indicate a successful open |
| @@ -566,7 +549,6 @@ err_deq_chan: | |||
| 566 | list_del(&newchannel->listentry); | 549 | list_del(&newchannel->listentry); |
| 567 | mutex_unlock(&vmbus_connection.channel_mutex); | 550 | mutex_unlock(&vmbus_connection.channel_mutex); |
| 568 | 551 | ||
| 569 | hv_event_tasklet_disable(newchannel); | ||
| 570 | if (newchannel->target_cpu != get_cpu()) { | 552 | if (newchannel->target_cpu != get_cpu()) { |
| 571 | put_cpu(); | 553 | put_cpu(); |
| 572 | smp_call_function_single(newchannel->target_cpu, | 554 | smp_call_function_single(newchannel->target_cpu, |
| @@ -575,7 +557,6 @@ err_deq_chan: | |||
| 575 | percpu_channel_deq(newchannel); | 557 | percpu_channel_deq(newchannel); |
| 576 | put_cpu(); | 558 | put_cpu(); |
| 577 | } | 559 | } |
| 578 | hv_event_tasklet_enable(newchannel); | ||
| 579 | 560 | ||
| 580 | vmbus_release_relid(newchannel->offermsg.child_relid); | 561 | vmbus_release_relid(newchannel->offermsg.child_relid); |
| 581 | 562 | ||
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index c4c7ae91f9d1..970771a5f739 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h | |||
| @@ -1437,9 +1437,6 @@ extern bool vmbus_prep_negotiate_resp(struct icmsg_hdr *icmsghdrp, u8 *buf, | |||
| 1437 | const int *srv_version, int srv_vercnt, | 1437 | const int *srv_version, int srv_vercnt, |
| 1438 | int *nego_fw_version, int *nego_srv_version); | 1438 | int *nego_fw_version, int *nego_srv_version); |
| 1439 | 1439 | ||
| 1440 | void hv_event_tasklet_disable(struct vmbus_channel *channel); | ||
| 1441 | void hv_event_tasklet_enable(struct vmbus_channel *channel); | ||
| 1442 | |||
| 1443 | void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid); | 1440 | void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid); |
| 1444 | 1441 | ||
| 1445 | void vmbus_setevent(struct vmbus_channel *channel); | 1442 | void vmbus_setevent(struct vmbus_channel *channel); |
