[PATCH 0/2] Drivers: hv: vmbus: Miscellaneous cleanup

[PATCH 0/2] Drivers: hv: vmbus: Miscellaneous cleanup

[K. Y. Srinivasan]

Cleanup hv_post_message() to minimize failures. Also disable preemption
when sampling CPU ID when preemption is otherwise possible.

K. Y. Srinivasan (2):
  Drivers: hv: vmbus: Cleanup hv_post_message()
  Drivers: hv: vmbus: Properly protect calls to smp_processor_id()

 drivers/hv/channel.c      |    7 +++++--
 drivers/hv/channel_mgmt.c |   21 +++++++++++++++------
 drivers/hv/hv.c           |   28 ++++++++++++++++------------
 drivers/hv/hyperv_vmbus.h |    4 ++++
 4 files changed, 40 insertions(+), 20 deletions(-)

-- 
1.7.4.1

[PATCH 1/2] Drivers: hv: vmbus: Cleanup hv_post_message()

[K. Y. Srinivasan]

Minimize failures in this function by pre-allocating the buffer
for posting messages. The hypercall for posting the message can fail
for a number of reasons:

        1. Transient resource related issues
        2. Buffer alignment
        3. Buffer cannot span a page boundry

We address issues 2 and 3 by preallocating a per-cpu page for the buffer.
Transient resource related failures are handled by retrying by the callers
of this function.

This patch is based on the investigation
done by Dexuan Cui <decui@microsoft.com>.

I would like to thank Sitsofe Wheeler <sitsofe@yahoo.com>
for reporting the issue and helping in debuggging.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reported-by: Sitsofe Wheeler <sitsofe@yahoo.com>
Cc: <stable@vger.kernel.org>
---
 drivers/hv/hv.c           |   27 +++++++++++++++------------
 drivers/hv/hyperv_vmbus.h |    4 ++++
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index edfc848..3e4235c 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -138,6 +138,8 @@ int hv_init(void)
 	memset(hv_context.synic_event_page, 0, sizeof(void *) * NR_CPUS);
 	memset(hv_context.synic_message_page, 0,
 	       sizeof(void *) * NR_CPUS);
+	memset(hv_context.post_msg_page, 0,
+	       sizeof(void *) * NR_CPUS);
 	memset(hv_context.vp_index, 0,
 	       sizeof(int) * NR_CPUS);
 	memset(hv_context.event_dpc, 0,
@@ -217,26 +219,18 @@ int hv_post_message(union hv_connection_id connection_id,
 		  enum hv_message_type message_type,
 		  void *payload, size_t payload_size)
 {
-	struct aligned_input {
-		u64 alignment8;
-		struct hv_input_post_message msg;
-	};
 
 	struct hv_input_post_message *aligned_msg;
 	u16 status;
-	unsigned long addr;
 
 	if (payload_size > HV_MESSAGE_PAYLOAD_BYTE_COUNT)
 		return -EMSGSIZE;
 
-	addr = (unsigned long)kmalloc(sizeof(struct aligned_input), GFP_ATOMIC);
-	if (!addr)
-		return -ENOMEM;
-
 	aligned_msg = (struct hv_input_post_message *)
-			(ALIGN(addr, HV_HYPERCALL_PARAM_ALIGN));
+			hv_context.post_msg_page[get_cpu()];
 
 	aligned_msg->connectionid = connection_id;
+	aligned_msg->reserved = 0;
 	aligned_msg->message_type = message_type;
 	aligned_msg->payload_size = payload_size;
 	memcpy((void *)aligned_msg->payload, payload, payload_size);
@@ -244,8 +238,7 @@ int hv_post_message(union hv_connection_id connection_id,
 	status = do_hypercall(HVCALL_POST_MESSAGE, aligned_msg, NULL)
 		& 0xFFFF;
 
-	kfree((void *)addr);
-
+	put_cpu();
 	return status;
 }
 
@@ -294,6 +287,14 @@ int hv_synic_alloc(void)
 			pr_err("Unable to allocate SYNIC event page\n");
 			goto err;
 		}
+
+		hv_context.post_msg_page[cpu] =
+			(void *)get_zeroed_page(GFP_ATOMIC);
+
+		if (hv_context.post_msg_page[cpu] == NULL) {
+			pr_err("Unable to allocate post msg page\n");
+			goto err;
+		}
 	}
 
 	return 0;
@@ -308,6 +309,8 @@ static void hv_synic_free_cpu(int cpu)
 		free_page((unsigned long)hv_context.synic_event_page[cpu]);
 	if (hv_context.synic_message_page[cpu])
 		free_page((unsigned long)hv_context.synic_message_page[cpu]);
+	if (hv_context.post_msg_page[cpu])
+		free_page((unsigned long)hv_context.post_msg_page[cpu]);
 }
 
 void hv_synic_free(void)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 22b7507..c386d8d 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -515,6 +515,10 @@ struct hv_context {
 	 * per-cpu list of the channels based on their CPU affinity.
 	 */
 	struct list_head percpu_list[NR_CPUS];
+	/*
+	 * buffer to post messages to the host.
+	 */
+	void *post_msg_page[NR_CPUS];
 };
 
 extern struct hv_context hv_context;
-- 
1.7.4.1

[PATCH 2/2] Drivers: hv: vmbus: Properly protect calls to smp_processor_id()

[K. Y. Srinivasan]

Disable preemption when sampling current processor ID when preemption
is otherwise possible.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c      |    7 +++++--
 drivers/hv/channel_mgmt.c |   21 +++++++++++++++------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index 19bad59..433f72a 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -486,11 +486,14 @@ static int vmbus_close_internal(struct vmbus_channel *channel)
 	channel->state = CHANNEL_OPEN_STATE;
 	channel->sc_creation_callback = NULL;
 	/* Stop callback and cancel the timer asap */
-	if (channel->target_cpu != smp_processor_id())
+	if (channel->target_cpu != get_cpu()) {
+		put_cpu();
 		smp_call_function_single(channel->target_cpu, reset_channel_cb,
 					 channel, true);
-	else
+	} else {
 		reset_channel_cb(channel);
+		put_cpu();
+	}
 
 	/* Send a closing message */
 
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index ed9350d..a2d1a96 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -224,11 +224,14 @@ static void vmbus_process_rescind_offer(struct work_struct *work)
 	msg.header.msgtype = CHANNELMSG_RELID_RELEASED;
 	vmbus_post_msg(&msg, sizeof(struct vmbus_channel_relid_released));
 
-	if (channel->target_cpu != smp_processor_id())
+	if (channel->target_cpu != get_cpu()) {
+		put_cpu();
 		smp_call_function_single(channel->target_cpu,
 					 percpu_channel_deq, channel, true);
-	else
+	} else {
 		percpu_channel_deq(channel);
+		put_cpu();
+	}
 
 	if (channel->primary_channel == NULL) {
 		spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
@@ -294,12 +297,15 @@ static void vmbus_process_offer(struct work_struct *work)
 	spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
 
 	if (enq) {
-		if (newchannel->target_cpu != smp_processor_id())
+		if (newchannel->target_cpu != get_cpu()) {
+			put_cpu();
 			smp_call_function_single(newchannel->target_cpu,
 						 percpu_channel_enq,
 						 newchannel, true);
-		else
+		} else {
 			percpu_channel_enq(newchannel);
+			put_cpu();
+		}
 	}
 	if (!fnew) {
 		/*
@@ -314,12 +320,15 @@ static void vmbus_process_offer(struct work_struct *work)
 			list_add_tail(&newchannel->sc_list, &channel->sc_list);
 			spin_unlock_irqrestore(&channel->sc_lock, flags);
 
-			if (newchannel->target_cpu != smp_processor_id())
+			if (newchannel->target_cpu != get_cpu()) {
+				put_cpu();
 				smp_call_function_single(newchannel->target_cpu,
 							 percpu_channel_enq,
 							 newchannel, true);
-			else
+			} else {
 				percpu_channel_enq(newchannel);
+				put_cpu();
+			}
 
 			newchannel->state = CHANNEL_OPEN_STATE;
 			if (channel->sc_creation_callback != NULL)
-- 
1.7.4.1

Re: [PATCH 0/2] Drivers: hv: vmbus: Miscellaneous cleanup

[Sitsofe Wheeler]

On Thu, Aug 28, 2014 at 06:29:33PM -0700, K. Y. Srinivasan wrote:
> Cleanup hv_post_message() to minimize failures. Also disable preemption
> when sampling CPU ID when preemption is otherwise possible.
> 
> K. Y. Srinivasan (2):
>   Drivers: hv: vmbus: Cleanup hv_post_message()
>   Drivers: hv: vmbus: Properly protect calls to smp_processor_id()

These patches (on top of the "Drivers: hv: vmbus: Eliminate calls to
BUG_ON()" series) resolve an issue I was seeing  where the Hyper-V
framebuffer driver would fail when booting a 3.17.0-rc2 verification
kernel on a UP system.

Tested-by: Sitsofe Wheeler <sitsofe@yahoo.com>

-- 
Sitsofe | http://sucs.org/~sits/

Re: [PATCH 1/2] Drivers: hv: vmbus: Cleanup hv_post_message()

[Sitsofe Wheeler]

On Thu, Aug 28, 2014 at 06:29:52PM -0700, K. Y. Srinivasan wrote:
> Minimize failures in this function by pre-allocating the buffer
> for posting messages. The hypercall for posting the message can fail
> for a number of reasons:
> 
<snip>
> 
> Cc: <stable@vger.kernel.org>

Is this patch useful for all longterm (2.6.32.63, 3.2.62, 3.4.103,
3.10.53, 3.12.27, 3.14.17) in addition to stable (3.16.1) kernels? I've
noticed some other LKML messages sometimes have the following (but there
doesn't seem to be a standard):

Cc: stable@vger.kernel.org # 3.4+

-- 
Sitsofe | http://sucs.org/~sits/