* [PATCH 0/2] Drivers: hv: vmbus: Miscellaneous fixes
@ 2018-11-26 2:28 kys
2018-11-26 2:29 ` [PATCH 1/2] Drivers: hv: vmbus: check the creation_status in vmbus_establish_gpadl() kys
0 siblings, 1 reply; 8+ messages in thread
From: kys @ 2018-11-26 2:28 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin,
Michael.H.Kelley, vkuznets
Cc: K. Y. Srinivasan
From: "K. Y. Srinivasan" <kys@microsoft.com>
Miscellaneous fixes.
Dexuan Cui (2):
Drivers: hv: vmbus: check the creation_status in
vmbus_establish_gpadl()
Drivers: hv: vmbus: offload the handling of channels to two workqueues
drivers/hv/channel.c | 8 ++
drivers/hv/channel_mgmt.c | 188 +++++++++++++++++++++++++-------------
drivers/hv/connection.c | 24 ++++-
drivers/hv/hyperv_vmbus.h | 7 ++
include/linux/hyperv.h | 7 ++
5 files changed, 169 insertions(+), 65 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] Drivers: hv: vmbus: check the creation_status in vmbus_establish_gpadl()
2018-11-26 2:28 [PATCH 0/2] Drivers: hv: vmbus: Miscellaneous fixes kys
@ 2018-11-26 2:29 ` kys
2018-11-26 2:29 ` [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues kys
[not found] ` <20181126095254.824D420672@mail.kernel.org>
0 siblings, 2 replies; 8+ messages in thread
From: kys @ 2018-11-26 2:29 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin,
Michael.H.Kelley, vkuznets
Cc: Dexuan Cui, K . Y . Srinivasan, Haiyang Zhang, stable
From: Dexuan Cui <decui@microsoft.com>
This is a longstanding issue: if the vmbus upper-layer drivers try to
consume too many GPADLs, the host may return with an error
0xC0000044 (STATUS_QUOTA_EXCEEDED), but currently we forget to check
the creation_status, and hence we can pass an invalid GPADL handle
into the OPEN_CHANNEL message, and get an error code 0xc0000225 in
open_info->response.open_result.status, and finally we hang in
vmbus_open() -> "goto error_free_info" -> vmbus_teardown_gpadl().
With this patch, we can exit gracefully on STATUS_QUOTA_EXCEEDED.
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: stable@vger.kernel.org
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
drivers/hv/channel.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index f96a77b18bb9..ce0ba2062723 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -516,6 +516,14 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
}
wait_for_completion(&msginfo->waitevent);
+ if (msginfo->response.gpadl_created.creation_status != 0) {
+ pr_err("Failed to establish GPADL: err = 0x%x\n",
+ msginfo->response.gpadl_created.creation_status);
+
+ ret = -EDQUOT;
+ goto cleanup;
+ }
+
if (channel->rescind) {
ret = -ENODEV;
goto cleanup;
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
2018-11-26 2:29 ` [PATCH 1/2] Drivers: hv: vmbus: check the creation_status in vmbus_establish_gpadl() kys
@ 2018-11-26 2:29 ` kys
2018-11-26 5:23 ` Sasha Levin
2018-11-26 19:35 ` Greg KH
[not found] ` <20181126095254.824D420672@mail.kernel.org>
1 sibling, 2 replies; 8+ messages in thread
From: kys @ 2018-11-26 2:29 UTC (permalink / raw)
To: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin,
Michael.H.Kelley, vkuznets
Cc: Dexuan Cui, stable, K . Y . Srinivasan, Haiyang Zhang
From: Dexuan Cui <decui@microsoft.com>
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-rc3.
So the patch should be applied to all the existing kernels.
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>
---
drivers/hv/channel_mgmt.c | 188 +++++++++++++++++++++++++-------------
drivers/hv/connection.c | 24 ++++-
drivers/hv/hyperv_vmbus.h | 7 ++
include/linux/hyperv.h | 7 ++
4 files changed, 161 insertions(+), 65 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 82e673671087..d01689079e9b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -434,60 +434,16 @@ void vmbus_free_channels(void)
}
}
-/*
- * vmbus_process_offer - Process the offer by creating a channel/device
- * associated with this offer
- */
-static void vmbus_process_offer(struct vmbus_channel *newchannel)
+/* Note: the function can run concurrently for primary/sub channels. */
+static void vmbus_add_channel_work(struct work_struct *work)
{
- struct vmbus_channel *channel;
- bool fnew = true;
+ struct vmbus_channel *newchannel =
+ container_of(work, struct vmbus_channel, add_channel_work);
+ struct vmbus_channel *primary_channel = newchannel->primary_channel;
unsigned long flags;
u16 dev_type;
int ret;
- /* Make sure this is a new offer */
- mutex_lock(&vmbus_connection.channel_mutex);
-
- /*
- * Now that we have acquired the channel_mutex,
- * we can release the potentially racing rescind thread.
- */
- atomic_dec(&vmbus_connection.offer_in_progress);
-
- list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
- if (!uuid_le_cmp(channel->offermsg.offer.if_type,
- newchannel->offermsg.offer.if_type) &&
- !uuid_le_cmp(channel->offermsg.offer.if_instance,
- newchannel->offermsg.offer.if_instance)) {
- fnew = false;
- break;
- }
- }
-
- if (fnew)
- list_add_tail(&newchannel->listentry,
- &vmbus_connection.chn_list);
-
- mutex_unlock(&vmbus_connection.channel_mutex);
-
- if (!fnew) {
- /*
- * Check to see if this is a sub-channel.
- */
- if (newchannel->offermsg.offer.sub_channel_index != 0) {
- /*
- * Process the sub-channel.
- */
- newchannel->primary_channel = channel;
- spin_lock_irqsave(&channel->lock, flags);
- list_add_tail(&newchannel->sc_list, &channel->sc_list);
- spin_unlock_irqrestore(&channel->lock, flags);
- } else {
- goto err_free_chan;
- }
- }
-
dev_type = hv_get_dev_type(newchannel);
init_vp_index(newchannel, dev_type);
@@ -505,27 +461,26 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
/*
* This state is used to indicate a successful open
* so that when we do close the channel normally, we
- * can cleanup properly
+ * can cleanup properly.
*/
newchannel->state = CHANNEL_OPEN_STATE;
- if (!fnew) {
- struct hv_device *dev
- = newchannel->primary_channel->device_obj;
+ if (primary_channel != NULL) {
+ /* newchannel is a sub-channel. */
+ struct hv_device *dev = primary_channel->device_obj;
if (vmbus_add_channel_kobj(dev, newchannel))
- goto err_free_chan;
+ goto err_deq_chan;
+
+ if (primary_channel->sc_creation_callback != NULL)
+ primary_channel->sc_creation_callback(newchannel);
- if (channel->sc_creation_callback != NULL)
- channel->sc_creation_callback(newchannel);
newchannel->probe_done = true;
return;
}
/*
- * Start the process of binding this offer to the driver
- * We need to set the DeviceObject field before calling
- * vmbus_child_dev_add()
+ * Start the process of binding the primary channel to the driver
*/
newchannel->device_obj = vmbus_device_create(
&newchannel->offermsg.offer.if_type,
@@ -554,13 +509,28 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
err_deq_chan:
mutex_lock(&vmbus_connection.channel_mutex);
- list_del(&newchannel->listentry);
+
+ /*
+ * We need to set the flag, otherwise
+ * vmbus_onoffer_rescind() can be blocked.
+ */
+ newchannel->probe_done = true;
+
+ if (primary_channel == NULL) {
+ list_del(&newchannel->listentry);
+ } else {
+ spin_lock_irqsave(&primary_channel->lock, flags);
+ list_del(&newchannel->sc_list);
+ spin_unlock_irqrestore(&primary_channel->lock, flags);
+ }
+
mutex_unlock(&vmbus_connection.channel_mutex);
if (newchannel->target_cpu != get_cpu()) {
put_cpu();
smp_call_function_single(newchannel->target_cpu,
- percpu_channel_deq, newchannel, true);
+ percpu_channel_deq,
+ newchannel, true);
} else {
percpu_channel_deq(newchannel);
put_cpu();
@@ -568,14 +538,104 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
vmbus_release_relid(newchannel->offermsg.child_relid);
-err_free_chan:
free_channel(newchannel);
}
+/*
+ * vmbus_process_offer - Process the offer by creating a channel/device
+ * associated with this offer
+ */
+static void vmbus_process_offer(struct vmbus_channel *newchannel)
+{
+ struct vmbus_channel *channel;
+ struct workqueue_struct *wq;
+ unsigned long flags;
+ bool fnew = true;
+
+ mutex_lock(&vmbus_connection.channel_mutex);
+
+ /*
+ * Now that we have acquired the channel_mutex,
+ * we can release the potentially racing rescind thread.
+ */
+ atomic_dec(&vmbus_connection.offer_in_progress);
+
+ list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
+ if (!uuid_le_cmp(channel->offermsg.offer.if_type,
+ newchannel->offermsg.offer.if_type) &&
+ !uuid_le_cmp(channel->offermsg.offer.if_instance,
+ newchannel->offermsg.offer.if_instance)) {
+ fnew = false;
+ break;
+ }
+ }
+
+ if (fnew)
+ list_add_tail(&newchannel->listentry,
+ &vmbus_connection.chn_list);
+ else {
+ /*
+ * Check to see if this is a valid sub-channel.
+ */
+ if (newchannel->offermsg.offer.sub_channel_index == 0) {
+ mutex_unlock(&vmbus_connection.channel_mutex);
+ /*
+ * Don't call free_channel(), because newchannel->kobj
+ * is not initialized yet.
+ */
+ kfree(newchannel);
+ WARN_ON_ONCE(1);
+ return;
+ }
+ /*
+ * Process the sub-channel.
+ */
+ newchannel->primary_channel = channel;
+ spin_lock_irqsave(&channel->lock, flags);
+ list_add_tail(&newchannel->sc_list, &channel->sc_list);
+ spin_unlock_irqrestore(&channel->lock, flags);
+ }
+
+ mutex_unlock(&vmbus_connection.channel_mutex);
+
+ /*
+ * 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.
+ */
+ INIT_WORK(&newchannel->add_channel_work, vmbus_add_channel_work);
+ wq = fnew ? vmbus_connection.handle_primary_chan_wq :
+ vmbus_connection.handle_sub_chan_wq;
+ queue_work(wq, &newchannel->add_channel_work);
+}
+
/*
* We use this state to statically distribute the channel interrupt load.
*/
static int next_numa_node_id;
+/*
+ * init_vp_index() accesses global variables like next_numa_node_id, and
+ * it can run concurrently for primary channels and sub-channels: see
+ * vmbus_process_offer(), so we need the lock to protect the global
+ * variables.
+ */
+static DEFINE_SPINLOCK(bind_channel_to_cpu_lock);
/*
* Starting with Win8, we can statically distribute the incoming
@@ -611,6 +671,8 @@ static void init_vp_index(struct vmbus_channel *channel, u16 dev_type)
return;
}
+ spin_lock(&bind_channel_to_cpu_lock);
+
/*
* Based on the channel affinity policy, we will assign the NUMA
* nodes.
@@ -693,6 +755,8 @@ static void init_vp_index(struct vmbus_channel *channel, u16 dev_type)
channel->target_cpu = cur_cpu;
channel->target_vp = hv_cpu_number_to_vp_number(cur_cpu);
+ spin_unlock(&bind_channel_to_cpu_lock);
+
free_cpumask_var(available_mask);
}
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index f4d08c8ac7f8..4fe117b761ce 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -190,6 +190,20 @@ int vmbus_connect(void)
goto cleanup;
}
+ vmbus_connection.handle_primary_chan_wq =
+ create_workqueue("hv_pri_chan");
+ if (!vmbus_connection.handle_primary_chan_wq) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
+ vmbus_connection.handle_sub_chan_wq =
+ create_workqueue("hv_sub_chan");
+ if (!vmbus_connection.handle_sub_chan_wq) {
+ ret = -ENOMEM;
+ goto cleanup;
+ }
+
INIT_LIST_HEAD(&vmbus_connection.chn_msg_list);
spin_lock_init(&vmbus_connection.channelmsg_lock);
@@ -280,10 +294,14 @@ void vmbus_disconnect(void)
*/
vmbus_initiate_unload(false);
- if (vmbus_connection.work_queue) {
- drain_workqueue(vmbus_connection.work_queue);
+ if (vmbus_connection.handle_sub_chan_wq)
+ destroy_workqueue(vmbus_connection.handle_sub_chan_wq);
+
+ if (vmbus_connection.handle_primary_chan_wq)
+ destroy_workqueue(vmbus_connection.handle_primary_chan_wq);
+
+ if (vmbus_connection.work_queue)
destroy_workqueue(vmbus_connection.work_queue);
- }
if (vmbus_connection.int_page) {
free_pages((unsigned long)vmbus_connection.int_page, 0);
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index f17c06a5e74b..313a07f5efa1 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -333,7 +333,14 @@ struct vmbus_connection {
struct list_head chn_list;
struct mutex channel_mutex;
+ /*
+ * An offer message is handled first on the work_queue, and then
+ * is further handled on handle_primary_chan_wq or
+ * handle_sub_chan_wq.
+ */
struct workqueue_struct *work_queue;
+ struct workqueue_struct *handle_primary_chan_wq;
+ struct workqueue_struct *handle_sub_chan_wq;
};
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 07a367f5e22f..f0885cc01db6 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -896,6 +896,13 @@ struct vmbus_channel {
bool probe_done;
+ /*
+ * We must offload the handling of the primary/sub channels
+ * from the single-threaded vmbus_connection.work_queue to
+ * two different workqueue, otherwise we can block
+ * vmbus_connection.work_queue and hang: see vmbus_process_offer().
+ */
+ struct work_struct add_channel_work;
};
static inline bool is_hvsock_channel(const struct vmbus_channel *c)
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
2018-11-26 2:29 ` [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues kys
@ 2018-11-26 5:23 ` Sasha Levin
2018-11-26 19:35 ` Greg KH
1 sibling, 0 replies; 8+ messages in thread
From: Sasha Levin @ 2018-11-26 5:23 UTC (permalink / raw)
To: kys
Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang, sthemmin,
Michael.H.Kelley, vkuznets, Dexuan Cui, stable, Haiyang Zhang
On Mon, Nov 26, 2018 at 02:29:57AM +0000, kys@linuxonhyperv.com wrote:
>From: Dexuan Cui <decui@microsoft.com>
>
>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-rc3.
>
>So the patch should be applied to all the existing kernels.
>
>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>
This patch doesn't apply on next/linus/char-misc; there seems to be a
missing patch that touches vmbus_process_offer() which isn't a part of
this series.
--
Thanks,
Sasha
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
2018-11-26 2:29 ` [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues kys
2018-11-26 5:23 ` Sasha Levin
@ 2018-11-26 19:35 ` Greg KH
2018-11-26 21:12 ` Dexuan Cui
2018-11-27 5:22 ` KY Srinivasan
1 sibling, 2 replies; 8+ messages in thread
From: Greg KH @ 2018-11-26 19:35 UTC (permalink / raw)
To: kys
Cc: linux-kernel, devel, olaf, apw, jasowang, sthemmin,
Michael.H.Kelley, vkuznets, Haiyang Zhang, stable
On Mon, Nov 26, 2018 at 02:29:57AM +0000, kys@linuxonhyperv.com wrote:
> From: Dexuan Cui <decui@microsoft.com>
>
> 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-rc3.
>
> So the patch should be applied to all the existing kernels.
>
> 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>
> ---
> drivers/hv/channel_mgmt.c | 188 +++++++++++++++++++++++++-------------
> drivers/hv/connection.c | 24 ++++-
> drivers/hv/hyperv_vmbus.h | 7 ++
> include/linux/hyperv.h | 7 ++
> 4 files changed, 161 insertions(+), 65 deletions(-)
As Sasha pointed out, this patch does not even apply :(
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 1/2] Drivers: hv: vmbus: check the creation_status in vmbus_establish_gpadl()
[not found] ` <20181126095254.824D420672@mail.kernel.org>
@ 2018-11-26 21:02 ` Dexuan Cui
0 siblings, 0 replies; 8+ messages in thread
From: Dexuan Cui @ 2018-11-26 21:02 UTC (permalink / raw)
To: Sasha Levin, gregkh, linux-kernel
Cc: Stephen Hemminger, KY Srinivasan, Haiyang Zhang, stable, stable
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
> From: Sasha Levin <sashal@kernel.org>
> Sent: Monday, November 26, 2018 1:53 AM
> Hi,
>
> [This is an automated email]
>
> This commit has been processed because it contains a -stable tag.
> The stable tag indicates that it's relevant for the following trees: all
>
> The bot has tested the following trees: v4.19.4, v4.14.83, v4.9.140, v4.4.164,
> v3.18.126.
>
> v4.19.4: Build OK!
> v4.14.83: Build OK!
> v4.9.140: Build OK!
> v4.4.164: Failed to apply! Possible dependencies:
> v3.18.126: Failed to apply! Possible dependencies:
> How should we proceed with this patch?
>
> Sasha
Hi Sasha,
Please see the attached patch, which is a rebase for both v4.4.164 and v3.18.126.
Thanks,
--Dexuan
[-- Attachment #2: 0001-Drivers-hv-vmbus-check-the-creation_status-in-vmbus_.patch --]
[-- Type: application/octet-stream, Size: 1770 bytes --]
From 038481c8b771bd22e40cd5d622dc147d486b97c8 Mon Sep 17 00:00:00 2001
From: Dexuan Cui <decui@microsoft.com>
Date: Mon, 26 Nov 2018 02:29:56 +0000
Subject: [PATCH] Drivers: hv: vmbus: check the creation_status in
vmbus_establish_gpadl()
This is a longstanding issue: if the vmbus upper-layer drivers try to
consume too many GPADLs, the host may return with an error
0xC0000044 (STATUS_QUOTA_EXCEEDED), but currently we forget to check
the creation_status, and hence we can pass an invalid GPADL handle
into the OPEN_CHANNEL message, and get an error code 0xc0000225 in
open_info->response.open_result.status, and finally we hang in
vmbus_open() -> "goto error_free_info" -> vmbus_teardown_gpadl().
With this patch, we can exit gracefully on STATUS_QUOTA_EXCEEDED.
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: stable@vger.kernel.org
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>
---
drivers/hv/channel.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index d037454..b81516c 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -424,6 +424,14 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer,
}
wait_for_completion(&msginfo->waitevent);
+ if (msginfo->response.gpadl_created.creation_status != 0) {
+ pr_err("Failed to establish GPADL: err = 0x%x\n",
+ msginfo->response.gpadl_created.creation_status);
+
+ ret = -EDQUOT;
+ goto cleanup;
+ }
+
/* At this point, we received the gpadl created msg */
*gpadl_handle = gpadlmsg->gpadl;
--
2.7.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
2018-11-26 19:35 ` Greg KH
@ 2018-11-26 21:12 ` Dexuan Cui
2018-11-27 5:22 ` KY Srinivasan
1 sibling, 0 replies; 8+ messages in thread
From: Dexuan Cui @ 2018-11-26 21:12 UTC (permalink / raw)
To: Greg KH, KY Srinivasan
Cc: olaf, Stephen Hemminger, jasowang, linux-kernel, stable,
Michael Kelley, apw, devel, vkuznets, Haiyang Zhang
> From: devel <driverdev-devel-bounces@linuxdriverproject.org> On Behalf Of
> Greg KH
> Sent: Monday, November 26, 2018 11:35 AM
> As Sasha pointed out, this patch does not even apply :(
Sorry, I'll rebase to char-misc's char-misc-testing branch, which has had one of the patches, i.e.
4d3c5c69191f ("Drivers: hv: vmbus: Remove the useless API vmbus_get_outgoing_channel()")
Thanks,
-- Dexuan
^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues
2018-11-26 19:35 ` Greg KH
2018-11-26 21:12 ` Dexuan Cui
@ 2018-11-27 5:22 ` KY Srinivasan
1 sibling, 0 replies; 8+ messages in thread
From: KY Srinivasan @ 2018-11-27 5:22 UTC (permalink / raw)
To: Greg KH
Cc: linux-kernel, devel, olaf, apw, jasowang, Stephen Hemminger,
Michael Kelley, vkuznets, Haiyang Zhang, stable
> -----Original Message-----
> From: Greg KH <gregkh@linuxfoundation.org>
> Sent: Monday, November 26, 2018 11:35 AM
> To: KY Srinivasan <kys@microsoft.com>
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; jasowang@redhat.com; Stephen
> Hemminger <sthemmin@microsoft.com>; Michael Kelley
> <mikelley@microsoft.com>; vkuznets <vkuznets@redhat.com>; Haiyang
> Zhang <haiyangz@microsoft.com>; stable@vger.kernel.org
> Subject: Re: [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels
> to two workqueues
>
> On Mon, Nov 26, 2018 at 02:29:57AM +0000, kys@linuxonhyperv.com wrote:
> > From: Dexuan Cui <decui@microsoft.com>
> >
> > 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-rc3.
> >
> > So the patch should be applied to all the existing kernels.
> >
> > 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>
> > ---
> > drivers/hv/channel_mgmt.c | 188 +++++++++++++++++++++++++---------
> ----
> > drivers/hv/connection.c | 24 ++++-
> > drivers/hv/hyperv_vmbus.h | 7 ++
> > include/linux/hyperv.h | 7 ++
> > 4 files changed, 161 insertions(+), 65 deletions(-)
>
> As Sasha pointed out, this patch does not even apply :(
Sorry about that. These patches applied cleanly on my tree (misc-next).
This series is to be applied on top of
patch 0001-Drivers-hv-vmbus-Remove-the-useless-API-vmbus_get_ou.patch
While the patch 0001-Drivers-hv-vmbus-Remove-the-useless-API-vmbus_get_ou.patch
has been committed to the char-misc-testing branch, it is not in the misc-linus branch and
that is the reason for this problem.
Regards,
K. Y
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-11-27 5:22 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-26 2:28 [PATCH 0/2] Drivers: hv: vmbus: Miscellaneous fixes kys
2018-11-26 2:29 ` [PATCH 1/2] Drivers: hv: vmbus: check the creation_status in vmbus_establish_gpadl() kys
2018-11-26 2:29 ` [PATCH 2/2] Drivers: hv: vmbus: offload the handling of channels to two workqueues kys
2018-11-26 5:23 ` Sasha Levin
2018-11-26 19:35 ` Greg KH
2018-11-26 21:12 ` Dexuan Cui
2018-11-27 5:22 ` KY Srinivasan
[not found] ` <20181126095254.824D420672@mail.kernel.org>
2018-11-26 21:02 ` [PATCH 1/2] Drivers: hv: vmbus: check the creation_status in vmbus_establish_gpadl() Dexuan Cui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).