From: kys@exchange.microsoft.com
To: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, olaf@aepfle.de, apw@canonical.com,
vkuznets@redhat.com, jasowang@redhat.com,
leann.ogasawara@canonical.com, marcelo.cerri@canonical.com
Cc: Dexuan Cui <decui@microsoft.com>,
"K. Y. Srinivasan" <kys@microsoft.com>,
Haiyang Zhang <haiyangz@microsoft.com>,
Stephen Hemminger <sthemmin@microsoft.com>
Subject: [PATCH 2/4] vmbus: remove hv_event_tasklet_disable/enable
Date: Sat, 4 Mar 2017 18:13:58 -0700 [thread overview]
Message-ID: <1488676440-4105-2-git-send-email-kys@exchange.microsoft.com> (raw)
In-Reply-To: <1488676406-4021-1-git-send-email-kys@exchange.microsoft.com>
From: Dexuan Cui <decui@microsoft.com>
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>
---
drivers/hv/channel.c | 12 ++++--------
drivers/hv/channel_mgmt.c | 19 -------------------
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 81a80c8..a5f4c43 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -530,15 +530,13 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
int ret;
/*
- * vmbus_on_event(), running in the tasklet, can race
+ * vmbus_on_event(), running in the per-channel tasklet, can race
* with vmbus_close_internal() in the case of SMP guest, e.g., when
* the former is accessing channel->inbound.ring_buffer, the latter
- * could be freeing the ring_buffer pages.
- *
- * To resolve the race, we can serialize them by disabling the
- * tasklet when the latter is running here.
+ * could be freeing the ring_buffer pages, so here we must stop it
+ * first.
*/
- hv_event_tasklet_disable(channel);
+ tasklet_disable(&channel->callback_event);
/*
* In case a device driver's probe() fails (e.g.,
@@ -605,8 +603,6 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
get_order(channel->ringbuffer_pagecount * PAGE_SIZE));
out:
- hv_event_tasklet_enable(channel);
-
return ret;
}
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index d2cfa3e..bf846d0 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -382,19 +382,6 @@ static void vmbus_release_relid(u32 relid)
true);
}
-void hv_event_tasklet_disable(struct vmbus_channel *channel)
-{
- tasklet_disable(&channel->callback_event);
-}
-
-void hv_event_tasklet_enable(struct vmbus_channel *channel)
-{
- tasklet_enable(&channel->callback_event);
-
- /* In case there is any pending event */
- tasklet_schedule(&channel->callback_event);
-}
-
void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
{
unsigned long flags;
@@ -403,7 +390,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
BUG_ON(!channel->rescind);
BUG_ON(!mutex_is_locked(&vmbus_connection.channel_mutex));
- hv_event_tasklet_disable(channel);
if (channel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(channel->target_cpu,
@@ -412,7 +398,6 @@ void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid)
percpu_channel_deq(channel);
put_cpu();
}
- hv_event_tasklet_enable(channel);
if (channel->primary_channel == NULL) {
list_del(&channel->listentry);
@@ -506,7 +491,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
init_vp_index(newchannel, dev_type);
- hv_event_tasklet_disable(newchannel);
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
@@ -516,7 +500,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
percpu_channel_enq(newchannel);
put_cpu();
}
- hv_event_tasklet_enable(newchannel);
/*
* This state is used to indicate a successful open
@@ -566,7 +549,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
list_del(&newchannel->listentry);
mutex_unlock(&vmbus_connection.channel_mutex);
- hv_event_tasklet_disable(newchannel);
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
@@ -575,7 +557,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
percpu_channel_deq(newchannel);
put_cpu();
}
- hv_event_tasklet_enable(newchannel);
vmbus_release_relid(newchannel->offermsg.child_relid);
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index c4c7ae9..970771a 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,
const int *srv_version, int srv_vercnt,
int *nego_fw_version, int *nego_srv_version);
-void hv_event_tasklet_disable(struct vmbus_channel *channel);
-void hv_event_tasklet_enable(struct vmbus_channel *channel);
-
void hv_process_channel_removal(struct vmbus_channel *channel, u32 relid);
void vmbus_setevent(struct vmbus_channel *channel);
--
1.7.1
next prev parent reply other threads:[~2017-03-05 1:15 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-05 1:13 [PATCH 0/4] Drivers: hv: vmbus: Miscellaneous Fixes kys
2017-03-05 1:13 ` [PATCH 1/4] vmbus: use rcu for per-cpu channel list kys
2017-03-05 1:13 ` kys [this message]
2017-03-05 1:13 ` [PATCH 3/4] Drivers: hv: util: move waiting for release to hv_utils_transport itself kys
2017-03-05 1:14 ` [PATCH 4/4] Drivers: hv: util: don't forget to init host_ts.lock kys
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1488676440-4105-2-git-send-email-kys@exchange.microsoft.com \
--to=kys@exchange.microsoft.com \
--cc=apw@canonical.com \
--cc=decui@microsoft.com \
--cc=devel@linuxdriverproject.org \
--cc=gregkh@linuxfoundation.org \
--cc=haiyangz@microsoft.com \
--cc=jasowang@redhat.com \
--cc=kys@microsoft.com \
--cc=leann.ogasawara@canonical.com \
--cc=linux-kernel@vger.kernel.org \
--cc=marcelo.cerri@canonical.com \
--cc=olaf@aepfle.de \
--cc=sthemmin@microsoft.com \
--cc=vkuznets@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®