* [PATCH 0/5] Drivers: hv: vmbus: Some miscellaneous fixes @ 2016-12-07 9:15 kys 2016-12-07 9:16 ` [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys 0 siblings, 1 reply; 6+ messages in thread From: kys @ 2016-12-07 9:15 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara Cc: K. Y. Srinivasan From: K. Y. Srinivasan <kys@microsoft.com> Some miscellaneous fixes. K. Y. Srinivasan (1): Drivers: hv: vmbus: Prevent sending data on a rescinded channel Vitaly Kuznetsov (4): Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() hv: allocate synic pages for all present CPUs hv: init percpu_list in hv_synic_alloc() hv: don't reset hv_context.tsc_page on crash drivers/hv/channel.c | 17 +++++++++-------- drivers/hv/channel_mgmt.c | 10 ++++++---- drivers/hv/connection.c | 17 ++++++++++++----- drivers/hv/hv.c | 13 +++++++------ drivers/hv/hyperv_vmbus.h | 2 +- drivers/hv/ring_buffer.c | 7 +++++++ 6 files changed, 42 insertions(+), 24 deletions(-) -- 1.7.4.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() 2016-12-07 9:15 [PATCH 0/5] Drivers: hv: vmbus: Some miscellaneous fixes kys @ 2016-12-07 9:16 ` kys 2016-12-07 9:16 ` [PATCH 2/5] hv: allocate synic pages for all present CPUs kys ` (3 more replies) 0 siblings, 4 replies; 6+ messages in thread From: kys @ 2016-12-07 9:16 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara Cc: K. Y. Srinivasan, stable From: Vitaly Kuznetsov <vkuznets@redhat.com> DoS protection conditions were altered in WS2016 and now it's easy to get -EAGAIN returned from vmbus_post_msg() (e.g. when we try changing MTU on a netvsc device in a loop). All vmbus_post_msg() callers don't retry the operation and we usually end up with a non-functional device or crash. While host's DoS protection conditions are unknown to me my tests show that it can take up to 10 seconds before the message is sent so doing udelay() is not an option, we really need to sleep. Almost all vmbus_post_msg() callers are ready to sleep but there is one special case: vmbus_initiate_unload() which can be called from interrupt/NMI context and we can't sleep there. I'm also not sure about the lonely vmbus_send_tl_connect_request() which has no in-tree users but its external users are most likely waiting for the host to reply so sleeping there is also appropriate. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: <stable@vger.kernel.org> --- drivers/hv/channel.c | 17 +++++++++-------- drivers/hv/channel_mgmt.c | 10 ++++++---- drivers/hv/connection.c | 17 ++++++++++++----- drivers/hv/hyperv_vmbus.h | 2 +- 4 files changed, 28 insertions(+), 18 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 5fb4c6d..d5b8d9f 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -181,7 +181,7 @@ int vmbus_open(struct vmbus_channel *newchannel, u32 send_ringbuffer_size, spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); ret = vmbus_post_msg(open_msg, - sizeof(struct vmbus_channel_open_channel)); + sizeof(struct vmbus_channel_open_channel), true); if (ret != 0) { err = ret; @@ -233,7 +233,7 @@ int vmbus_send_tl_connect_request(const uuid_le *shv_guest_servie_id, conn_msg.guest_endpoint_id = *shv_guest_servie_id; conn_msg.host_service_id = *shv_host_servie_id; - return vmbus_post_msg(&conn_msg, sizeof(conn_msg)); + return vmbus_post_msg(&conn_msg, sizeof(conn_msg), true); } EXPORT_SYMBOL_GPL(vmbus_send_tl_connect_request); @@ -419,7 +419,7 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer, spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); ret = vmbus_post_msg(gpadlmsg, msginfo->msgsize - - sizeof(*msginfo)); + sizeof(*msginfo), true); if (ret != 0) goto cleanup; @@ -433,8 +433,8 @@ int vmbus_establish_gpadl(struct vmbus_channel *channel, void *kbuffer, gpadl_body->gpadl = next_gpadl_handle; ret = vmbus_post_msg(gpadl_body, - submsginfo->msgsize - - sizeof(*submsginfo)); + submsginfo->msgsize - sizeof(*submsginfo), + true); if (ret != 0) goto cleanup; @@ -485,8 +485,8 @@ int vmbus_teardown_gpadl(struct vmbus_channel *channel, u32 gpadl_handle) list_add_tail(&info->msglistentry, &vmbus_connection.chn_msg_list); spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); - ret = vmbus_post_msg(msg, - sizeof(struct vmbus_channel_gpadl_teardown)); + ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_gpadl_teardown), + true); if (ret) goto post_msg_err; @@ -557,7 +557,8 @@ static int vmbus_close_internal(struct vmbus_channel *channel) msg->header.msgtype = CHANNELMSG_CLOSECHANNEL; msg->child_relid = channel->offermsg.child_relid; - ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel)); + ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_close_channel), + true); if (ret) { pr_err("Close failed: close post msg return is %d\n", ret); diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 26b4192..b1e85d2 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -321,7 +321,8 @@ static void vmbus_release_relid(u32 relid) memset(&msg, 0, sizeof(struct vmbus_channel_relid_released)); msg.child_relid = relid; msg.header.msgtype = CHANNELMSG_RELID_RELEASED; - vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released)); + vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released), + true); } void hv_event_tasklet_disable(struct vmbus_channel *channel) @@ -728,7 +729,8 @@ void vmbus_initiate_unload(bool crash) init_completion(&vmbus_connection.unload_event); memset(&hdr, 0, sizeof(struct vmbus_channel_message_header)); hdr.msgtype = CHANNELMSG_UNLOAD; - vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header)); + vmbus_post_msg(&hdr, sizeof(struct vmbus_channel_message_header), + !crash); /* * vmbus_initiate_unload() is also called on crash and the crash can be @@ -1116,8 +1118,8 @@ int vmbus_request_offers(void) msg->msgtype = CHANNELMSG_REQUESTOFFERS; - ret = vmbus_post_msg(msg, - sizeof(struct vmbus_channel_message_header)); + ret = vmbus_post_msg(msg, sizeof(struct vmbus_channel_message_header), + true); if (ret != 0) { pr_err("Unable to request offers - %d\n", ret); diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index 6ce8b87..9b72ebc 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -111,7 +111,8 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo, spin_unlock_irqrestore(&vmbus_connection.channelmsg_lock, flags); ret = vmbus_post_msg(msg, - sizeof(struct vmbus_channel_initiate_contact)); + sizeof(struct vmbus_channel_initiate_contact), + true); if (ret != 0) { spin_lock_irqsave(&vmbus_connection.channelmsg_lock, flags); list_del(&msginfo->msglistentry); @@ -435,7 +436,7 @@ void vmbus_on_event(unsigned long data) /* * vmbus_post_msg - Send a msg on the vmbus's message connection */ -int vmbus_post_msg(void *buffer, size_t buflen) +int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep) { union hv_connection_id conn_id; int ret = 0; @@ -450,7 +451,7 @@ int vmbus_post_msg(void *buffer, size_t buflen) * insufficient resources. Retry the operation a couple of * times before giving up. */ - while (retries < 20) { + while (retries < 100) { ret = hv_post_message(conn_id, 1, buffer, buflen); switch (ret) { @@ -473,8 +474,14 @@ int vmbus_post_msg(void *buffer, size_t buflen) } retries++; - udelay(usec); - if (usec < 2048) + if (can_sleep && usec > 1000) + msleep(usec / 1000); + else if (usec < MAX_UDELAY_MS * 1000) + udelay(usec); + else + mdelay(usec / 1000); + + if (usec < 256000) usec *= 2; } return ret; diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 0675b39..27982df 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -683,7 +683,7 @@ struct hv_device *vmbus_device_create(const uuid_le *type, int vmbus_connect(void); void vmbus_disconnect(void); -int vmbus_post_msg(void *buffer, size_t buflen); +int vmbus_post_msg(void *buffer, size_t buflen, bool can_sleep); void vmbus_on_event(unsigned long data); void vmbus_on_msg_dpc(unsigned long data); -- 1.7.4.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/5] hv: allocate synic pages for all present CPUs 2016-12-07 9:16 ` [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys @ 2016-12-07 9:16 ` kys 2016-12-07 9:16 ` [PATCH 3/5] hv: init percpu_list in hv_synic_alloc() kys ` (2 subsequent siblings) 3 siblings, 0 replies; 6+ messages in thread From: kys @ 2016-12-07 9:16 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara Cc: K. Y. Srinivasan, stable From: Vitaly Kuznetsov <vkuznets@redhat.com> It may happen that not all CPUs are online when we do hv_synic_alloc() and in case more CPUs come online later we may try accessing these allocated structures. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: <stable@vger.kernel.org> --- drivers/hv/hv.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index 446802a..c326c67 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -411,7 +411,7 @@ int hv_synic_alloc(void) goto err; } - for_each_online_cpu(cpu) { + for_each_present_cpu(cpu) { hv_context.event_dpc[cpu] = kmalloc(size, GFP_ATOMIC); if (hv_context.event_dpc[cpu] == NULL) { pr_err("Unable to allocate event dpc\n"); @@ -482,7 +482,7 @@ void hv_synic_free(void) int cpu; kfree(hv_context.hv_numa_map); - for_each_online_cpu(cpu) + for_each_present_cpu(cpu) hv_synic_free_cpu(cpu); } -- 1.7.4.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/5] hv: init percpu_list in hv_synic_alloc() 2016-12-07 9:16 ` [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys 2016-12-07 9:16 ` [PATCH 2/5] hv: allocate synic pages for all present CPUs kys @ 2016-12-07 9:16 ` kys 2016-12-07 9:16 ` [PATCH 4/5] hv: don't reset hv_context.tsc_page on crash kys 2016-12-07 9:16 ` [PATCH 5/5] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys 3 siblings, 0 replies; 6+ messages in thread From: kys @ 2016-12-07 9:16 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara Cc: K. Y. Srinivasan, stable From: Vitaly Kuznetsov <vkuznets@redhat.com> Initializing hv_context.percpu_list in hv_synic_alloc() helps to prevent a crash in percpu_channel_enq() when not all CPUs were online during initialization and it naturally belongs there. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: <stable@vger.kernel.org> --- drivers/hv/hv.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index c326c67..c11393c 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -457,6 +457,8 @@ int hv_synic_alloc(void) pr_err("Unable to allocate post msg page\n"); goto err; } + + INIT_LIST_HEAD(&hv_context.percpu_list[cpu]); } return 0; @@ -552,8 +554,6 @@ void hv_synic_init(void *arg) rdmsrl(HV_X64_MSR_VP_INDEX, vp_index); hv_context.vp_index[cpu] = (u32)vp_index; - INIT_LIST_HEAD(&hv_context.percpu_list[cpu]); - /* * Register the per-cpu clockevent source. */ -- 1.7.4.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4/5] hv: don't reset hv_context.tsc_page on crash 2016-12-07 9:16 ` [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys 2016-12-07 9:16 ` [PATCH 2/5] hv: allocate synic pages for all present CPUs kys 2016-12-07 9:16 ` [PATCH 3/5] hv: init percpu_list in hv_synic_alloc() kys @ 2016-12-07 9:16 ` kys 2016-12-07 9:16 ` [PATCH 5/5] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys 3 siblings, 0 replies; 6+ messages in thread From: kys @ 2016-12-07 9:16 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara Cc: K. Y. Srinivasan, stable From: Vitaly Kuznetsov <vkuznets@redhat.com> It may happen that secondary CPUs are still alive and resetting hv_context.tsc_page will cause a consequent crash in read_hv_clock_tsc() as we don't check for it being not NULL there. It is safe as we're not freeing this page anyways. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: <stable@vger.kernel.org> --- drivers/hv/hv.c | 5 +++-- 1 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index c11393c..60f14c4 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c @@ -309,9 +309,10 @@ void hv_cleanup(bool crash) hypercall_msr.as_uint64 = 0; wrmsrl(HV_X64_MSR_REFERENCE_TSC, hypercall_msr.as_uint64); - if (!crash) + if (!crash) { vfree(hv_context.tsc_page); - hv_context.tsc_page = NULL; + hv_context.tsc_page = NULL; + } } #endif } -- 1.7.4.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 5/5] Drivers: hv: vmbus: Prevent sending data on a rescinded channel 2016-12-07 9:16 ` [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys ` (2 preceding siblings ...) 2016-12-07 9:16 ` [PATCH 4/5] hv: don't reset hv_context.tsc_page on crash kys @ 2016-12-07 9:16 ` kys 3 siblings, 0 replies; 6+ messages in thread From: kys @ 2016-12-07 9:16 UTC (permalink / raw) To: gregkh, linux-kernel, devel, olaf, apw, vkuznets, jasowang, leann.ogasawara Cc: K. Y. Srinivasan, Stable From: K. Y. Srinivasan <kys@microsoft.com> After the channel is rescinded, the host does not read from the rescinded channel. Fail writes to a channel that has already been rescinded. If we permit writes on a rescinded channel, since the host will not respond we will have situations where we will be unable to unload vmbus drivers that cannot have any outstanding requests to the host at the point they are unoaded. Signed-off-by: K. Y. Srinivasan <kys@microsoft.com> Cc: <Stable@vger.kernel.org> --- drivers/hv/ring_buffer.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/drivers/hv/ring_buffer.c b/drivers/hv/ring_buffer.c index cd49cb1..2cd4029 100644 --- a/drivers/hv/ring_buffer.c +++ b/drivers/hv/ring_buffer.c @@ -298,6 +298,9 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, unsigned long flags = 0; struct hv_ring_buffer_info *outring_info = &channel->outbound; + if (channel->rescind) + return -ENODEV; + for (i = 0; i < kv_count; i++) totalbytes_towrite += kv_list[i].iov_len; @@ -350,6 +353,10 @@ int hv_ringbuffer_write(struct vmbus_channel *channel, spin_unlock_irqrestore(&outring_info->ring_lock, flags); hv_signal_on_write(old_write, channel, kick_q); + + if (channel->rescind) + return -ENODEV; + return 0; } -- 1.7.4.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-07 7:36 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-12-07 9:15 [PATCH 0/5] Drivers: hv: vmbus: Some miscellaneous fixes kys 2016-12-07 9:16 ` [PATCH 1/5] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg() kys 2016-12-07 9:16 ` [PATCH 2/5] hv: allocate synic pages for all present CPUs kys 2016-12-07 9:16 ` [PATCH 3/5] hv: init percpu_list in hv_synic_alloc() kys 2016-12-07 9:16 ` [PATCH 4/5] hv: don't reset hv_context.tsc_page on crash kys 2016-12-07 9:16 ` [PATCH 5/5] Drivers: hv: vmbus: Prevent sending data on a rescinded channel kys
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®