[PATCH] catch valid mem range at onlining memory

[PATCH] catch valid mem range at onlining memory

[KAMEZAWA Hiroyuki]

This patch allows hot-add memory which is not aligned to section.
Based on linux-2.6.17-rc2-mm1 + memory hotadd ioresource register patch.

iomem resource patch is here.
http://www.uwsg.indiana.edu/hypermail/linux/kernel/0604.3/1188.html

Now, hot-added memory has to be aligned to section size.
Considering big section sized archs, this is not useful.

When hot-added memory is registerd as iomem resoruce by iomem resource patch,
we can make use of that information to detect valid memory range.

Note: With this, not-aligned memory can be registerd. To allow hot-add
      memory with holes, we have to do more work around add_memory().
      (It doesn't allows add memory to already existing mem section.)
      

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



Index: linux-2.6.17-rc2-mm1/kernel/resource.c
===================================================================
--- linux-2.6.17-rc2-mm1.orig/kernel/resource.c	2006-04-27 18:00:16.000000000 +0900
+++ linux-2.6.17-rc2-mm1/kernel/resource.c	2006-04-28 11:19:25.000000000 +0900
@@ -242,6 +242,45 @@
 
 EXPORT_SYMBOL(release_resource);
 
+#ifdef CONFIG_MEMORY_HOTPLUG
+/*
+ * Finds the lowest memory reosurce exists within [res->start.res->end)
+ * the caller must specify res->start, res->end, res->flags.
+ * If found, returns 0, res is overwritten, if not found, returns -1.
+ */
+int find_next_system_ram(struct resource *res)
+{
+	u64 start, end;
+	struct resource *p;
+
+	BUG_ON(!res);
+
+	start = res->start;
+	end = res->end;
+
+	read_lock(&resource_lock);
+	for( p = iomem_resource.child; p ; p = p->sibling) {
+		/* system ram is just marked as IORESOURCE_MEM */
+		if (p->flags != res->flags)
+			continue;
+		if (p->start > end) {
+			p = NULL;
+			break;
+		}
+		if (p->start >= start)
+			break;
+	}
+	read_unlock(&resource_lock);
+	if (!p)
+		return -1;
+	/* copy data */
+	res->start = p->start;
+	res->end = p->end;
+	return 0;
+}
+
+#endif
+
 /*
  * Find empty slot in the resource tree given range and alignment.
  */
Index: linux-2.6.17-rc2-mm1/include/linux/ioport.h
===================================================================
--- linux-2.6.17-rc2-mm1.orig/include/linux/ioport.h	2006-04-27 18:00:16.000000000 +0900
+++ linux-2.6.17-rc2-mm1/include/linux/ioport.h	2006-04-27 21:47:25.000000000 +0900
@@ -105,6 +105,10 @@
 			     void *alignf_data);
 int adjust_resource(struct resource *res, u64 start,
 		    u64 size);
+#ifdef CONFIG_MEMORY_HOTPLUG
+/* get registered SYSTEM_RAM resources in specified area */
+extern int find_next_system_ram(struct resource *res);
+#endif
 
 /* Convenience shorthand with allocation */
 #define request_region(start,n,name)	__request_region(&ioport_resource, (start), (n), (name))
Index: linux-2.6.17-rc2-mm1/mm/memory_hotplug.c
===================================================================
--- linux-2.6.17-rc2-mm1.orig/mm/memory_hotplug.c	2006-04-27 20:21:32.000000000 +0900
+++ linux-2.6.17-rc2-mm1/mm/memory_hotplug.c	2006-04-28 11:11:43.000000000 +0900
@@ -123,6 +123,9 @@
 	unsigned long i;
 	unsigned long flags;
 	unsigned long onlined_pages = 0;
+	struct resource res;
+	u64 section_end;
+	unsigned long start_pfn;
 	struct zone *zone;
 	int need_zonelists_rebuild = 0;
 
@@ -145,10 +148,27 @@
 	if (!populated_zone(zone))
 		need_zonelists_rebuild = 1;
 
-	for (i = 0; i < nr_pages; i++) {
-		struct page *page = pfn_to_page(pfn + i);
-		online_page(page);
-		onlined_pages++;
+	res.start = (u64)pfn << PAGE_SHIFT;
+	res.end = res.start + ((u64)nr_pages << PAGE_SHIFT) - 1;
+	res.flags = IORESOUECE_MEM; /* we just need system ram */
+	section_end = res.end;
+
+	while (find_next_system_ram(&res) >= 0) {
+		start_pfn = (unsigned long)(res.start >> PAGE_SHIFT);
+		nr_pages = (unsigned long)
+                           ((res.end + 1 - res.start) >> PAGE_SHIFT);
+
+		if (PageReserved(pfn_to_page(start_pfn))) {
+			/* this region's page is not onlined now */
+			for (i = 0; i < nr_pages; i++) {
+				struct page *page = pfn_to_page(start_pfn + i);
+				online_page(page);
+				onlined_pages++;
+			}
+		}
+
+		res.start = res.end + 1;
+		res.end = section_end;
 	}
 	zone->present_pages += onlined_pages;
 	zone->zone_pgdat->node_present_pages += onlined_pages;

Re: [PATCH] catch valid mem range at onlining memory

[Andrew Morton]

KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> This patch allows hot-add memory which is not aligned to section.
> Based on linux-2.6.17-rc2-mm1 + memory hotadd ioresource register patch.
> 
> iomem resource patch is here.
> http://www.uwsg.indiana.edu/hypermail/linux/kernel/0604.3/1188.html
> 
> Now, hot-added memory has to be aligned to section size.
> Considering big section sized archs, this is not useful.
> 
> When hot-added memory is registerd as iomem resoruce by iomem resource patch,
> we can make use of that information to detect valid memory range.
> 
> Note: With this, not-aligned memory can be registerd. To allow hot-add
>       memory with holes, we have to do more work around add_memory().
>       (It doesn't allows add memory to already existing mem section.)
>       
> 

Looks sane, thanks.

> +#ifdef CONFIG_MEMORY_HOTPLUG
> +/*
> + * Finds the lowest memory reosurce exists within [res->start.res->end)
> + * the caller must specify res->start, res->end, res->flags.
> + * If found, returns 0, res is overwritten, if not found, returns -1.
> + */
> +int find_next_system_ram(struct resource *res)
> +{
> +	u64 start, end;
> +	struct resource *p;
> +
> +	BUG_ON(!res);
> +
> +	start = res->start;
> +	end = res->end;
> +
> +	read_lock(&resource_lock);
> +	for( p = iomem_resource.child; p ; p = p->sibling) {
> +		/* system ram is just marked as IORESOURCE_MEM */
> +		if (p->flags != res->flags)
> +			continue;
> +		if (p->start > end) {
> +			p = NULL;
> +			break;
> +		}
> +		if (p->start >= start)
> +			break;
> +	}
> +	read_unlock(&resource_lock);
> +	if (!p)
> +		return -1;
> +	/* copy data */
> +	res->start = p->start;
> +	res->end = p->end;
> +	return 0;
> +}
> +
> +#endif

This all looks fairly (but trivially) dependent upon the 64-bit-resource
patches in Greg's tree.  Greg, were you planning on merging them in the
post-2.6.17 flood?

> ===================================================================
> --- linux-2.6.17-rc2-mm1.orig/include/linux/ioport.h	2006-04-27 18:00:16.000000000 +0900
> +++ linux-2.6.17-rc2-mm1/include/linux/ioport.h	2006-04-27 21:47:25.000000000 +0900
> @@ -105,6 +105,10 @@
>  			     void *alignf_data);
>  int adjust_resource(struct resource *res, u64 start,
>  		    u64 size);
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +/* get registered SYSTEM_RAM resources in specified area */
> +extern int find_next_system_ram(struct resource *res);
> +#endif

I'll remove the ifdefs - the only thing they give us if someone makes a
mistake is an additional compile-time warning, and I don't think that's
beneficial enough to warrant the addition of an ifdef.

Re: [PATCH] catch valid mem range at onlining memory

[Greg KH]

On Fri, Apr 28, 2006 at 04:34:09PM -0700, Andrew Morton wrote:
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> >
> > This patch allows hot-add memory which is not aligned to section.
> > Based on linux-2.6.17-rc2-mm1 + memory hotadd ioresource register patch.
> > 
> > iomem resource patch is here.
> > http://www.uwsg.indiana.edu/hypermail/linux/kernel/0604.3/1188.html
> > 
> > Now, hot-added memory has to be aligned to section size.
> > Considering big section sized archs, this is not useful.
> > 
> > When hot-added memory is registerd as iomem resoruce by iomem resource patch,
> > we can make use of that information to detect valid memory range.
> > 
> > Note: With this, not-aligned memory can be registerd. To allow hot-add
> >       memory with holes, we have to do more work around add_memory().
> >       (It doesn't allows add memory to already existing mem section.)
> >       
> > 
> 
> Looks sane, thanks.
> 
> > +#ifdef CONFIG_MEMORY_HOTPLUG
> > +/*
> > + * Finds the lowest memory reosurce exists within [res->start.res->end)
> > + * the caller must specify res->start, res->end, res->flags.
> > + * If found, returns 0, res is overwritten, if not found, returns -1.
> > + */
> > +int find_next_system_ram(struct resource *res)
> > +{
> > +	u64 start, end;
> > +	struct resource *p;
> > +
> > +	BUG_ON(!res);
> > +
> > +	start = res->start;
> > +	end = res->end;
> > +
> > +	read_lock(&resource_lock);
> > +	for( p = iomem_resource.child; p ; p = p->sibling) {
> > +		/* system ram is just marked as IORESOURCE_MEM */
> > +		if (p->flags != res->flags)
> > +			continue;
> > +		if (p->start > end) {
> > +			p = NULL;
> > +			break;
> > +		}
> > +		if (p->start >= start)
> > +			break;
> > +	}
> > +	read_unlock(&resource_lock);
> > +	if (!p)
> > +		return -1;
> > +	/* copy data */
> > +	res->start = p->start;
> > +	res->end = p->end;
> > +	return 0;
> > +}
> > +
> > +#endif
> 
> This all looks fairly (but trivially) dependent upon the 64-bit-resource
> patches in Greg's tree.  Greg, were you planning on merging them in the
> post-2.6.17 flood?

Yes, I was, unless there are any objections to me doing this?

thanks,

greg k-h

Re: [PATCH] catch valid mem range at onlining memory

[Andrew Morton]

Greg KH <greg@kroah.com> wrote:
>
> > This all looks fairly (but trivially) dependent upon the 64-bit-resource
> > patches in Greg's tree.  Greg, were you planning on merging them in the
> > post-2.6.17 flood?
> 
> Yes, I was,

OK, thanks.

> unless there are any objections to me doing this?

I'd consider the patches as they stand to be ready to roll.

All the code bloat's a bit sad though.  It would have been nice to have
made the type of resource.start and .end Kconfigurable.  What happened
to that?

Re: [PATCH] catch valid mem range at onlining memory

[Greg KH]

On Fri, Apr 28, 2006 at 05:05:19PM -0700, Andrew Morton wrote:
> Greg KH <greg@kroah.com> wrote:
> >
> > > This all looks fairly (but trivially) dependent upon the 64-bit-resource
> > > patches in Greg's tree.  Greg, were you planning on merging them in the
> > > post-2.6.17 flood?
> > 
> > Yes, I was,
> 
> OK, thanks.
> 
> > unless there are any objections to me doing this?
> 
> I'd consider the patches as they stand to be ready to roll.
> 
> All the code bloat's a bit sad though.  It would have been nice to have
> made the type of resource.start and .end Kconfigurable.  What happened
> to that?

Hm, I didn't remember anything about that.  Vivek, any thoughts?

thanks,

greg k-h

Re: [PATCH] catch valid mem range at onlining memory

[Vivek Goyal]

On Sat, Apr 29, 2006 at 12:18:18AM -0700, Greg KH wrote:
> On Fri, Apr 28, 2006 at 05:05:19PM -0700, Andrew Morton wrote:
> > Greg KH <greg@kroah.com> wrote:
> > >
> > > > This all looks fairly (but trivially) dependent upon the 64-bit-resource
> > > > patches in Greg's tree.  Greg, were you planning on merging them in the
> > > > post-2.6.17 flood?
> > > 
> > > Yes, I was,
> > 
> > OK, thanks.
> > 
> > > unless there are any objections to me doing this?
> > 
> > I'd consider the patches as they stand to be ready to roll.
> > 
> > All the code bloat's a bit sad though.  It would have been nice to have
> > made the type of resource.start and .end Kconfigurable.  What happened
> > to that?
> 
> Hm, I didn't remember anything about that.  Vivek, any thoughts?
>

Having resource size configurable is nice but it brings added complexity
with it. The question would be if code bloat is significant enough to 
go for other option. Last time I had posted few compilation results on
i386. I am summarizing these again.

allmodconfig (CONFIG_DEBUG_INFO=n)
-----------

vmlinux bloat:4096 bytes

allyesconfig  (CONFIG_DEBUG_INFO=n)
-----------
                                                                                
vmlinux size bloat: 52K

So even with allyesconfig total bloat is 52K and I am assuming the
systems where memory is at premium are going to use a very limited set
of modules and effectively will see much lesser code bloat than 52K.

For Kconfigurable resource size, probably dma_addr_t is not the very 
appropriate as at lots of places size also needs to be 64 bit and 
using dma_addr_t is not good. This will then boil down to introducing
a new type like dma_addr_t whose size is Kconfigurable.

Even if it is done, it will not get rid of code bloat completely as lots
of code bloat comes from printk() statemets where we explicitly typecast
the resource to (unsigned long long) to avoid compilation warnings across
the platforms. This typecasting will continue to be there even with
Kconfigurable resources.

Personally I would tend to think that we can live with this code bloat.

Thanks
Vivek

Re: [PATCH] catch valid mem range at onlining memory

[Andrew Morton]

Vivek Goyal <vgoyal@in.ibm.com> wrote:
>
> > > All the code bloat's a bit sad though.  It would have been nice to have
> > > made the type of resource.start and .end Kconfigurable.  What happened
> > > to that?
> > 
> > Hm, I didn't remember anything about that.  Vivek, any thoughts?
> >
> 
> Having resource size configurable is nice but it brings added complexity
> with it. The question would be if code bloat is significant enough to 
> go for other option. Last time I had posted few compilation results on
> i386. I am summarizing these again.
> 
> allmodconfig (CONFIG_DEBUG_INFO=n)
> -----------
> 
> vmlinux bloat:4096 bytes
> 
> allyesconfig  (CONFIG_DEBUG_INFO=n)
> -----------
>                                                                                 
> vmlinux size bloat: 52K
> 
> So even with allyesconfig total bloat is 52K and I am assuming the
> systems where memory is at premium are going to use a very limited set
> of modules and effectively will see much lesser code bloat than 52K.
> 
> For Kconfigurable resource size, probably dma_addr_t is not the very 
> appropriate as at lots of places size also needs to be 64 bit and 
> using dma_addr_t is not good. This will then boil down to introducing
> a new type like dma_addr_t whose size is Kconfigurable.

Yes, it would need to to be a new type - resource_addr_t, perhaps.

> Even if it is done, it will not get rid of code bloat completely as lots
> of code bloat comes from printk() statemets where we explicitly typecast
> the resource to (unsigned long long) to avoid compilation warnings across
> the platforms. This typecasting will continue to be there even with
> Kconfigurable resources.

True.

But you know, just because a printk is present doesn't actually mean that
it's useful.

IOW, the default should be to just delete the printks (or the
resource.start/end parts of them), and to grudgingly put them back if
someone screams loudly enough.  We have other ways of viewing the
iomem_resource and ioport_resource trees.

> Personally I would tend to think that we can live with this code bloat.

Every little bit counts ;) We tend to be a bit obsessive about this, but I
think it's good, although perhaps wasteful of developer time..

I wouldn't view any of this as blocking the present patches.  It's
a separable project.

Re: [PATCH] catch valid mem range at onlining memory

[Vivek Goyal]

On Sat, Apr 29, 2006 at 04:40:43PM -0700, Andrew Morton wrote:
> Vivek Goyal <vgoyal@in.ibm.com> wrote:
> >
> > > > All the code bloat's a bit sad though.  It would have been nice to have
> > > > made the type of resource.start and .end Kconfigurable.  What happened
> > > > to that?
> > > 
> > > Hm, I didn't remember anything about that.  Vivek, any thoughts?
> > >
> > 
> > Having resource size configurable is nice but it brings added complexity
> > with it. The question would be if code bloat is significant enough to 
> > go for other option. Last time I had posted few compilation results on
> > i386. I am summarizing these again.
> > 
> > allmodconfig (CONFIG_DEBUG_INFO=n)
> > -----------
> > 
> > vmlinux bloat:4096 bytes
> > 
> > allyesconfig  (CONFIG_DEBUG_INFO=n)
> > -----------
> >                                                                                 
> > vmlinux size bloat: 52K
> > 
> > So even with allyesconfig total bloat is 52K and I am assuming the
> > systems where memory is at premium are going to use a very limited set
> > of modules and effectively will see much lesser code bloat than 52K.
> > 
> > For Kconfigurable resource size, probably dma_addr_t is not the very 
> > appropriate as at lots of places size also needs to be 64 bit and 
> > using dma_addr_t is not good. This will then boil down to introducing
> > a new type like dma_addr_t whose size is Kconfigurable.
> 
> Yes, it would need to to be a new type - resource_addr_t, perhaps.
> 

How about "res_sz_t". "resource_addr_t" probably is not a very appropriate
keyword as at lots of places we also need to represent size and alignment
with this typedef.

Thanks
Vivek

Re: [PATCH] catch valid mem range at onlining memory

[Andrew Morton]

On Tue, 2 May 2006 16:42:16 -0400
Vivek Goyal <vgoyal@in.ibm.com> wrote:

> > Yes, it would need to to be a new type - resource_addr_t, perhaps.
> > 
> 
> How about "res_sz_t".

resource_size_t, please.

> "resource_addr_t" probably is not a very appropriate
> keyword as at lots of places we also need to represent size and alignment
> with this typedef.

OK.

Re: [PATCH] catch valid mem range at onlining memory

[KAMEZAWA Hiroyuki]

On Fri, 28 Apr 2006 16:34:09 -0700
Andrew Morton <akpm@osdl.org> wrote:

> 
> This all looks fairly (but trivially) dependent upon the 64-bit-resource
> patches in Greg's tree.  
yes.

> Greg, were you planning on merging them in the post-2.6.17 flood?
> 

For kdump, I'm glad if it can be mergerd.

-Kame