All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Weiner <hannes@cmpxchg.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Joonsoo Kim <js1304@gmail.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: [PATCH 4/4] bootmem: fix wrong call parameter for free_bootmem()
Date: Mon, 12 Nov 2012 20:24:37 -0500	[thread overview]
Message-ID: <20121113012436.GG10092@cmpxchg.org> (raw)
In-Reply-To: <20121112152342.ce90052a.akpm@linux-foundation.org>

On Mon, Nov 12, 2012 at 03:23:42PM -0800, Andrew Morton wrote:
> On Tue, 13 Nov 2012 01:31:55 +0900
> Joonsoo Kim <js1304@gmail.com> wrote:
> 
> > It is somehow strange that alloc_bootmem return virtual address
> > and free_bootmem require physical address.
> > Anyway, free_bootmem()'s first parameter should be physical address.
> > 
> > There are some call sites for free_bootmem() with virtual address.
> > So fix them.
> 
> Well gee, I wonder how that happened :(

free_bootmem() is the counterpart to reserve_bootmem() to configure
physical memory ranges.  Allocations are assumed to be permanent.

> How does this look?

Much better.  How about this in addition?

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---

diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
index 7b74452..b519cb2 100644
--- a/include/linux/bootmem.h
+++ b/include/linux/bootmem.h
@@ -49,7 +49,7 @@ extern unsigned long free_all_bootmem_node(pg_data_t *pgdat);
 extern unsigned long free_all_bootmem(void);
 
 extern void free_bootmem_node(pg_data_t *pgdat,
-			      unsigned long addr,
+			      unsigned long physaddr,
 			      unsigned long size);
 extern void free_bootmem(unsigned long physaddr, unsigned long size);
 extern void free_bootmem_late(unsigned long physaddr, unsigned long size);
diff --git a/mm/bootmem.c b/mm/bootmem.c
index 4c079ba..5135d84 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -147,7 +147,7 @@ unsigned long __init init_bootmem(unsigned long start, unsigned long pages)
 
 /*
  * free_bootmem_late - free bootmem pages directly to page allocator
- * @addr: starting physical address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * This is only useful when the bootmem allocator has already been torn
@@ -385,7 +385,7 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
 
 /**
  * free_bootmem - mark a page range as usable
- * @addr: starting physical address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * Partial pages will be considered reserved and left as they are.
diff --git a/mm/nobootmem.c b/mm/nobootmem.c
index 714d5d6..a532960 100644
--- a/mm/nobootmem.c
+++ b/mm/nobootmem.c
@@ -58,21 +58,21 @@ static void * __init __alloc_memory_core_early(int nid, u64 size, u64 align,
 
 /*
  * free_bootmem_late - free bootmem pages directly to page allocator
- * @addr: starting address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * This is only useful when the bootmem allocator has already been torn
  * down, but we are still initializing the system.  Pages are given directly
  * to the page allocator, no bootmem metadata is updated because it is gone.
  */
-void __init free_bootmem_late(unsigned long addr, unsigned long size)
+void __init free_bootmem_late(unsigned long physaddr, unsigned long size)
 {
 	unsigned long cursor, end;
 
-	kmemleak_free_part(__va(addr), size);
+	kmemleak_free_part(__va(physaddr), size);
 
-	cursor = PFN_UP(addr);
-	end = PFN_DOWN(addr + size);
+	cursor = PFN_UP(physaddr);
+	end = PFN_DOWN(physaddr + size);
 
 	for (; cursor < end; cursor++) {
 		__free_pages_bootmem(pfn_to_page(cursor), 0);
@@ -188,17 +188,17 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
 
 /**
  * free_bootmem - mark a page range as usable
- * @addr: starting address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * Partial pages will be considered reserved and left as they are.
  *
  * The range must be contiguous but may span node boundaries.
  */
-void __init free_bootmem(unsigned long addr, unsigned long size)
+void __init free_bootmem(unsigned long physaddr, unsigned long size)
 {
-	kmemleak_free_part(__va(addr), size);
-	memblock_free(addr, size);
+	kmemleak_free_part(__va(physaddr), size);
+	memblock_free(physaddr, size);
 }
 
 static void * __init ___alloc_bootmem_nopanic(unsigned long size,

--
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>

WARNING: multiple messages have this Message-ID (diff)
From: Johannes Weiner <hannes@cmpxchg.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Joonsoo Kim <js1304@gmail.com>,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: Re: [PATCH 4/4] bootmem: fix wrong call parameter for free_bootmem()
Date: Mon, 12 Nov 2012 20:24:37 -0500	[thread overview]
Message-ID: <20121113012436.GG10092@cmpxchg.org> (raw)
In-Reply-To: <20121112152342.ce90052a.akpm@linux-foundation.org>

On Mon, Nov 12, 2012 at 03:23:42PM -0800, Andrew Morton wrote:
> On Tue, 13 Nov 2012 01:31:55 +0900
> Joonsoo Kim <js1304@gmail.com> wrote:
> 
> > It is somehow strange that alloc_bootmem return virtual address
> > and free_bootmem require physical address.
> > Anyway, free_bootmem()'s first parameter should be physical address.
> > 
> > There are some call sites for free_bootmem() with virtual address.
> > So fix them.
> 
> Well gee, I wonder how that happened :(

free_bootmem() is the counterpart to reserve_bootmem() to configure
physical memory ranges.  Allocations are assumed to be permanent.

> How does this look?

Much better.  How about this in addition?

Signed-off-by: Johannes Weiner <hannes@cmpxchg.org>
---

diff --git a/include/linux/bootmem.h b/include/linux/bootmem.h
index 7b74452..b519cb2 100644
--- a/include/linux/bootmem.h
+++ b/include/linux/bootmem.h
@@ -49,7 +49,7 @@ extern unsigned long free_all_bootmem_node(pg_data_t *pgdat);
 extern unsigned long free_all_bootmem(void);
 
 extern void free_bootmem_node(pg_data_t *pgdat,
-			      unsigned long addr,
+			      unsigned long physaddr,
 			      unsigned long size);
 extern void free_bootmem(unsigned long physaddr, unsigned long size);
 extern void free_bootmem_late(unsigned long physaddr, unsigned long size);
diff --git a/mm/bootmem.c b/mm/bootmem.c
index 4c079ba..5135d84 100644
--- a/mm/bootmem.c
+++ b/mm/bootmem.c
@@ -147,7 +147,7 @@ unsigned long __init init_bootmem(unsigned long start, unsigned long pages)
 
 /*
  * free_bootmem_late - free bootmem pages directly to page allocator
- * @addr: starting physical address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * This is only useful when the bootmem allocator has already been torn
@@ -385,7 +385,7 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
 
 /**
  * free_bootmem - mark a page range as usable
- * @addr: starting physical address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * Partial pages will be considered reserved and left as they are.
diff --git a/mm/nobootmem.c b/mm/nobootmem.c
index 714d5d6..a532960 100644
--- a/mm/nobootmem.c
+++ b/mm/nobootmem.c
@@ -58,21 +58,21 @@ static void * __init __alloc_memory_core_early(int nid, u64 size, u64 align,
 
 /*
  * free_bootmem_late - free bootmem pages directly to page allocator
- * @addr: starting address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * This is only useful when the bootmem allocator has already been torn
  * down, but we are still initializing the system.  Pages are given directly
  * to the page allocator, no bootmem metadata is updated because it is gone.
  */
-void __init free_bootmem_late(unsigned long addr, unsigned long size)
+void __init free_bootmem_late(unsigned long physaddr, unsigned long size)
 {
 	unsigned long cursor, end;
 
-	kmemleak_free_part(__va(addr), size);
+	kmemleak_free_part(__va(physaddr), size);
 
-	cursor = PFN_UP(addr);
-	end = PFN_DOWN(addr + size);
+	cursor = PFN_UP(physaddr);
+	end = PFN_DOWN(physaddr + size);
 
 	for (; cursor < end; cursor++) {
 		__free_pages_bootmem(pfn_to_page(cursor), 0);
@@ -188,17 +188,17 @@ void __init free_bootmem_node(pg_data_t *pgdat, unsigned long physaddr,
 
 /**
  * free_bootmem - mark a page range as usable
- * @addr: starting address of the range
+ * @physaddr: starting physical address of the range
  * @size: size of the range in bytes
  *
  * Partial pages will be considered reserved and left as they are.
  *
  * The range must be contiguous but may span node boundaries.
  */
-void __init free_bootmem(unsigned long addr, unsigned long size)
+void __init free_bootmem(unsigned long physaddr, unsigned long size)
 {
-	kmemleak_free_part(__va(addr), size);
-	memblock_free(addr, size);
+	kmemleak_free_part(__va(physaddr), size);
+	memblock_free(physaddr, size);
 }
 
 static void * __init ___alloc_bootmem_nopanic(unsigned long size,

  parent reply	other threads:[~2012-11-13  1:24 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-12 16:31 [PATCH 1/4] bootmem: remove not implemented function call, bootmem_arch_preferred_node() Joonsoo Kim
2012-11-12 16:31 ` Joonsoo Kim
2012-11-12 16:31 ` [PATCH 2/4] avr32, kconfig: remove HAVE_ARCH_BOOTMEM Joonsoo Kim
2012-11-12 16:31   ` Joonsoo Kim
2012-11-12 18:36   ` Hans-Christian Egtvedt
2012-11-12 18:36     ` Hans-Christian Egtvedt
2012-11-13  0:50   ` Johannes Weiner
2012-11-13  0:50     ` Johannes Weiner
2012-11-12 16:31 ` [PATCH 3/4] bootmem: remove alloc_arch_preferred_bootmem() Joonsoo Kim
2012-11-12 16:31   ` Joonsoo Kim
2012-11-13  0:55   ` Johannes Weiner
2012-11-13  0:55     ` Johannes Weiner
2012-11-12 16:31 ` [PATCH 4/4] bootmem: fix wrong call parameter for free_bootmem() Joonsoo Kim
2012-11-12 16:31   ` Joonsoo Kim
2012-11-12 23:23   ` Andrew Morton
2012-11-12 23:23     ` Andrew Morton
2012-11-13  0:36     ` JoonSoo Kim
2012-11-13  0:36       ` JoonSoo Kim
2012-11-13  1:24     ` Johannes Weiner [this message]
2012-11-13  1:24       ` Johannes Weiner
2012-11-13  0:58   ` Johannes Weiner
2012-11-13  0:58     ` Johannes Weiner
2012-11-13  1:10   ` Johannes Weiner
2012-11-13  1:10     ` Johannes Weiner
2012-11-13  1:13     ` Johannes Weiner
2012-11-13  1:13       ` Johannes Weiner
2012-11-13  0:49 ` [PATCH 1/4] bootmem: remove not implemented function call, bootmem_arch_preferred_node() Johannes Weiner
2012-11-13  0:49   ` Johannes Weiner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121113012436.GG10092@cmpxchg.org \
    --to=hannes@cmpxchg.org \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=js1304@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.