Re: [PATCH 2/2] Drivers: hv: balloon: Online the hot-added memory "in context"

Re: [PATCH 2/2] Drivers: hv: balloon: Online the hot-added memory "in context"

[Dave Hansen]

On 07/24/2013 02:29 PM, K. Y. Srinivasan wrote:
>  		/*
> -		 * Wait for the memory block to be onlined.
> -		 * Since the hot add has succeeded, it is ok to
> -		 * proceed even if the pages in the hot added region
> -		 * have not been "onlined" within the allowed time.
> +		 * Before proceeding to hot add the next segment,
> +		 * online the segment that has been hot added.
>  		 */
> -		wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
> +		online_memory_block(start_pfn);

Ahhhhh....  You've got a timeout in the code in order to tell the
hypervisor that you were successfully able to add the memory?  The
userspace addition code probably wasn't running within this timeout
period.  right?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

RE: [PATCH 2/2] Drivers: hv: balloon: Online the hot-added memory "in context"

[KY Srinivasan]

> -----Original Message-----
> From: Dave Hansen [mailto:dave@sr71.net]
> Sent: Wednesday, July 24, 2013 5:04 PM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> andi@firstfloor.org; akpm@linux-foundation.org; linux-mm@kvack.org;
> kamezawa.hiroyuki@gmail.com; mhocko@suse.cz; hannes@cmpxchg.org;
> yinghan@google.com
> Subject: Re: [PATCH 2/2] Drivers: hv: balloon: Online the hot-added memory "in
> context"
> 
> On 07/24/2013 02:29 PM, K. Y. Srinivasan wrote:
> >  		/*
> > -		 * Wait for the memory block to be onlined.
> > -		 * Since the hot add has succeeded, it is ok to
> > -		 * proceed even if the pages in the hot added region
> > -		 * have not been "onlined" within the allowed time.
> > +		 * Before proceeding to hot add the next segment,
> > +		 * online the segment that has been hot added.
> >  		 */
> > -		wait_for_completion_timeout(&dm_device.ol_waitevent,
> 5*HZ);
> > +		online_memory_block(start_pfn);
> 
> Ahhhhh....  You've got a timeout in the code in order to tell the
> hypervisor that you were successfully able to add the memory?  The
> userspace addition code probably wasn't running within this timeout
> period.  right?

As I have always said, the onlining would not occur within a specified amount
of time (under some conditions). The timeout here is to ensure that we are able 
to online the memory before attempting to hot-add more memory. With the ability
to online memory from within the kernel, we don't need this timeout and the code is
much more predictable.

Regards,

K. Y



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 0/2] Drivers: hv: balloon: Online memory segments "in context"

[K. Y. Srinivasan]

The current code depends on user level code to bring online memory
segments that have been hot added. Change this code to online memory
in the same context that is hot adding the memory.

This patch set implements the necessary infrastructure for making
it possible to online memory segments from within a driver.

K. Y. Srinivasan (2):
  Drivers: base: memory: Export functionality for "in kernel" onlining
    of memory
  Drivers: hv: balloon: Online the hot-added memory "in context"

 drivers/base/memory.c   |   35 +++++++++++++++++++++++++++++++++++
 drivers/hv/hv_balloon.c |   20 +++-----------------
 include/linux/memory.h  |    5 +++++
 3 files changed, 43 insertions(+), 17 deletions(-)

-- 
1.7.4.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] Drivers: base: memory: Export functionality for "in kernel" onlining of memory

[K. Y. Srinivasan]

The current machinery for hot-adding memory requires having user
level to bring the memory segments online. Export the necessary functionality
to bring the memory segment online without involving user space code.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
---
 drivers/base/memory.c  |   35 +++++++++++++++++++++++++++++++++++
 include/linux/memory.h |    5 +++++
 2 files changed, 40 insertions(+), 0 deletions(-)

diff --git a/drivers/base/memory.c b/drivers/base/memory.c
index 2b7813e..a7140b3 100644
--- a/drivers/base/memory.c
+++ b/drivers/base/memory.c
@@ -541,6 +541,41 @@ struct memory_block *find_memory_block(struct mem_section *section)
 	return find_memory_block_hinted(section, NULL);
 }
 
+/*
+ * Given the start pfn of a memory block; bring the memory
+ * block online. This API would be useful for drivers that may
+ * want to bring "online" the memory that has been hot-added.
+ */
+
+int online_memory_block(unsigned long start_pfn)
+{
+	struct mem_section *cur_section;
+	struct memory_block *cur_memory_block;
+	int ret_val = -EINVAL;
+
+	cur_section = __pfn_to_section(start_pfn);
+
+	if (!valid_section(cur_section))
+		return ret_val;
+
+	mutex_lock(&mem_sysfs_mutex);
+	cur_memory_block = find_memory_block(cur_section);
+
+	if (!cur_memory_block) {
+		mutex_unlock(&mem_sysfs_mutex);
+		return ret_val;
+	}
+
+	lock_device_hotplug();
+	ret_val = memory_block_change_state(cur_memory_block, MEM_ONLINE,
+						MEM_OFFLINE, ONLINE_KEEP);
+	unlock_device_hotplug();
+	mutex_unlock(&mem_sysfs_mutex);
+
+	return ret_val;
+}
+EXPORT_SYMBOL_GPL(online_memory_block);
+
 static struct attribute *memory_memblk_attrs[] = {
 	&dev_attr_phys_index.attr,
 	&dev_attr_end_phys_index.attr,
diff --git a/include/linux/memory.h b/include/linux/memory.h
index 85c31a8..7d0d2a6 100644
--- a/include/linux/memory.h
+++ b/include/linux/memory.h
@@ -109,12 +109,17 @@ static inline int memory_isolate_notify(unsigned long val, void *v)
 {
 	return 0;
 }
+static inline int online_memory_block(unsigned long start_pfn)
+{
+	return 0;
+}
 #else
 extern int register_memory_notifier(struct notifier_block *nb);
 extern void unregister_memory_notifier(struct notifier_block *nb);
 extern int register_memory_isolate_notifier(struct notifier_block *nb);
 extern void unregister_memory_isolate_notifier(struct notifier_block *nb);
 extern int register_new_memory(int, struct mem_section *);
+extern int online_memory_block(unsigned long start_pfn);
 #ifdef CONFIG_MEMORY_HOTREMOVE
 extern int unregister_memory_section(struct mem_section *);
 #endif
-- 
1.7.4.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] Drivers: hv: balloon: Online the hot-added memory "in context"

[K. Y. Srinivasan]

Leverage the newly exported functionality to bring memory online
without involving user level code.

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

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 2d094cf..c2eec17 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -515,11 +515,6 @@ struct hv_dynmem_device {
 	bool host_specified_ha_region;
 
 	/*
-	 * State to synchronize hot-add.
-	 */
-	struct completion  ol_waitevent;
-	bool ha_waiting;
-	/*
 	 * This thread handles hot-add
 	 * requests from the host as well as notifying
 	 * the host with regards to memory pressure in
@@ -581,9 +576,6 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 
 		has->covered_end_pfn +=  processed_pfn;
 
-		init_completion(&dm_device.ol_waitevent);
-		dm_device.ha_waiting = true;
-
 		nid = memory_add_physaddr_to_nid(PFN_PHYS(start_pfn));
 		ret = add_memory(nid, PFN_PHYS((start_pfn)),
 				(HA_CHUNK << PAGE_SHIFT));
@@ -606,12 +598,10 @@ static void hv_mem_hot_add(unsigned long start, unsigned long size,
 		}
 
 		/*
-		 * Wait for the memory block to be onlined.
-		 * Since the hot add has succeeded, it is ok to
-		 * proceed even if the pages in the hot added region
-		 * have not been "onlined" within the allowed time.
+		 * Before proceeding to hot add the next segment,
+		 * online the segment that has been hot added.
 		 */
-		wait_for_completion_timeout(&dm_device.ol_waitevent, 5*HZ);
+		online_memory_block(start_pfn);
 
 	}
 
@@ -625,10 +615,6 @@ static void hv_online_page(struct page *pg)
 	unsigned long cur_start_pgp;
 	unsigned long cur_end_pgp;
 
-	if (dm_device.ha_waiting) {
-		dm_device.ha_waiting = false;
-		complete(&dm_device.ol_waitevent);
-	}
 
 	list_for_each(cur, &dm_device.ha_region_list) {
 		has = list_entry(cur, struct hv_hotadd_state, list);
-- 
1.7.4.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>