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
On Tue, 14 Dec 2010 16:01:39 +0100
Ingo Korb <[email protected]> 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
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
On Wed, 15 Dec 2010 10:25:51 +0100
Ingo Korb <[email protected]> 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 <[email protected]>
Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
---
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. .*/
On Thu, Dec 16, 2010 at 09:06:57AM +0900, KAMEZAWA Hiroyuki wrote:
> On Wed, 15 Dec 2010 10:25:51 +0100
> Ingo Korb <[email protected]> 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 <[email protected]>
> Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
Other than the spelling mistakes in the changelog and the lack of a
subject;
Acked-by: Mel Gorman <[email protected]>
> ---
> 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
On Thu, 16 Dec 2010 10:26:41 +0000
Mel Gorman <[email protected]> wrote:
> Other than the spelling mistakes in the changelog and the lack of a
> subject;
>
> Acked-by: Mel Gorman <[email protected]>
>
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 <[email protected]>
Acked-by: Mel Gorman <[email protected]>
Signed-off-by: KAMEZAWA Hiroyuki <[email protected]>
---
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. .*/