Thread (5 messages) flat view 5 messages, 3 authors, 2012-10-09

Re: memory-hotplug : suppres "Trying to free nonexistent resource <XXXXXXXXXXXXXXXX-YYYYYYYYYYYYYYYY>" warning

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2012-10-05 21:09:40
Also in: linux-acpi, linux-mm

On Thu, 4 Oct 2012 14:31:09 +0900
Yasuaki Ishimatsu [off-list ref] wrote:
quoted hunk ↗ jump to hunk
When our x86 box calls __remove_pages(), release_mem_region() shows
many warnings. And x86 box cannot unregister iomem_resource.

"Trying to free nonexistent resource <XXXXXXXXXXXXXXXX-YYYYYYYYYYYYYYYY>"

release_mem_region() has been changed as called in each PAGES_PER_SECTION
chunk since applying a patch(de7f0cba96786c). Because powerpc registers
iomem_resource in each PAGES_PER_SECTION chunk. But when I hot add memory
on x86 box, iomem_resource is register in each _CRS not PAGES_PER_SECTION
chunk. So x86 box unregisters iomem_resource.

The patch fixes the problem.
--- linux-3.6.orig/arch/powerpc/platforms/pseries/hotplug-memory.c	2012-10-04 14:22:59.833520792 +0900
+++ linux-3.6/arch/powerpc/platforms/pseries/hotplug-memory.c	2012-10-04 14:23:05.150521411 +0900
@@ -77,7 +77,8 @@ static int pseries_remove_memblock(unsig
 {
 	unsigned long start, start_pfn;
 	struct zone *zone;
-	int ret;
+	int i, ret;
+	int sections_to_remove;
 
 	start_pfn = base >> PAGE_SHIFT;
 
@@ -97,9 +98,13 @@ static int pseries_remove_memblock(unsig
 	 * to sysfs "state" file and we can't remove sysfs entries
 	 * while writing to it. So we have to defer it to here.
 	 */
-	ret = __remove_pages(zone, start_pfn, memblock_size >> PAGE_SHIFT);
-	if (ret)
-		return ret;
+	sections_to_remove = (memblock_size >> PAGE_SHIFT) / PAGES_PER_SECTION;
+	for (i = 0; i < sections_to_remove; i++) {
+		unsigned long pfn = start_pfn + i * PAGES_PER_SECTION;
+		ret = __remove_pages(zone, start_pfn,  PAGES_PER_SECTION);
+		if (ret)
+			return ret;
+	}
It is inappropriate that `i' have a signed 32-bit type.  I doubt if
there's any possibility of an overflow bug here, but using a consistent
and well-chosen type would eliminate all doubt.

Note that __remove_pages() does use an unsigned long for this, although
it stupidly calls that variable "i", despite the C programmers'
expectation that a variable called "i" has type "int".

The same applies to `sections_to_remove', but __remove_pages() went and
decided to use an `int' for that variable.  Sigh.

Anyway, please have a think, and see if we can come up with the best
and most accurate choice of types and identifiers in this code.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help