LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V2 0/7] Drivers: hv: Miscellaneous fixes 
@ 2015-03-07  7:04 K. Y. Srinivasan
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
  0 siblings, 1 reply; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan

This patch-set has miscellaneous fixes for both the VMBUS as well as the balloon
driver. There is also a fix for the hv tools makefile.

In this version of the patchset, I have included a patch from Dexuan.
Furthermore, I have addressed a memory leak issue in the patch:
Drivers: hv: vmbus: Perform device register in the per-channel work element.

Dan Carpenter (1):
  hv: vmbus: missing curly braces in vmbus_process_offer()

Dexuan Cui (1):
  tools: hv: fcopy_daemon: support >2GB files for x86_32 guest

K. Y. Srinivasan (2):
  Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
  Drivers: hv: vmbus: Perform device register in the per-channel work
    element

Nick Meier (1):
  Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY

Vitaly Kuznetsov (2):
  Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure
  Drivers: hv: hv_balloon: don't lose memory when onlining order is not
    natural

 drivers/hv/channel.c      |    1 +
 drivers/hv/channel_mgmt.c |  146 +++++++++++++++++++++++++++++++-------------
 drivers/hv/connection.c   |    6 ++-
 drivers/hv/hv_balloon.c   |   15 ++---
 drivers/hv/hyperv_vmbus.h |    4 +-
 tools/hv/Makefile         |    2 +-
 6 files changed, 117 insertions(+), 57 deletions(-)

-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
  2015-03-07  7:04 [PATCH V2 0/7] Drivers: hv: Miscellaneous fixes K. Y. Srinivasan
@ 2015-03-07  7:04 ` K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 2/7] Drivers: hv: vmbus: Perform device register in the per-channel work element K. Y. Srinivasan
                     ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan

Export the vmbus_sendpacket_pagebuffer_ctl() interface.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index da53180..e58cdb7 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -710,6 +710,7 @@ int vmbus_sendpacket_pagebuffer_ctl(struct vmbus_channel *channel,
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer_ctl);
 
 /*
  * vmbus_sendpacket_pagebuffer - Send a range of single-page buffer
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 2/7] Drivers: hv: vmbus: Perform device register in the per-channel work element
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
@ 2015-03-07  7:04   ` K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 3/7] Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure K. Y. Srinivasan
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan

This patch is a continuation of the rescind handling cleanup work. We cannot
block in the global message handling work context especially if we are blocking
waiting for the host to wake us up. I would like to thank
Dexuan Cui <decui@microsoft.com> for observing this problem.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 - Free up the work element after processing rescind.
 drivers/hv/channel_mgmt.c |  143 +++++++++++++++++++++++++++++++-------------
 drivers/hv/connection.c   |    6 ++-
 drivers/hv/hyperv_vmbus.h |    2 +-
 3 files changed, 107 insertions(+), 44 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 6117891..5f8e47b 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -23,6 +23,7 @@
 #include <linux/kernel.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
+#include <linux/delay.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/list.h>
@@ -37,6 +38,10 @@ struct vmbus_channel_message_table_entry {
 	void (*message_handler)(struct vmbus_channel_message_header *msg);
 };
 
+struct vmbus_rescind_work {
+	struct work_struct work;
+	struct vmbus_channel *channel;
+};
 
 /**
  * vmbus_prep_negotiate_resp() - Create default response for Hyper-V Negotiate message
@@ -134,20 +139,6 @@ fw_error:
 
 EXPORT_SYMBOL_GPL(vmbus_prep_negotiate_resp);
 
-static void vmbus_process_device_unregister(struct work_struct *work)
-{
-	struct device *dev;
-	struct vmbus_channel *channel = container_of(work,
-							struct vmbus_channel,
-							work);
-
-	dev = get_device(&channel->device_obj->device);
-	if (dev) {
-		vmbus_device_unregister(channel->device_obj);
-		put_device(dev);
-	}
-}
-
 static void vmbus_sc_creation_cb(struct work_struct *work)
 {
 	struct vmbus_channel *newchannel = container_of(work,
@@ -220,6 +211,40 @@ static void free_channel(struct vmbus_channel *channel)
 	queue_work(vmbus_connection.work_queue, &channel->work);
 }
 
+static void process_rescind_fn(struct work_struct *work)
+{
+	struct vmbus_rescind_work *rc_work;
+	struct vmbus_channel *channel;
+	struct device *dev;
+
+	rc_work = container_of(work, struct vmbus_rescind_work, work);
+	channel = rc_work->channel;
+
+	/*
+	 * We have already acquired a reference on the channel
+	 * and so it cannot vanish underneath us.
+	 * It is possible (while very unlikely) that we may
+	 * get here while the processing of the initial offer
+	 * is still not complete. Deal with this situation by
+	 * just waiting until the channel is in the correct state.
+	 */
+
+	while (channel->work.func != release_channel)
+		msleep(1000);
+
+	if (channel->device_obj) {
+		dev = get_device(&channel->device_obj->device);
+		if (dev) {
+			vmbus_device_unregister(channel->device_obj);
+			put_device(dev);
+		}
+	} else {
+		hv_process_channel_removal(channel,
+					   channel->offermsg.child_relid);
+	}
+	kfree(work);
+}
+
 static void percpu_channel_enq(void *arg)
 {
 	struct vmbus_channel *channel = arg;
@@ -282,6 +307,46 @@ void vmbus_free_channels(void)
 	}
 }
 
+static void vmbus_do_device_register(struct work_struct *work)
+{
+	struct hv_device *device_obj;
+	int ret;
+	unsigned long flags;
+	struct vmbus_channel *newchannel = container_of(work,
+						     struct vmbus_channel,
+						     work);
+
+	ret = vmbus_device_register(newchannel->device_obj);
+	if (ret != 0) {
+		pr_err("unable to add child device object (relid %d)\n",
+			newchannel->offermsg.child_relid);
+		spin_lock_irqsave(&vmbus_connection.channel_lock, flags);
+		list_del(&newchannel->listentry);
+		device_obj = newchannel->device_obj;
+		newchannel->device_obj = NULL;
+		spin_unlock_irqrestore(&vmbus_connection.channel_lock, flags);
+
+		if (newchannel->target_cpu != get_cpu()) {
+			put_cpu();
+			smp_call_function_single(newchannel->target_cpu,
+					 percpu_channel_deq, newchannel, true);
+		} else {
+			percpu_channel_deq(newchannel);
+			put_cpu();
+		}
+
+		kfree(device_obj);
+		if (!newchannel->rescind) {
+			free_channel(newchannel);
+			return;
+		}
+	}
+	/*
+	 * The next state for this channel is to be freed.
+	 */
+	INIT_WORK(&newchannel->work, release_channel);
+}
+
 /*
  * vmbus_process_offer - Process the offer by creating a channel/device
  * associated with this offer
@@ -291,7 +356,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	struct vmbus_channel *channel;
 	bool fnew = true;
 	bool enq = false;
-	int ret;
 	unsigned long flags;
 
 	/* Make sure this is a new offer */
@@ -393,16 +457,13 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 	 * Add the new device to the bus. This will kick off device-driver
 	 * binding which eventually invokes the device driver's AddDevice()
 	 * method.
+	 * Invoke this call on the per-channel work context.
+	 * Until we return from this function, rescind offer message
+	 * cannot be processed as we are running on the global message
+	 * handling work.
 	 */
-	ret = vmbus_device_register(newchannel->device_obj);
-	if (ret != 0) {
-		pr_err("unable to add child device object (relid %d)\n",
-			   newchannel->offermsg.child_relid);
-
-		kfree(newchannel->device_obj);
-		goto err_deq_chan;
-	}
-
+	INIT_WORK(&newchannel->work, vmbus_do_device_register);
+	queue_work(newchannel->controlwq, &newchannel->work);
 	return;
 
 err_deq_chan:
@@ -556,33 +617,31 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr)
 {
 	struct vmbus_channel_rescind_offer *rescind;
 	struct vmbus_channel *channel;
-	unsigned long flags;
+	struct vmbus_rescind_work *rc_work;
 
 	rescind = (struct vmbus_channel_rescind_offer *)hdr;
-	channel = relid2channel(rescind->child_relid);
+	channel = relid2channel(rescind->child_relid, true);
 
 	if (channel == NULL) {
 		hv_process_channel_removal(NULL, rescind->child_relid);
 		return;
 	}
 
-	spin_lock_irqsave(&channel->lock, flags);
-	channel->rescind = true;
-	spin_unlock_irqrestore(&channel->lock, flags);
-
-	if (channel->device_obj) {
-		/*
-		 * We will have to unregister this device from the
-		 * driver core. Do this in the per-channel work context.
-		 * Note that we are currently executing on the global
-		 * workq for handling messages from the host.
-		 */
-		INIT_WORK(&channel->work, vmbus_process_device_unregister);
-		queue_work(channel->controlwq, &channel->work);
-	} else {
-		hv_process_channel_removal(channel,
-					   channel->offermsg.child_relid);
+	/*
+	 * We have acquired a reference on the channel and have posted
+	 * the rescind state. Perform further cleanup in a work context
+	 * that is different from the global work context in which
+	 * we process messages from the host (we are currently executing
+	 * on that global context.
+	 */
+	rc_work = kzalloc(sizeof(struct vmbus_rescind_work), GFP_KERNEL);
+	if (!rc_work) {
+		pr_err("Unable to allocate memory for rescind processing ");
+		return;
 	}
+	rc_work->channel = channel;
+	INIT_WORK(&rc_work->work, process_rescind_fn);
+	schedule_work(&rc_work->work);
 }
 
 /*
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 583d7d4..8bcd307 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -270,7 +270,7 @@ static struct vmbus_channel *pcpu_relid2channel(u32 relid)
  * relid2channel - Get the channel object given its
  * child relative id (ie channel id)
  */
-struct vmbus_channel *relid2channel(u32 relid)
+struct vmbus_channel *relid2channel(u32 relid, bool rescind)
 {
 	struct vmbus_channel *channel;
 	struct vmbus_channel *found_channel  = NULL;
@@ -282,6 +282,8 @@ struct vmbus_channel *relid2channel(u32 relid)
 	list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) {
 		if (channel->offermsg.child_relid == relid) {
 			found_channel = channel;
+			if (rescind)
+				found_channel->rescind = true;
 			break;
 		} else if (!list_empty(&channel->sc_list)) {
 			/*
@@ -292,6 +294,8 @@ struct vmbus_channel *relid2channel(u32 relid)
 							sc_list);
 				if (cur_sc->offermsg.child_relid == relid) {
 					found_channel = cur_sc;
+					if (rescind)
+						found_channel->rescind = true;
 					break;
 				}
 			}
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 88af4ec..6339589 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -698,7 +698,7 @@ void vmbus_device_unregister(struct hv_device *device_obj);
 /* VmbusChildDeviceDestroy( */
 /* struct hv_device *); */
 
-struct vmbus_channel *relid2channel(u32 relid);
+struct vmbus_channel *relid2channel(u32 relid, bool rescind);
 
 void vmbus_free_channels(void);
 
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 3/7] Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 2/7] Drivers: hv: vmbus: Perform device register in the per-channel work element K. Y. Srinivasan
@ 2015-03-07  7:04   ` K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 4/7] Drivers: hv: hv_balloon: don't lose memory when onlining order is not natural K. Y. Srinivasan
                     ` (4 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan

From: Vitaly Kuznetsov <vkuznets@redhat.com>

When add_memory() fails the following BUG is observed:
[  743.646107] hv_balloon: hot_add memory failed error is -17
[  743.679973]
[  743.680930] =====================================
[  743.680930] [ BUG: bad unlock balance detected! ]
[  743.680930] 3.19.0-rc5_bug1131426+ #552 Not tainted
[  743.680930] -------------------------------------
[  743.680930] kworker/0:2/255 is trying to release lock (&dm_device.ha_region_mutex) at:
[  743.680930] [<ffffffff81aae5fe>] mutex_unlock+0xe/0x10
[  743.680930] but there are no more locks to release!

This happens as we don't acquire ha_region_mutex and hot_add_req() expects us
to as it does unconditional mutex_unlock(). Acquire the lock on the error path.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_balloon.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index c5bb872..f1f17c5 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -652,6 +652,7 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 			}
 			has->ha_end_pfn -= HA_CHUNK;
 			has->covered_end_pfn -=  processed_pfn;
+			mutex_lock(&dm_device.ha_region_mutex);
 			break;
 		}
 
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 4/7] Drivers: hv: hv_balloon: don't lose memory when onlining order is not natural
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 2/7] Drivers: hv: vmbus: Perform device register in the per-channel work element K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 3/7] Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure K. Y. Srinivasan
@ 2015-03-07  7:04   ` K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 5/7] Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY K. Y. Srinivasan
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets; +Cc: K. Y. Srinivasan

From: Vitaly Kuznetsov <vkuznets@redhat.com>

Memory blocks can be onlined in random order. When this order is not natural
some memory pages are not onlined because of the redundant check in
hv_online_page().

Here is a real world scenario:
1) Host tries to hot-add the following (process_hot_add):
  pg_start=rg_start=0x48000, pfn_cnt=111616, rg_size=262144

2) This results in adding 4 memory blocks:
[  109.057866] init_memory_mapping: [mem 0x48000000-0x4fffffff]
[  114.102698] init_memory_mapping: [mem 0x50000000-0x57ffffff]
[  119.168039] init_memory_mapping: [mem 0x58000000-0x5fffffff]
[  124.233053] init_memory_mapping: [mem 0x60000000-0x67ffffff]
The last one is incomplete but we have special has->covered_end_pfn counter to
avoid onlining non-backed frames and hv_bring_pgs_online() function to bring
them online later on.

3) Now we have 4 offline memory blocks: /sys/devices/system/memory/memory9-12
$ for f in /sys/devices/system/memory/memory*/state; do echo $f `cat $f`; done | grep -v onlin
/sys/devices/system/memory/memory10/state offline
/sys/devices/system/memory/memory11/state offline
/sys/devices/system/memory/memory12/state offline
/sys/devices/system/memory/memory9/state offline

4) We bring them online in non-natural order:
$grep MemTotal /proc/meminfo
MemTotal:         966348 kB
$echo online > /sys/devices/system/memory/memory12/state && grep MemTotal /proc/meminfo
MemTotal:        1019596 kB
$echo online > /sys/devices/system/memory/memory11/state && grep MemTotal /proc/meminfo
MemTotal:        1150668 kB
$echo online > /sys/devices/system/memory/memory9/state && grep MemTotal /proc/meminfo
MemTotal:        1150668 kB

As you can see memory9 block gives us zero additional memory. We can also
observe a huge discrepancy between host- and guest-reported memory sizes.

The root cause of the issue is the redundant pg >= covered_start_pfn check (and
covered_start_pfn advancing) in hv_online_page(). When upper memory block in
being onlined before the lower one (memory12 and memory11 in the above case) we
advance the covered_start_pfn pointer and all memory9 pages do not pass the
check. If the assumption that host always gives us requests in sequential order
and pg_start always equals rg_start when the first request for the new HA
region is received (that's the case in my testing) is correct than we can get
rid of covered_start_pfn and pg >= start_pfn check in hv_online_page() is
sufficient.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hv_balloon.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index f1f17c5..014256a 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -428,14 +428,13 @@ struct dm_info_msg {
  * currently hot added. We hot add in multiples of 128M
  * chunks; it is possible that we may not be able to bring
  * online all the pages in the region. The range
- * covered_start_pfn : covered_end_pfn defines the pages that can
+ * covered_end_pfn defines the pages that can
  * be brough online.
  */
 
 struct hv_hotadd_state {
 	struct list_head list;
 	unsigned long start_pfn;
-	unsigned long covered_start_pfn;
 	unsigned long covered_end_pfn;
 	unsigned long ha_end_pfn;
 	unsigned long end_pfn;
@@ -679,8 +678,7 @@ static void hv_online_page(struct page *pg)
 
 	list_for_each(cur, &dm_device.ha_region_list) {
 		has = list_entry(cur, struct hv_hotadd_state, list);
-		cur_start_pgp = (unsigned long)
-				pfn_to_page(has->covered_start_pfn);
+		cur_start_pgp = (unsigned long)pfn_to_page(has->start_pfn);
 		cur_end_pgp = (unsigned long)pfn_to_page(has->covered_end_pfn);
 
 		if (((unsigned long)pg >= cur_start_pgp) &&
@@ -692,7 +690,6 @@ static void hv_online_page(struct page *pg)
 			__online_page_set_limits(pg);
 			__online_page_increment_counters(pg);
 			__online_page_free(pg);
-			has->covered_start_pfn++;
 		}
 	}
 }
@@ -736,10 +733,9 @@ static bool pfn_covered(unsigned long start_pfn, unsigned long pfn_cnt)
 		 * is, update it.
 		 */
 
-		if (has->covered_end_pfn != start_pfn) {
+		if (has->covered_end_pfn != start_pfn)
 			has->covered_end_pfn = start_pfn;
-			has->covered_start_pfn = start_pfn;
-		}
+
 		return true;
 
 	}
@@ -784,7 +780,6 @@ static unsigned long handle_pg_range(unsigned long pg_start,
 				pgs_ol = pfn_cnt;
 			hv_bring_pgs_online(start_pfn, pgs_ol);
 			has->covered_end_pfn +=  pgs_ol;
-			has->covered_start_pfn +=  pgs_ol;
 			pfn_cnt -= pgs_ol;
 		}
 
@@ -845,7 +840,6 @@ static unsigned long process_hot_add(unsigned long pg_start,
 		list_add_tail(&ha_region->list, &dm_device.ha_region_list);
 		ha_region->start_pfn = rg_start;
 		ha_region->ha_end_pfn = rg_start;
-		ha_region->covered_start_pfn = pg_start;
 		ha_region->covered_end_pfn = pg_start;
 		ha_region->end_pfn = rg_start + rg_size;
 	}
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 5/7] Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
                     ` (2 preceding siblings ...)
  2015-03-07  7:04   ` [PATCH V2 4/7] Drivers: hv: hv_balloon: don't lose memory when onlining order is not natural K. Y. Srinivasan
@ 2015-03-07  7:04   ` K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 6/7] hv: vmbus: missing curly braces in vmbus_process_offer() K. Y. Srinivasan
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets
  Cc: Nick Meier, K. Y. Srinivasan

From: Nick Meier <nmeier@microsoft.com>

HV_CRASH_CTL_CRASH_NOTIFY is a 64 bit number.  Depending on the usage context,
the value may be truncated. This patch is in response from the following
email from Intel:

    [char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9: sparse:
                   constant 0x8000000000000000 is so big it is unsigned long

    tree:   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-testing
    head:   b3de8e3719e582f3182bb504295e4a8e43c8c96f
    commit: 96c1d0581d00f7abe033350edb021a9d947d8d81 [25/45] Drivers: hv: vmbus: Add support for VMBus panic notifier handler
    reproduce:
      # apt-get install sparse
      git checkout 96c1d0581d00f7abe033350edb021a9d947d8d81
      make ARCH=x86_64 allmodconfig
      make C=1 CF=-D__CHECK_ENDIAN__

    sparse warnings: (new ones prefixed by >>)

    drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000000000000000 is so big it is unsigned long
    ...

Signed-off-by: Nick Meier <nmeier@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/hyperv_vmbus.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 6339589..c8e27e0 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -58,7 +58,7 @@ enum hv_cpuid_function {
 #define HV_X64_MSR_CRASH_P4   0x40000104
 #define HV_X64_MSR_CRASH_CTL  0x40000105
 
-#define HV_CRASH_CTL_CRASH_NOTIFY 0x8000000000000000
+#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
 
 /* Define version of the synthetic interrupt controller. */
 #define HV_SYNIC_VERSION		(1)
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 6/7] hv: vmbus: missing curly braces in vmbus_process_offer()
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
                     ` (3 preceding siblings ...)
  2015-03-07  7:04   ` [PATCH V2 5/7] Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY K. Y. Srinivasan
@ 2015-03-07  7:04   ` K. Y. Srinivasan
  2015-03-07  7:04   ` [PATCH V2 7/7] tools: hv: fcopy_daemon: support >2GB files for x86_32 guest K. Y. Srinivasan
  2015-03-10  7:42   ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Greg KH
  6 siblings, 0 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets
  Cc: Dan Carpenter, K. Y. Srinivasan

From: Dan Carpenter <dan.carpenter@oracle.com>

The indenting makes it clear that there were curly braces intended here.

Fixes: 2dd37cb81580 ('Drivers: hv: vmbus: Handle both rescind and offer messages in the same context')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/hv/channel_mgmt.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 5f8e47b..25dbbaf 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -415,7 +415,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 
 			newchannel->state = CHANNEL_OPEN_STATE;
 			channel->num_sc++;
-			if (channel->sc_creation_callback != NULL)
+			if (channel->sc_creation_callback != NULL) {
 				/*
 				 * We need to invoke the sub-channel creation
 				 * callback; invoke this in a seperate work
@@ -427,6 +427,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel)
 					  vmbus_sc_creation_cb);
 				queue_work(newchannel->controlwq,
 					   &newchannel->work);
+			}
 
 			return;
 		}
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V2 7/7] tools: hv: fcopy_daemon: support >2GB files for x86_32 guest
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
                     ` (4 preceding siblings ...)
  2015-03-07  7:04   ` [PATCH V2 6/7] hv: vmbus: missing curly braces in vmbus_process_offer() K. Y. Srinivasan
@ 2015-03-07  7:04   ` K. Y. Srinivasan
  2015-03-10  7:42   ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Greg KH
  6 siblings, 0 replies; 12+ messages in thread
From: K. Y. Srinivasan @ 2015-03-07  7:04 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, vkuznets
  Cc: Dexuan Cui, Alex Ng, K. Y. Srinivasan

From: Dexuan Cui <decui@microsoft.com>

Without this patch, hv_fcopy_daemon's hv_copy_data() -> pwrite()
will fail for >2GB file offset.

Signed-off-by: Alex Ng <alexng@microsoft.com>
Signed-off-by: Dexuan Cui <decui@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 tools/hv/Makefile |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/hv/Makefile b/tools/hv/Makefile
index 99ffe61..a8ab795 100644
--- a/tools/hv/Makefile
+++ b/tools/hv/Makefile
@@ -3,7 +3,7 @@
 CC = $(CROSS_COMPILE)gcc
 PTHREAD_LIBS = -lpthread
 WARNINGS = -Wall -Wextra
-CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS)
+CFLAGS = $(WARNINGS) -g $(PTHREAD_LIBS) $(shell getconf LFS_CFLAGS)
 
 all: hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon
 %: %.c
-- 
1.7.4.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
  2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
                     ` (5 preceding siblings ...)
  2015-03-07  7:04   ` [PATCH V2 7/7] tools: hv: fcopy_daemon: support >2GB files for x86_32 guest K. Y. Srinivasan
@ 2015-03-10  7:42   ` Greg KH
  2015-03-10 15:05     ` KY Srinivasan
  6 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2015-03-10  7:42 UTC (permalink / raw)
  To: K. Y. Srinivasan; +Cc: linux-kernel, devel, olaf, apw, vkuznets

On Fri, Mar 06, 2015 at 11:04:28PM -0800, K. Y. Srinivasan wrote:
> Export the vmbus_sendpacket_pagebuffer_ctl() interface.

Why?  Nothing in this patch needs it.  Please include this in the patch
that needs the symbol, or at least give a hint as to what is going on.

As it is, this just looks like a random export for no reason at all :(

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
  2015-03-10  7:42   ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Greg KH
@ 2015-03-10 15:05     ` KY Srinivasan
  2015-03-10 15:14       ` Greg KH
  0 siblings, 1 reply; 12+ messages in thread
From: KY Srinivasan @ 2015-03-10 15:05 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-kernel, devel, olaf, apw, vkuznets



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, March 10, 2015 12:42 AM
> To: KY Srinivasan
> Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com
> Subject: Re: [PATCH V2 1/7] Drivers: hv: vmbus: Export the
> vmbus_sendpacket_pagebuffer_ctl()
> 
> On Fri, Mar 06, 2015 at 11:04:28PM -0800, K. Y. Srinivasan wrote:
> > Export the vmbus_sendpacket_pagebuffer_ctl() interface.
> 
> Why?  Nothing in this patch needs it.  Please include this in the patch that
> needs the symbol, or at least give a hint as to what is going on.
> 
> As it is, this just looks like a random export for no reason at all :(

This will be used by Hyper-V networking driver to optimize signaling on
the send path. I wanted these patches committed before I sent the networking
patch to David Miller. I implemented the two vmbus APIs for sending messages:
1) vmbus_sendpacket_ctl()
2) vmbus_sendpacket_pagebuffer_ctl()

and I forgot to export the second form of the send API in an earlier patch-set that you have
already  committed. I need both to be exported to
for the netvsc patch I have.

On a different note, upstream Linux is currently broken on Hyper-V and a patch in this set
Fixes it:
0002-Drivers-hv-vmbus-Perform-device-register-in-the-per-.patch

Do you want me to resend this set?

Regards,

K. Y


> 
> greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
  2015-03-10 15:05     ` KY Srinivasan
@ 2015-03-10 15:14       ` Greg KH
  2015-03-10 18:20         ` KY Srinivasan
  0 siblings, 1 reply; 12+ messages in thread
From: Greg KH @ 2015-03-10 15:14 UTC (permalink / raw)
  To: KY Srinivasan; +Cc: apw, devel, olaf, linux-kernel

On Tue, Mar 10, 2015 at 03:05:26PM +0000, KY Srinivasan wrote:
> 
> 
> > -----Original Message-----
> > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > Sent: Tuesday, March 10, 2015 12:42 AM
> > To: KY Srinivasan
> > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com
> > Subject: Re: [PATCH V2 1/7] Drivers: hv: vmbus: Export the
> > vmbus_sendpacket_pagebuffer_ctl()
> > 
> > On Fri, Mar 06, 2015 at 11:04:28PM -0800, K. Y. Srinivasan wrote:
> > > Export the vmbus_sendpacket_pagebuffer_ctl() interface.
> > 
> > Why?  Nothing in this patch needs it.  Please include this in the patch that
> > needs the symbol, or at least give a hint as to what is going on.
> > 
> > As it is, this just looks like a random export for no reason at all :(
> 
> This will be used by Hyper-V networking driver to optimize signaling on
> the send path. I wanted these patches committed before I sent the networking
> patch to David Miller. I implemented the two vmbus APIs for sending messages:
> 1) vmbus_sendpacket_ctl()
> 2) vmbus_sendpacket_pagebuffer_ctl()
> 
> and I forgot to export the second form of the send API in an earlier patch-set that you have
> already  committed. I need both to be exported to
> for the netvsc patch I have.

Then just send it as part of the netvsc patches please.

> On a different note, upstream Linux is currently broken on Hyper-V and a patch in this set
> Fixes it:
> 0002-Drivers-hv-vmbus-Perform-device-register-in-the-per-.patch

That should have been pointed out somewhere :)

> Do you want me to resend this set?

Yes, please do.  Break it up into two different sets, one for 4.0-final,
and one for 4.1-rc1.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl()
  2015-03-10 15:14       ` Greg KH
@ 2015-03-10 18:20         ` KY Srinivasan
  0 siblings, 0 replies; 12+ messages in thread
From: KY Srinivasan @ 2015-03-10 18:20 UTC (permalink / raw)
  To: Greg KH; +Cc: apw, devel, olaf, linux-kernel



> -----Original Message-----
> From: Greg KH [mailto:gregkh@linuxfoundation.org]
> Sent: Tuesday, March 10, 2015 8:15 AM
> To: KY Srinivasan
> Cc: apw@canonical.com; devel@linuxdriverproject.org; olaf@aepfle.de;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH V2 1/7] Drivers: hv: vmbus: Export the
> vmbus_sendpacket_pagebuffer_ctl()
> 
> On Tue, Mar 10, 2015 at 03:05:26PM +0000, KY Srinivasan wrote:
> >
> >
> > > -----Original Message-----
> > > From: Greg KH [mailto:gregkh@linuxfoundation.org]
> > > Sent: Tuesday, March 10, 2015 12:42 AM
> > > To: KY Srinivasan
> > > Cc: linux-kernel@vger.kernel.org; devel@linuxdriverproject.org;
> > > olaf@aepfle.de; apw@canonical.com; vkuznets@redhat.com
> > > Subject: Re: [PATCH V2 1/7] Drivers: hv: vmbus: Export the
> > > vmbus_sendpacket_pagebuffer_ctl()
> > >
> > > On Fri, Mar 06, 2015 at 11:04:28PM -0800, K. Y. Srinivasan wrote:
> > > > Export the vmbus_sendpacket_pagebuffer_ctl() interface.
> > >
> > > Why?  Nothing in this patch needs it.  Please include this in the
> > > patch that needs the symbol, or at least give a hint as to what is going on.
> > >
> > > As it is, this just looks like a random export for no reason at all
> > > :(
> >
> > This will be used by Hyper-V networking driver to optimize signaling
> > on the send path. I wanted these patches committed before I sent the
> > networking patch to David Miller. I implemented the two vmbus APIs for
> sending messages:
> > 1) vmbus_sendpacket_ctl()
> > 2) vmbus_sendpacket_pagebuffer_ctl()
> >
> > and I forgot to export the second form of the send API in an earlier
> > patch-set that you have already  committed. I need both to be exported
> > to for the netvsc patch I have.
> 
> Then just send it as part of the netvsc patches please.

Done.
> 
> > On a different note, upstream Linux is currently broken on Hyper-V and
> > a patch in this set Fixes it:
> > 0002-Drivers-hv-vmbus-Perform-device-register-in-the-per-.patch
> 
> That should have been pointed out somewhere :)
> 
> > Do you want me to resend this set?
> 
> Yes, please do.  Break it up into two different sets, one for 4.0-final, and one
> for 4.1-rc1.

The upstream issue I noted is only on linux-next being queued up for 4.1-rc1. I will just resubmit
the patch-set for your  char-misc tree.

Thanks,

K. Y
> 
> thanks,
> 
> greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-03-10 18:20 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-07  7:04 [PATCH V2 0/7] Drivers: hv: Miscellaneous fixes K. Y. Srinivasan
2015-03-07  7:04 ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() K. Y. Srinivasan
2015-03-07  7:04   ` [PATCH V2 2/7] Drivers: hv: vmbus: Perform device register in the per-channel work element K. Y. Srinivasan
2015-03-07  7:04   ` [PATCH V2 3/7] Drivers: hv: hv_balloon: keep locks balanced on add_memory() failure K. Y. Srinivasan
2015-03-07  7:04   ` [PATCH V2 4/7] Drivers: hv: hv_balloon: don't lose memory when onlining order is not natural K. Y. Srinivasan
2015-03-07  7:04   ` [PATCH V2 5/7] Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY K. Y. Srinivasan
2015-03-07  7:04   ` [PATCH V2 6/7] hv: vmbus: missing curly braces in vmbus_process_offer() K. Y. Srinivasan
2015-03-07  7:04   ` [PATCH V2 7/7] tools: hv: fcopy_daemon: support >2GB files for x86_32 guest K. Y. Srinivasan
2015-03-10  7:42   ` [PATCH V2 1/7] Drivers: hv: vmbus: Export the vmbus_sendpacket_pagebuffer_ctl() Greg KH
2015-03-10 15:05     ` KY Srinivasan
2015-03-10 15:14       ` Greg KH
2015-03-10 18:20         ` KY Srinivasan

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).