PS3: Fix memory hotplug

Started by Geoff Levand on May 13, 2008 · 13 messages from 4 authors · Last reply May 21, 2008

open the first message on its own page

  1. [patch v2] PS3: Fix memory hotplug
    Related patches
  2. [rfc] [patch] LMB: Add basic spin locking to lmb
    Related patches
  3. [patch v2] LMB: Add basic spin locking to lmb
    Related patches

Geoff Levand

Message details and related content
Open message details and related content
A change was made to walk_memory_resource() in commit
4b119e21d0c66c22e8ca03df05d9de623d0eb50f that added a
check of find_lmb().  Add the corresponding lmb_add()
call to ps3_mm_add_memory() so that that check will
succeed.

This fixes the condition where the PS3 boots up with
just the 128 MiB of boot memory.

Signed-off-by: Geoff Levand <redacted>
---

Paul,

Please send this one upstream at your earliest convenience.

-Geoff

 arch/powerpc/platforms/ps3/mm.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -317,6 +317,8 @@ static int __init ps3_mm_add_memory(void
 		return result;
 	}
 
+	lmb_add(start_addr, map.r1.size);
+
 	result = online_pages(start_pfn, nr_pages);
 
 	if (result)

Benjamin Herrenschmidt replied to Geoff Levand

Message details and related content
Open message details and related content
On Tue, 2008-05-13 at 09:23 -0700, Geoff Levand wrote:
A change was made to walk_memory_resource() in commit
4b119e21d0c66c22e8ca03df05d9de623d0eb50f that added a
check of find_lmb().  Add the corresponding lmb_add()
call to ps3_mm_add_memory() so that that check will
succeed.

This fixes the condition where the PS3 boots up with
just the 128 MiB of boot memory.

Signed-off-by: Geoff Levand <redacted>
---
When you do an lmb_add you should probably also do an lmb_analyze to
update the total memory count etc...

That leads to some interesting issues such as the LMB stuff wasn't
really meant to be dynamically modified after boot, and thus the kernel
has no locks in there. That can be an issue...

Paul, any thoughts here ? Should we add a lock ? That would mean being
careful as the LMB stuff can be called very early, and spinlock wants
things like PACA and possibly lockdep to be around.. 

Ben.
quoted hunk
Paul,

Please send this one upstream at your earliest convenience.

-Geoff

 arch/powerpc/platforms/ps3/mm.c |    2 ++
 1 file changed, 2 insertions(+)

--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -317,6 +317,8 @@ static int __init ps3_mm_add_memory(void
 		return result;
 	}
 
+	lmb_add(start_addr, map.r1.size);
+
 	result = online_pages(start_pfn, nr_pages);
 
 	if (result)

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Benjamin Herrenschmidt replied to Benjamin Herrenschmidt

Message details and related content
Open message details and related content
Paul, any thoughts here ? Should we add a lock ? That would mean being
careful as the LMB stuff can be called very early, and spinlock wants
things like PACA and possibly lockdep to be around.. 
Actually, we call early_init_devtree(), which populates the LMB, after
we initialize the PACA and lockdep, so it -should- be safe to add a
spinlock.

Dave ? Any objection to adding a spinlock to the LMB code ?

Cheers,
Ben.





  

    

      
      David Miller      replied to Benjamin Herrenschmidt      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Wed, 14 May 2008 23:51:29 -0700

quoted
Paul, any thoughts here ? Should we add a lock ? That would mean being
careful as the LMB stuff can be called very early, and spinlock wants
things like PACA and possibly lockdep to be around.. 
Actually, we call early_init_devtree(), which populates the LMB, after
we initialize the PACA and lockdep, so it -should- be safe to add a
spinlock.

Dave ? Any objection to adding a spinlock to the LMB code ?
No objections, just don't add any bugs. :-)





  

    

      
      Paul Mackerras      replied to Benjamin Herrenschmidt      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
Benjamin Herrenschmidt writes:

When you do an lmb_add you should probably also do an lmb_analyze to
update the total memory count etc...

That leads to some interesting issues such as the LMB stuff wasn't
really meant to be dynamically modified after boot, and thus the kernel
has no locks in there. That can be an issue...

Paul, any thoughts here ? Should we add a lock ? That would mean being
careful as the LMB stuff can be called very early, and spinlock wants
things like PACA and possibly lockdep to be around.. 
Either that, or we give in and use iomem_resource to track where
system RAM is, as well as the other things in the physical address
space, like other architectures do...

Paul.





  


[patch v2] PS3: Fix memory hotplug
    

      
      Geoff Levand      replied to Geoff Levand      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
A change was made to walk_memory_resource() in commit
4b119e21d0c66c22e8ca03df05d9de623d0eb50f that added a
check of find_lmb().  Add the coresponding lmb_add()
call to ps3_mm_add_memory() so that that check will
succeed.

This fixes the condition where the PS3 boots up with
only the 128 MiB of boot memory.

Signed-off-by: Geoff Levand <redacted>
---

v2: Add call to lmb_analyze().

 arch/powerpc/platforms/ps3/mm.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -317,6 +317,9 @@ static int __init ps3_mm_add_memory(void
 		return result;
 	}
 
+	lmb_add(start_addr, map.r1.size);
+	lmb_analyze();
+
 	result = online_pages(start_pfn, nr_pages);
 
 	if (result)





  


[rfc] [patch] LMB: Add basic spin locking to lmb
    

      
      Geoff Levand      replied to David Miller      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
Add a spinlock to struct lmb to enforce concurrency in
lmb_add(), lmb_remove(), lmb_analyze(), and lmb_dump_all().

This locking is needed for SMP systems that access the lmb structure
during hot memory add and remove operations after secondary cpus
have been started.

Signed-off-by: Geoff Levand <redacted>
---

This patch just adds locks for the few lmb routines that would
be used for hot memory adding and removing.

-Geoff


 include/linux/lmb.h |    1 
 lib/lmb.c           |   54 +++++++++++++++++++++++++++++++++++++++-------------
 2 files changed, 42 insertions(+), 13 deletions(-)

--- a/include/linux/lmb.h
+++ b/include/linux/lmb.h
@@ -30,6 +30,7 @@ struct lmb_region {
 };
 
 struct lmb {
+	spinlock_t lock;
 	unsigned long debug;
 	u64 rmo_size;
 	struct lmb_region memory;
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -32,28 +32,33 @@ early_param("lmb", early_lmb);
 void lmb_dump_all(void)
 {
 	unsigned long i;
+	struct lmb tmp;
 
 	if (!lmb_debug)
 		return;
 
+	spin_lock(&lmb.lock);
+	tmp = lmb;
+	spin_unlock(&lmb.lock);
+
 	pr_info("lmb_dump_all:\n");
-	pr_info("    memory.cnt		  = 0x%lx\n", lmb.memory.cnt);
+	pr_info("    memory.cnt		  = 0x%lx\n", tmp.memory.cnt);
 	pr_info("    memory.size		  = 0x%llx\n",
-	    (unsigned long long)lmb.memory.size);
-	for (i=0; i < lmb.memory.cnt ;i++) {
+	    (unsigned long long)tmp.memory.size);
+	for (i=0; i < tmp.memory.cnt ;i++) {
 		pr_info("    memory.region[0x%lx].base       = 0x%llx\n",
-		    i, (unsigned long long)lmb.memory.region[i].base);
+		    i, (unsigned long long)tmp.memory.region[i].base);
 		pr_info("		      .size     = 0x%llx\n",
-		    (unsigned long long)lmb.memory.region[i].size);
+		    (unsigned long long)tmp.memory.region[i].size);
 	}
 
-	pr_info("    reserved.cnt	  = 0x%lx\n", lmb.reserved.cnt);
-	pr_info("    reserved.size	  = 0x%lx\n", lmb.reserved.size);
-	for (i=0; i < lmb.reserved.cnt ;i++) {
+	pr_info("    reserved.cnt	  = 0x%lx\n", tmp.reserved.cnt);
+	pr_info("    reserved.size	  = 0x%lx\n", tmp.reserved.size);
+	for (i=0; i < tmp.reserved.cnt ;i++) {
 		pr_info("    reserved.region[0x%lx].base       = 0x%llx\n",
-		    i, (unsigned long long)lmb.reserved.region[i].base);
+		    i, (unsigned long long)tmp.reserved.region[i].base);
 		pr_info("		      .size     = 0x%llx\n",
-		    (unsigned long long)lmb.reserved.region[i].size);
+		    (unsigned long long)tmp.reserved.region[i].size);
 	}
 }
 
@@ -105,6 +110,8 @@ static void lmb_coalesce_regions(struct 
 
 void __init lmb_init(void)
 {
+	spin_lock_init(&lmb.lock);
+
 	/* Create a dummy zero size LMB which will get coalesced away later.
 	 * This simplifies the lmb_add() code below...
 	 */
@@ -122,10 +129,14 @@ void __init lmb_analyze(void)
 {
 	int i;
 
+	spin_lock(&lmb.lock);
+
 	lmb.memory.size = 0;
 
 	for (i = 0; i < lmb.memory.cnt; i++)
 		lmb.memory.size += lmb.memory.region[i].size;
+
+	spin_unlock(&lmb.lock);
 }
 
 static long lmb_add_region(struct lmb_region *rgn, u64 base, u64 size)
@@ -194,18 +205,25 @@ static long lmb_add_region(struct lmb_re
 
 long lmb_add(u64 base, u64 size)
 {
+	long ret;
 	struct lmb_region *_rgn = &lmb.memory;
 
+	spin_lock(&lmb.lock);
+
 	/* On pSeries LPAR systems, the first LMB is our RMO region. */
 	if (base == 0)
 		lmb.rmo_size = size;
 
-	return lmb_add_region(_rgn, base, size);
+	ret = lmb_add_region(_rgn, base, size);
+
+	spin_unlock(&lmb.lock);
+	return ret;
 
 }
 
 long lmb_remove(u64 base, u64 size)
 {
+	long ret;
 	struct lmb_region *rgn = &(lmb.memory);
 	u64 rgnbegin, rgnend;
 	u64 end = base + size;
@@ -213,6 +231,8 @@ long lmb_remove(u64 base, u64 size)
 
 	rgnbegin = rgnend = 0; /* supress gcc warnings */
 
+	spin_lock(&lmb.lock);
+
 	/* Find the region where (base, size) belongs to */
 	for (i=0; i < rgn->cnt; i++) {
 		rgnbegin = rgn->region[i].base;
@@ -223,12 +243,15 @@ long lmb_remove(u64 base, u64 size)
 	}
 
 	/* Didn't find the region */
-	if (i == rgn->cnt)
+	if (i == rgn->cnt) {
+		spin_unlock(&lmb.lock);
 		return -1;
+	}
 
 	/* Check to see if we are removing entire region */
 	if ((rgnbegin == base) && (rgnend == end)) {
 		lmb_remove_region(rgn, i);
+		spin_unlock(&lmb.lock);
 		return 0;
 	}
 
@@ -236,12 +259,14 @@ long lmb_remove(u64 base, u64 size)
 	if (rgnbegin == base) {
 		rgn->region[i].base = end;
 		rgn->region[i].size -= size;
+		spin_unlock(&lmb.lock);
 		return 0;
 	}
 
 	/* Check to see if the region is matching at the end */
 	if (rgnend == end) {
 		rgn->region[i].size -= size;
+		spin_unlock(&lmb.lock);
 		return 0;
 	}
 
@@ -250,7 +275,10 @@ long lmb_remove(u64 base, u64 size)
 	 * beginging of the hole and add the region after hole.
 	 */
 	rgn->region[i].size = base - rgn->region[i].base;
-	return lmb_add_region(rgn, end, rgnend - end);
+	ret = lmb_add_region(rgn, end, rgnend - end);
+
+	spin_unlock(&lmb.lock);
+	return ret;
 }
 
 long __init lmb_reserve(u64 base, u64 size)





  


[patch v2] LMB: Add basic spin locking to lmb
    

      
      Geoff Levand      replied to Geoff Levand      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
Add a spinlock to struct lmb to enforce concurrency in
lmb_add(), lmb_remove(), lmb_analyze(), lmb_find(), and
lmb_dump_all().

This locking is needed for SMP systems that access the lmb structure
during hot memory add and remove operations after secondary cpus
have been started.

Signed-off-by: Geoff Levand <redacted>
---

v2: o Add locking to lmb_find().

 include/linux/lmb.h |    1 
 lib/lmb.c           |   62 ++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 49 insertions(+), 14 deletions(-)

--- a/include/linux/lmb.h
+++ b/include/linux/lmb.h
@@ -30,6 +30,7 @@ struct lmb_region {
 };
 
 struct lmb {
+	spinlock_t lock;
 	unsigned long debug;
 	u64 rmo_size;
 	struct lmb_region memory;
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -32,28 +32,33 @@ early_param("lmb", early_lmb);
 void lmb_dump_all(void)
 {
 	unsigned long i;
+	struct lmb tmp;
 
 	if (!lmb_debug)
 		return;
 
+	spin_lock(&lmb.lock);
+	tmp = lmb;
+	spin_unlock(&lmb.lock);
+
 	pr_info("lmb_dump_all:\n");
-	pr_info("    memory.cnt		  = 0x%lx\n", lmb.memory.cnt);
+	pr_info("    memory.cnt		  = 0x%lx\n", tmp.memory.cnt);
 	pr_info("    memory.size		  = 0x%llx\n",
-	    (unsigned long long)lmb.memory.size);
-	for (i=0; i < lmb.memory.cnt ;i++) {
+	    (unsigned long long)tmp.memory.size);
+	for (i=0; i < tmp.memory.cnt ;i++) {
 		pr_info("    memory.region[0x%lx].base       = 0x%llx\n",
-		    i, (unsigned long long)lmb.memory.region[i].base);
+		    i, (unsigned long long)tmp.memory.region[i].base);
 		pr_info("		      .size     = 0x%llx\n",
-		    (unsigned long long)lmb.memory.region[i].size);
+		    (unsigned long long)tmp.memory.region[i].size);
 	}
 
-	pr_info("    reserved.cnt	  = 0x%lx\n", lmb.reserved.cnt);
-	pr_info("    reserved.size	  = 0x%lx\n", lmb.reserved.size);
-	for (i=0; i < lmb.reserved.cnt ;i++) {
+	pr_info("    reserved.cnt	  = 0x%lx\n", tmp.reserved.cnt);
+	pr_info("    reserved.size	  = 0x%lx\n", tmp.reserved.size);
+	for (i=0; i < tmp.reserved.cnt ;i++) {
 		pr_info("    reserved.region[0x%lx].base       = 0x%llx\n",
-		    i, (unsigned long long)lmb.reserved.region[i].base);
+		    i, (unsigned long long)tmp.reserved.region[i].base);
 		pr_info("		      .size     = 0x%llx\n",
-		    (unsigned long long)lmb.reserved.region[i].size);
+		    (unsigned long long)tmp.reserved.region[i].size);
 	}
 }
 
@@ -105,6 +110,8 @@ static void lmb_coalesce_regions(struct 
 
 void __init lmb_init(void)
 {
+	spin_lock_init(&lmb.lock);
+
 	/* Create a dummy zero size LMB which will get coalesced away later.
 	 * This simplifies the lmb_add() code below...
 	 */
@@ -122,10 +129,14 @@ void __init lmb_analyze(void)
 {
 	int i;
 
+	spin_lock(&lmb.lock);
+
 	lmb.memory.size = 0;
 
 	for (i = 0; i < lmb.memory.cnt; i++)
 		lmb.memory.size += lmb.memory.region[i].size;
+
+	spin_unlock(&lmb.lock);
 }
 
 static long lmb_add_region(struct lmb_region *rgn, u64 base, u64 size)
@@ -194,18 +205,25 @@ static long lmb_add_region(struct lmb_re
 
 long lmb_add(u64 base, u64 size)
 {
+	long ret;
 	struct lmb_region *_rgn = &lmb.memory;
 
+	spin_lock(&lmb.lock);
+
 	/* On pSeries LPAR systems, the first LMB is our RMO region. */
 	if (base == 0)
 		lmb.rmo_size = size;
 
-	return lmb_add_region(_rgn, base, size);
+	ret = lmb_add_region(_rgn, base, size);
+
+	spin_unlock(&lmb.lock);
+	return ret;
 
 }
 
 long lmb_remove(u64 base, u64 size)
 {
+	long ret;
 	struct lmb_region *rgn = &(lmb.memory);
 	u64 rgnbegin, rgnend;
 	u64 end = base + size;
@@ -213,6 +231,8 @@ long lmb_remove(u64 base, u64 size)
 
 	rgnbegin = rgnend = 0; /* supress gcc warnings */
 
+	spin_lock(&lmb.lock);
+
 	/* Find the region where (base, size) belongs to */
 	for (i=0; i < rgn->cnt; i++) {
 		rgnbegin = rgn->region[i].base;
@@ -223,12 +243,15 @@ long lmb_remove(u64 base, u64 size)
 	}
 
 	/* Didn't find the region */
-	if (i == rgn->cnt)
+	if (i == rgn->cnt) {
+		spin_unlock(&lmb.lock);
 		return -1;
+	}
 
 	/* Check to see if we are removing entire region */
 	if ((rgnbegin == base) && (rgnend == end)) {
 		lmb_remove_region(rgn, i);
+		spin_unlock(&lmb.lock);
 		return 0;
 	}
 
@@ -236,12 +259,14 @@ long lmb_remove(u64 base, u64 size)
 	if (rgnbegin == base) {
 		rgn->region[i].base = end;
 		rgn->region[i].size -= size;
+		spin_unlock(&lmb.lock);
 		return 0;
 	}
 
 	/* Check to see if the region is matching at the end */
 	if (rgnend == end) {
 		rgn->region[i].size -= size;
+		spin_unlock(&lmb.lock);
 		return 0;
 	}
 
@@ -250,7 +275,10 @@ long lmb_remove(u64 base, u64 size)
 	 * beginging of the hole and add the region after hole.
 	 */
 	rgn->region[i].size = base - rgn->region[i].base;
-	return lmb_add_region(rgn, end, rgnend - end);
+	ret = lmb_add_region(rgn, end, rgnend - end);
+
+	spin_unlock(&lmb.lock);
+	return ret;
 }
 
 long __init lmb_reserve(u64 base, u64 size)
@@ -502,12 +530,16 @@ int lmb_find(struct lmb_property *res)
 	rstart = res->base;
 	rend = rstart + res->size - 1;
 
+	spin_lock(&lmb.lock);
+
 	for (i = 0; i < lmb.memory.cnt; i++) {
 		u64 start = lmb.memory.region[i].base;
 		u64 end = start + lmb.memory.region[i].size - 1;
 
-		if (start > rend)
+		if (start > rend) {
+			spin_unlock(&lmb.lock);
 			return -1;
+		}
 
 		if ((end >= rstart) && (start < rend)) {
 			/* adjust the request */
@@ -517,8 +549,10 @@ int lmb_find(struct lmb_property *res)
 				rend = end;
 			res->base = rstart;
 			res->size = rend - rstart + 1;
+			spin_unlock(&lmb.lock);
 			return 0;
 		}
 	}
+	spin_unlock(&lmb.lock);
 	return -1;
 }





  


Re: [patch v2] LMB: Add basic spin locking to lmb
    

      
      David Miller      replied to Geoff Levand      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
From: Geoff Levand <redacted>
Date: Mon, 19 May 2008 17:55:45 -0700

Add a spinlock to struct lmb to enforce concurrency in
lmb_add(), lmb_remove(), lmb_analyze(), lmb_find(), and
lmb_dump_all().

This locking is needed for SMP systems that access the lmb structure
during hot memory add and remove operations after secondary cpus
have been started.

Signed-off-by: Geoff Levand <redacted>
---

v2: o Add locking to lmb_find().
I'm not against this patch, but I'm pretty sure it's not
necessary.  Isn't memory hotplug already synchronized at
a higher level?

If not, it should be.





  


Re: [patch v2] LMB: Add basic spin locking to lmb
    

      
      Benjamin Herrenschmidt      replied to David Miller      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
On Mon, 2008-05-19 at 19:22 -0700, David Miller wrote:
From: Geoff Levand <redacted>
Date: Mon, 19 May 2008 17:55:45 -0700

quoted
Add a spinlock to struct lmb to enforce concurrency in
lmb_add(), lmb_remove(), lmb_analyze(), lmb_find(), and
lmb_dump_all().

This locking is needed for SMP systems that access the lmb structure
during hot memory add and remove operations after secondary cpus
have been started.

Signed-off-by: Geoff Levand <redacted>
---

v2: o Add locking to lmb_find().
I'm not against this patch, but I'm pretty sure it's not
necessary.  Isn't memory hotplug already synchronized at
a higher level?

If not, it should be.
I think the core memory hotplug is... However, we used to not change the
LMB when doing so (afaik, I'm travelling and not looking at the code
right now). However, things like PS3 memory hotplug tries to keep LMB is
sync for the sake of /dev/mem or similar and that happens before the
memory is added to the core.

Ben.





  


Re: [patch v2] LMB: Add basic spin locking to lmb
    

      
      David Miller      replied to Benjamin Herrenschmidt      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Date: Mon, 19 May 2008 22:32:58 -0400

I think the core memory hotplug is... However, we used to not change the
LMB when doing so (afaik, I'm travelling and not looking at the code
right now). However, things like PS3 memory hotplug tries to keep LMB is
sync for the sake of /dev/mem or similar and that happens before the
memory is added to the core.
But if the memory hotplug is synchronized, so are changes to the LMB
tables.

And if there are LMB read side access concernes outside of the hotplug
event, ideally we should use the synchronization mechanism that
hotplug uses instead of adding a new one.





  


Re: [patch v2] PS3: Fix memory hotplug
    

      
      Geoff Levand      replied to Geoff Levand      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
Geoff Levand wrote:
quoted hunk
A change was made to walk_memory_resource() in commit
4b119e21d0c66c22e8ca03df05d9de623d0eb50f that added a
check of find_lmb().  Add the coresponding lmb_add()
call to ps3_mm_add_memory() so that that check will
succeed.

This fixes the condition where the PS3 boots up with
only the 128 MiB of boot memory.

Signed-off-by: Geoff Levand <redacted>
---

v2: Add call to lmb_analyze().

 arch/powerpc/platforms/ps3/mm.c |    3 +++
 1 file changed, 3 insertions(+)

--- a/arch/powerpc/platforms/ps3/mm.c
+++ b/arch/powerpc/platforms/ps3/mm.c
@@ -317,6 +317,9 @@ static int __init ps3_mm_add_memory(void
 		return result;
 	}
 
+	lmb_add(start_addr, map.r1.size);
+	lmb_analyze();
+
 	result = online_pages(start_pfn, nr_pages);
 
 	if (result)
Hi Paul,

Could you please merge this one in for 2.6.26.  Without it the
system boots with just 128 of the total 256 MiB of memory.

There is the concurrency problem as Ben commented on, but I
think not having the 128 MiB of memory is worse than having the
potential race, which we can work on as a separate fix.

-Geoff





  

    

      
      Geoff Levand      replied to Paul Mackerras      
      
    

  

  

    Message details and related content
    
Open message details and related content

  

  
Paul Mackerras wrote:
Benjamin Herrenschmidt writes:

quoted
When you do an lmb_add you should probably also do an lmb_analyze to
update the total memory count etc...

That leads to some interesting issues such as the LMB stuff wasn't
really meant to be dynamically modified after boot, and thus the kernel
has no locks in there. That can be an issue...

Paul, any thoughts here ? Should we add a lock ? That would mean being
careful as the LMB stuff can be called very early, and spinlock wants
things like PACA and possibly lockdep to be around.. 
Either that, or we give in and use iomem_resource to track where
system RAM is, as well as the other things in the physical address
space, like other architectures do...
The generic hot plug routines already use iomem_resource
(mm/memory_hotplug.c).  Both __add_pages() and add_memory()
add the new mem to iomem_resource, and so it seems there
is no need for the powerpc specific walk_memory_resource(),
since the generic one does its check with iomem_resource.

I need to look a little closer at how the pSeries does
its memory hot plug, but I think removing the powerpc
specific walk_memory_resource() won't effect pSeries since
it seems to have its own hot plug routines that do their
own thing entirely with lmb.

It doesn't seem that it would be difficult to make the
pSeries hot plug code to use iomem_resource, but some of the
generic hot plug routines cannot be called until fairly
late in the startup.

The other thing to do then would be to change the other
powerpc startup code to use iomem_resource instead of lmb.

-Geoff










    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread / next series-diff position

          
kprevious message in thread / previous series-diff position

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help