linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* PROBLEM: __offline_isolated_pages may offline too many pages
@ 2010-12-14 15:01 Ingo Korb
  2010-12-15  0:21 ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Korb @ 2010-12-14 15:01 UTC (permalink / raw)
  To: linux-mm; +Cc: akpm, mel, cl, yinghai, andi.kleen, linux-kernel

Hi!

[1.] One line summary of the problem:
__offline_isolated_pages may isolate too many pages

[2.] Full description of the problem/report:
While experimenting with remove_memory/online_pages, removing as few 
pages as possible (pageblock_nr_pages, 512 on my box) I noticed that the 
number of pages marked "reserved" increased even though both functions 
did not indicate an error. Following the code it was clear that 
__offline_isolated_pages marked twice as many pages as it should:

=== start paste (from dmesg) ===
Offlined Pages 512
remove from free list c00 1024 e00
=== end paste ===

The issue seems to be that __offline_isolated_pages blindly uses 
page_order() to determine how many pages it should mark as reserved in 
the current loop iteration, without checking if this would exceed the 
limit set by end_pfn.

I'm not sure what the correct way to fix this would be - is memory 
isolation supposed to touch the order of a page if it crosses the end 
(or beginning!) of the range of pages to be isolated?

[3.] Keywords (i.e., modules, networking, kernel):
kernel mm memory-hotplug

[4.] Kernel information
[4.1.] Kernel version (from /proc/version):
Linux version 2.6.35-00002-g76c52bb (ingo@memtester) (gcc version 4.4.5 
(Debian 4.4.5-6) ) #7 SMP Tue Dec 14 14:28:17 CET 2010

The diff between vanilla 2.6.35 and this version is available at 
http://akana.de/memtest35.diff - the only changes are a reduced timeout 
in remove_memory and a bunch of additional exported symbols.

[4.2.] Kernel .config file:
http://akana.de/config-memtest35

[5.] Most recent kernel version which did not have the bug:
Probably none

[8.] Environment
[8.1.] Software (add the output of the ver_linux script here)
Linux memtester 2.6.35-00002-g76c52bb #7 SMP Tue Dec 14 14:28:17 CET 
2010 x86_64 GNU/Linux

Gnu C                  4.4.5
Gnu make               3.81
binutils               2.20.1
util-linux             (no fdformat on the system)
mount                  support
module-init-tools      found
Linux C Library        2.11.2
Dynamic linker (ldd)   2.11.2
Procps                 3.2.8
Kbd                    1.15.2
Sh-utils               8.5
Modules Loaded         phys_mem ipv6 pcspkr i2c_piix4 i2c_core shpchp e1000

Distribution is Debian testing if it matters

[8.2.] Processor information (from /proc/cpuinfo):
AMD Phenom 9650, but the system is running inside a VMWare Player 
instance with just a single virtual CPU

[8.3.] Module information (from /proc/modules):
phys_mem 15068 0 - Live 0xffffffffa00d1000
ipv6 340746 24 - Live 0xffffffffa0068000
pcspkr 2022 0 - Live 0xffffffffa0062000
i2c_piix4 13334 0 - Live 0xffffffffa0059000
i2c_core 28244 1 i2c_piix4, Live 0xffffffffa004b000
shpchp 35612 0 - Live 0xffffffffa003b000
e1000 164575 0 - Live 0xffffffffa0000000

[8.4.] Loaded driver and hardware information (/proc/ioports, /proc/iomem)
[8.5.] PCI information ('lspci -vvv' as root)
[8.6.] SCSI information (from /proc/scsi/scsi)
As far as I can tell irrelevant to this problem?
(forgot to copy those, will add later if neccessary)

-ik

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PROBLEM: __offline_isolated_pages may offline too many pages
  2010-12-14 15:01 PROBLEM: __offline_isolated_pages may offline too many pages Ingo Korb
@ 2010-12-15  0:21 ` KAMEZAWA Hiroyuki
  2010-12-15  9:25   ` Ingo Korb
  0 siblings, 1 reply; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-12-15  0:21 UTC (permalink / raw)
  To: Ingo Korb; +Cc: linux-mm, akpm, mel, cl, yinghai, andi.kleen, linux-kernel

On Tue, 14 Dec 2010 16:01:39 +0100
Ingo Korb <ingo@akana.de> wrote:

> Hi!
> 
> [1.] One line summary of the problem:
> __offline_isolated_pages may isolate too many pages
> 
> [2.] Full description of the problem/report:
> While experimenting with remove_memory/online_pages, removing as few 
> pages as possible (pageblock_nr_pages, 512 on my box) I noticed that the 
> number of pages marked "reserved" increased even though both functions 
> did not indicate an error. Following the code it was clear that 
> __offline_isolated_pages marked twice as many pages as it should:
> 

It's designed for offline memory section > MAX_ORDER. pageblock_nr_pages
is tend to be smaller than that.

Do you see the problem with _exsisting_ user interface of memory hotplug ?
I think we have no control other than memory section.

> === start paste (from dmesg) ===
> Offlined Pages 512
> remove from free list c00 1024 e00
> === end paste ===
> 
> The issue seems to be that __offline_isolated_pages blindly uses 
> page_order() to determine how many pages it should mark as reserved in 
> the current loop iteration, without checking if this would exceed the 
> limit set by end_pfn.
> 

It's because designed to work under memory section, it's aligned to MAX_ORDER.
Its blindness works correctly.


> I'm not sure what the correct way to fix this would be - is memory 
> isolation supposed to touch the order of a page if it crosses the end 
> (or beginning!) of the range of pages to be isolated?
> 

Nothing to be fixed. If you _need_ another functionality, please add a new
feature. But, in theory, memory offline doesn't work in the range smaller
than MAX_ORDER because of buddy allocator.

Thanks,
-Kame

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PROBLEM: __offline_isolated_pages may offline too many pages
  2010-12-15  0:21 ` KAMEZAWA Hiroyuki
@ 2010-12-15  9:25   ` Ingo Korb
  2010-12-16  0:06     ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 6+ messages in thread
From: Ingo Korb @ 2010-12-15  9:25 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: linux-mm, akpm, mel, cl, yinghai, andi.kleen, linux-kernel

On 15.12.2010 01:21, KAMEZAWA Hiroyuki wrote:

> It's designed for offline memory section>  MAX_ORDER. pageblock_nr_pages
> is tend to be smaller than that.
>
> Do you see the problem with _exsisting_ user interface of memory hotplug ?
> I think we have no control other than memory section.

The existing, exported interface (remove_memory() - the check itself is 
in offline_pages()) only checks if both start and end of the 
to-be-removed block are aligned to pageblock_nr_pages. As you noted the 
actual size and alignment requirements in __offline_isolated_pages can 
be larger that that, so I think the checks in offline_pages() should be 
changed (if 1<<MAX_ORDER is always >= pageblock_nr_pages) or extended 
(if there can be any relation between the two).

-ik

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PROBLEM: __offline_isolated_pages may offline too many pages
  2010-12-15  9:25   ` Ingo Korb
@ 2010-12-16  0:06     ` KAMEZAWA Hiroyuki
  2010-12-16 10:26       ` Mel Gorman
  0 siblings, 1 reply; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-12-16  0:06 UTC (permalink / raw)
  To: Ingo Korb; +Cc: linux-mm, akpm, mel, cl, yinghai, andi.kleen, linux-kernel

On Wed, 15 Dec 2010 10:25:51 +0100
Ingo Korb <ingo@akana.de> wrote:

> On 15.12.2010 01:21, KAMEZAWA Hiroyuki wrote:
> 
> > It's designed for offline memory section>  MAX_ORDER. pageblock_nr_pages
> > is tend to be smaller than that.
> >
> > Do you see the problem with _exsisting_ user interface of memory hotplug ?
> > I think we have no control other than memory section.
> 
> The existing, exported interface (remove_memory() - the check itself is 
> in offline_pages()) only checks if both start and end of the 
> to-be-removed block are aligned to pageblock_nr_pages. As you noted the 
> actual size and alignment requirements in __offline_isolated_pages can 
> be larger that that, so I think the checks in offline_pages() should be 
> changed (if 1<<MAX_ORDER is always >= pageblock_nr_pages) or extended 
> (if there can be any relation between the two).
> 

Ok, maybe my mistake. This is a fix. Thank you for reporting.
==

offline_pages()'s sanity check of given range is wrong. It should
be aligned to MAX_ORDER. Current exsiting caller uses SECTION_SIZE
alignment, so this change has no influence to exsisting callers.

Reported-by: Ingo Korb <ingo@akana.de>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/memory_hotplug.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: linux-2.6.37-rc5/mm/memory_hotplug.c
===================================================================
--- linux-2.6.37-rc5.orig/mm/memory_hotplug.c
+++ linux-2.6.37-rc5/mm/memory_hotplug.c
@@ -798,10 +798,14 @@ static int offline_pages(unsigned long s
 	struct memory_notify arg;
 
 	BUG_ON(start_pfn >= end_pfn);
-	/* at least, alignment against pageblock is necessary */
-	if (!IS_ALIGNED(start_pfn, pageblock_nr_pages))
+	/*
+	 * Considering buddy allocator which joins nearby pages, the range
+	 * in offline should be aligned to MAX_ORDER. If not, isolated
+	 * page will be joined to other (not isolated) pages.
+	 */
+	if (!IS_ALIGNED(start_pfn, MAX_ORDER_NR_PAGES))
 		return -EINVAL;
-	if (!IS_ALIGNED(end_pfn, pageblock_nr_pages))
+	if (!IS_ALIGNED(end_pfn, MAX_ORDER_NR_PAGES))
 		return -EINVAL;
 	/* This makes hotplug much easier...and readable.
 	   we assume this for now. .*/

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PROBLEM: __offline_isolated_pages may offline too many pages
  2010-12-16  0:06     ` KAMEZAWA Hiroyuki
@ 2010-12-16 10:26       ` Mel Gorman
  2010-12-16 23:57         ` [PATCH] memory hotplug: fix alignment check (Was " KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 6+ messages in thread
From: Mel Gorman @ 2010-12-16 10:26 UTC (permalink / raw)
  To: KAMEZAWA Hiroyuki
  Cc: Ingo Korb, linux-mm, akpm, cl, yinghai, andi.kleen, linux-kernel

On Thu, Dec 16, 2010 at 09:06:57AM +0900, KAMEZAWA Hiroyuki wrote:
> On Wed, 15 Dec 2010 10:25:51 +0100
> Ingo Korb <ingo@akana.de> wrote:
> 
> > On 15.12.2010 01:21, KAMEZAWA Hiroyuki wrote:
> > 
> > > It's designed for offline memory section>  MAX_ORDER. pageblock_nr_pages
> > > is tend to be smaller than that.
> > >
> > > Do you see the problem with _exsisting_ user interface of memory hotplug ?
> > > I think we have no control other than memory section.
> > 
> > The existing, exported interface (remove_memory() - the check itself is 
> > in offline_pages()) only checks if both start and end of the 
> > to-be-removed block are aligned to pageblock_nr_pages. As you noted the 
> > actual size and alignment requirements in __offline_isolated_pages can 
> > be larger that that, so I think the checks in offline_pages() should be 
> > changed (if 1<<MAX_ORDER is always >= pageblock_nr_pages) or extended 
> > (if there can be any relation between the two).
> > 
> 
> Ok, maybe my mistake. This is a fix. Thank you for reporting.
> ==
> 
> offline_pages()'s sanity check of given range is wrong. It should
> be aligned to MAX_ORDER. Current exsiting caller uses SECTION_SIZE
> alignment, so this change has no influence to exsisting callers.
> 
> Reported-by: Ingo Korb <ingo@akana.de>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

Other than the spelling mistakes in the changelog and the lack of a
subject;

Acked-by: Mel Gorman <mel@csn.ul.ie>

> ---
>  mm/memory_hotplug.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6.37-rc5/mm/memory_hotplug.c
> ===================================================================
> --- linux-2.6.37-rc5.orig/mm/memory_hotplug.c
> +++ linux-2.6.37-rc5/mm/memory_hotplug.c
> @@ -798,10 +798,14 @@ static int offline_pages(unsigned long s
>  	struct memory_notify arg;
>  
>  	BUG_ON(start_pfn >= end_pfn);
> -	/* at least, alignment against pageblock is necessary */
> -	if (!IS_ALIGNED(start_pfn, pageblock_nr_pages))
> +	/*
> +	 * Considering buddy allocator which joins nearby pages, the range
> +	 * in offline should be aligned to MAX_ORDER. If not, isolated
> +	 * page will be joined to other (not isolated) pages.
> +	 */
> +	if (!IS_ALIGNED(start_pfn, MAX_ORDER_NR_PAGES))
>  		return -EINVAL;
> -	if (!IS_ALIGNED(end_pfn, pageblock_nr_pages))
> +	if (!IS_ALIGNED(end_pfn, MAX_ORDER_NR_PAGES))
>  		return -EINVAL;
>  	/* This makes hotplug much easier...and readable.
>  	   we assume this for now. .*/
> 

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] memory hotplug: fix alignment check (Was Re: PROBLEM: __offline_isolated_pages may offline too many pages
  2010-12-16 10:26       ` Mel Gorman
@ 2010-12-16 23:57         ` KAMEZAWA Hiroyuki
  0 siblings, 0 replies; 6+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-12-16 23:57 UTC (permalink / raw)
  To: Mel Gorman
  Cc: Ingo Korb, linux-mm, akpm, cl, yinghai, andi.kleen, linux-kernel

On Thu, 16 Dec 2010 10:26:41 +0000
Mel Gorman <mel@csn.ul.ie> wrote:

> Other than the spelling mistakes in the changelog and the lack of a
> subject;
> 
> Acked-by: Mel Gorman <mel@csn.ul.ie>
> 
Thank you. fixed one.
==

offline_pages()'s sanity check of given range is wrong. It should
be aligned to MAX_ORDER. Current existing caller uses SECTION_SIZE
alignment, so this change has no influence to existing callers.

Reported-by: Ingo Korb <ingo@akana.de>
Acked-by: Mel Gorman <mel@csn.ul.ie>
Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
 mm/memory_hotplug.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: linux-2.6.37-rc5/mm/memory_hotplug.c
===================================================================
--- linux-2.6.37-rc5.orig/mm/memory_hotplug.c
+++ linux-2.6.37-rc5/mm/memory_hotplug.c
@@ -798,10 +798,14 @@ static int offline_pages(unsigned long s
 	struct memory_notify arg;
 
 	BUG_ON(start_pfn >= end_pfn);
-	/* at least, alignment against pageblock is necessary */
-	if (!IS_ALIGNED(start_pfn, pageblock_nr_pages))
+	/*
+	 * Considering Buddy system which joins nearby pages, the range
+	 * in offline should be aligned to MAX_ORDER. If not, isolated
+	 * page will be joined to other (not isolated) pages.
+	 */
+	if (!IS_ALIGNED(start_pfn, MAX_ORDER_NR_PAGES))
 		return -EINVAL;
-	if (!IS_ALIGNED(end_pfn, pageblock_nr_pages))
+	if (!IS_ALIGNED(end_pfn, MAX_ORDER_NR_PAGES))
 		return -EINVAL;
 	/* This makes hotplug much easier...and readable.
 	   we assume this for now. .*/
 

--
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/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-12-17  0:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-14 15:01 PROBLEM: __offline_isolated_pages may offline too many pages Ingo Korb
2010-12-15  0:21 ` KAMEZAWA Hiroyuki
2010-12-15  9:25   ` Ingo Korb
2010-12-16  0:06     ` KAMEZAWA Hiroyuki
2010-12-16 10:26       ` Mel Gorman
2010-12-16 23:57         ` [PATCH] memory hotplug: fix alignment check (Was " KAMEZAWA Hiroyuki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).