[PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[KAMEZAWA Hiroyuki]

After Keith's report of memory hotadd failure, I increased test patterns.
These patches are a result of new patterns. But I cannot cover all existing
memory layout in the world, more tests are needed.
Now, I think my patch can make things better and want this codes to be tested
in -mm.patche series is consitsts of 5 patches.
 
Thanks,
-Kame

==
ioresouce handling code in memory hotplug allows not-aligned memory hot add.
But when memmap and other memory structures are initialized, parameters
should be aligned. (if not aligned, initialization of mem_map will do wrong,
it assumes parameters are aligned.) This patch fix it.

And this patch allows ioresource collision check to handle -EEXIST.

Signed-Off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>


 mm/memory_hotplug.c |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

Index: linux-2.6.18-rc3/mm/memory_hotplug.c
===================================================================
--- linux-2.6.18-rc3.orig/mm/memory_hotplug.c	2006-08-01 16:11:56.000000000 +0900
+++ linux-2.6.18-rc3/mm/memory_hotplug.c	2006-08-01 16:38:19.000000000 +0900
@@ -76,15 +76,22 @@
 {
 	unsigned long i;
 	int err = 0;
+	int start_sec, end_sec;
+	/* during initialize mem_map, align hot-added range to section */
+	start_sec = pfn_to_section_nr(phys_start_pfn);
+	end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
 
-	for (i = 0; i < nr_pages; i += PAGES_PER_SECTION) {
-		err = __add_section(zone, phys_start_pfn + i);
+	for (i = start_sec; i <= end_sec; i++) {
+		err = __add_section(zone, i << PFN_SECTION_SHIFT);
 
-		/* We want to keep adding the rest of the
-		 * sections if the first ones already exist
+		/*
+		 * EEXIST is finally dealed with by ioresource collision
+		 * check. see add_memory() => register_memory_resource()
+		 * Warning will be printed if there is collision.
 		 */
 		if (err && (err != -EEXIST))
 			break;
+		err = 0;
 	}
 
 	return err;
@@ -213,10 +220,10 @@
 }
 
 /* add this memory to iomem resource */
-static void register_memory_resource(u64 start, u64 size)
+static int register_memory_resource(u64 start, u64 size)
 {
 	struct resource *res;
-
+	int ret = 0;
 	res = kzalloc(sizeof(struct resource), GFP_KERNEL);
 	BUG_ON(!res);
 
@@ -228,7 +235,9 @@
 		printk("System RAM resource %llx - %llx cannot be added\n",
 		(unsigned long long)res->start, (unsigned long long)res->end);
 		kfree(res);
+		ret = -EEXIST;
 	}
+	return ret;
 }
 
 
@@ -269,7 +278,7 @@
 	}
 
 	/* register this memory as resource */
-	register_memory_resource(start, size);
+	ret = register_memory_resource(start, size);
 
 	return ret;
 error:

Re: [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[Andrew Morton]

On Thu, 3 Aug 2006 12:30:39 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> After Keith's report of memory hotadd failure, I increased test patterns.
> These patches are a result of new patterns. But I cannot cover all existing
> memory layout in the world, more tests are needed.
> Now, I think my patch can make things better and want this codes to be tested
> in -mm.patche series is consitsts of 5 patches.

I expect the code which these patches touch is completely untested in -mm, so
all we'll get is compile testing and some review.

Given that these patches touch pretty much nothing but the memory hot-add
paths I'd be inclined to fast-track them into 2.6.18.  Do you agree that
these patches are sufficiently safe and that the problems that they solve
are sufficiently serious for us to take that approach?

Either way, could I ask that interested parties review this work closely
and promptly?

Thanks.

Re: [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[KAMEZAWA Hiroyuki]

On Wed, 2 Aug 2006 23:38:02 -0700
Andrew Morton <akpm@osdl.org> wrote:

> On Thu, 3 Aug 2006 12:30:39 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > After Keith's report of memory hotadd failure, I increased test patterns.
> > These patches are a result of new patterns. But I cannot cover all existing
> > memory layout in the world, more tests are needed.
> > Now, I think my patch can make things better and want this codes to be tested
> > in -mm.patche series is consitsts of 5 patches.
> 
> I expect the code which these patches touch is completely untested in -mm, so
> all we'll get is compile testing and some review.
> 
yes.. just tested on my emulation box with some patterns, including patterns in
hot-add-failure report.(very small chunks in one section, and very big contiguous
memory hot add and unaligned memory hot-add.)

> Given that these patches touch pretty much nothing but the memory hot-add
> paths I'd be inclined to fast-track them into 2.6.18. 
> Do you agree that these patches are sufficiently safe and that the problems 
> that they solve are sufficiently serious for us to take that approach?
> 
I think this patch fixes serious problems. This patch can enlarge memory-hot-add
supported hardware. And fast-track paths sounds attractiveto me.
But I want more tests and reviews.
I posted this 3 weeks ago to -lhms but no positive/negative answers from the list.

Thanks,
-Kame

Re: [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[Yasunori Goto]

> > After Keith's report of memory hotadd failure, I increased test patterns.
> > These patches are a result of new patterns. But I cannot cover all existing
> > memory layout in the world, more tests are needed.
> > Now, I think my patch can make things better and want this codes to be tested
> > in -mm.patche series is consitsts of 5 patches.
> 
> I expect the code which these patches touch is completely untested in -mm, so
> all we'll get is compile testing and some review.
> 
> Given that these patches touch pretty much nothing but the memory hot-add
> paths I'd be inclined to fast-track them into 2.6.18.  Do you agree that
> these patches are sufficiently safe and that the problems that they solve
> are sufficiently serious for us to take that approach?
> 
> Either way, could I ask that interested parties review this work closely
> and promptly?

Hmm. I reviewed them a bit, and I couldn't find any problems.

However, my ia64 box is same of his. And emulation environment is very
close too. So, my perspective must be very similar from him.
I think my review is not enough. Keith-san's test is better if he can.

Anyway, I'll test them with -mm. Something different environment
may be good for test.

Thanks.


-- 
Yasunori Goto 

Re: [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[keith mannthey]

On Thu, 2006-08-03 at 16:47 +0900, Yasunori Goto wrote:
> > > After Keith's report of memory hotadd failure, I increased test patterns.
> > > These patches are a result of new patterns. But I cannot cover all existing
> > > memory layout in the world, more tests are needed.
> > > Now, I think my patch can make things better and want this codes to be tested
> > > in -mm.patche series is consitsts of 5 patches.
> > 
> > I expect the code which these patches touch is completely untested in -mm, so
> > all we'll get is compile testing and some review.
> > 
> > Given that these patches touch pretty much nothing but the memory hot-add
> > paths I'd be inclined to fast-track them into 2.6.18.  Do you agree that
> > these patches are sufficiently safe and that the problems that they solve
> > are sufficiently serious for us to take that approach?
> > 
> > Either way, could I ask that interested parties review this work closely
> > and promptly?
> 
> Hmm. I reviewed them a bit, and I couldn't find any problems.
> 
> However, my ia64 box is same of his. And emulation environment is very
> close too. So, my perspective must be very similar from him.
> I think my review is not enough. Keith-san's test is better if he can.
> 

I will test and review these patches today and will report back.  

Thanks,
  Keith

Re: [Lhms-devel] [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[Yasunori Goto]

> > > After Keith's report of memory hotadd failure, I increased test patterns.
> > > These patches are a result of new patterns. But I cannot cover all existing
> > > memory layout in the world, more tests are needed.
> > > Now, I think my patch can make things better and want this codes to be tested
> > > in -mm.patche series is consitsts of 5 patches.
> > 
> > I expect the code which these patches touch is completely untested in -mm, so
> > all we'll get is compile testing and some review.
> > 
> > Given that these patches touch pretty much nothing but the memory hot-add
> > paths I'd be inclined to fast-track them into 2.6.18.  Do you agree that
> > these patches are sufficiently safe and that the problems that they solve
> > are sufficiently serious for us to take that approach?
> > 
> > Either way, could I ask that interested parties review this work closely
> > and promptly?
> 
> Hmm. I reviewed them a bit, and I couldn't find any problems.
> 
> However, my ia64 box is same of his. And emulation environment is very
> close too. So, my perspective must be very similar from him.
> I think my review is not enough. Keith-san's test is better if he can.
> 
> Anyway, I'll test them with -mm. Something different environment
> may be good for test.

I tested them (includes 6/5) with -mm.
There is no regression on my emulation.

Acked-by: Yasunori Goto <y-goto@jp.fujitsu.com>


-- 
Yasunori Goto 

Re: [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[keith mannthey]

On Thu, 2006-08-03 at 12:30 +0900, KAMEZAWA Hiroyuki wrote:
> After Keith's report of memory hotadd failure, I increased test patterns.
> These patches are a result of new patterns. But I cannot cover all existing
> memory layout in the world, more tests are needed.
> Now, I think my patch can make things better and want this codes to be tested
> in -mm.patche series is consitsts of 5 patches.

I will test and review today.  
 
Thanks,
  Keith 

> ==
> ioresouce handling code in memory hotplug allows not-aligned memory hot add.
> But when memmap and other memory structures are initialized, parameters
> should be aligned. (if not aligned, initialization of mem_map will do wrong,
> it assumes parameters are aligned.) This patch fix it.
> 
> And this patch allows ioresource collision check to handle -EEXIST.
> 
> Signed-Off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> 
>  mm/memory_hotplug.c |   23 ++++++++++++++++-------
>  1 files changed, 16 insertions(+), 7 deletions(-)
> 
> Index: linux-2.6.18-rc3/mm/memory_hotplug.c
> ===================================================================
> --- linux-2.6.18-rc3.orig/mm/memory_hotplug.c	2006-08-01 16:11:56.000000000 +0900
> +++ linux-2.6.18-rc3/mm/memory_hotplug.c	2006-08-01 16:38:19.000000000 +0900
> @@ -76,15 +76,22 @@
>  {
>  	unsigned long i;
>  	int err = 0;
> +	int start_sec, end_sec;
> +	/* during initialize mem_map, align hot-added range to section */
> +	start_sec = pfn_to_section_nr(phys_start_pfn);
> +	end_sec = pfn_to_section_nr(phys_start_pfn + nr_pages - 1);
>  
> -	for (i = 0; i < nr_pages; i += PAGES_PER_SECTION) {
> -		err = __add_section(zone, phys_start_pfn + i);
> +	for (i = start_sec; i <= end_sec; i++) {
> +		err = __add_section(zone, i << PFN_SECTION_SHIFT);
>  
> -		/* We want to keep adding the rest of the
> -		 * sections if the first ones already exist
> +		/*
> +		 * EEXIST is finally dealed with by ioresource collision
> +		 * check. see add_memory() => register_memory_resource()
> +		 * Warning will be printed if there is collision.
>  		 */
>  		if (err && (err != -EEXIST))
>  			break;
> +		err = 0;
>  	}
>  
>  	return err;
> @@ -213,10 +220,10 @@
>  }
>  
>  /* add this memory to iomem resource */
> -static void register_memory_resource(u64 start, u64 size)
> +static int register_memory_resource(u64 start, u64 size)
>  {
>  	struct resource *res;
> -
> +	int ret = 0;
>  	res = kzalloc(sizeof(struct resource), GFP_KERNEL);
>  	BUG_ON(!res);
>  
> @@ -228,7 +235,9 @@
>  		printk("System RAM resource %llx - %llx cannot be added\n",
>  		(unsigned long long)res->start, (unsigned long long)res->end);
>  		kfree(res);
> +		ret = -EEXIST;
>  	}
> +	return ret;
>  }
>  
> 
> @@ -269,7 +278,7 @@
>  	}
>  
>  	/* register this memory as resource */
> -	register_memory_resource(start, size);
> +	ret = register_memory_resource(start, size);
>  
>  	return ret;
>  error:

Re: [PATCH] memory hotadd fixes [1/5] not-aligned memory hotadd handling fix

[keith mannthey]

> ioresouce handling code in memory hotplug allows not-aligned memory hot add.
> But when memmap and other memory structures are initialized, parameters
> should be aligned. (if not aligned, initialization of mem_map will do wrong,
> it assumes parameters are aligned.) This patch fix it.
> 
> And this patch allows ioresource collision check to handle -EEXIST.
>
> Signed-Off-By: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> 
> 
>  mm/memory_hotplug.c |   23 ++++++++++++++++-------
>  1 files changed, 16 insertions(+), 7 deletions(-)

This code looks and boots fine for me with x86_64.

 
Acked-by: Keith Mannthey <kmannth@us.ibm.com>