2013-05-26 05:58:58

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 1/6] mm/memory-hotplug: fix lowmem count overflow when offline pages

Changelog:
v1 -> v2:
* show number of HighTotal before hotremove
* remove CONFIG_HIGHMEM
* cc stable kernels
* add Michal reviewed-by

Logic memory-remove code fails to correctly account the Total High Memory
when a memory block which contains High Memory is offlined as shown in the
example below. The following patch fixes it.

Stable for 2.6.24+.

Before logic memory remove:

MemTotal: 7603740 kB
MemFree: 6329612 kB
Buffers: 94352 kB
Cached: 872008 kB
SwapCached: 0 kB
Active: 626932 kB
Inactive: 519216 kB
Active(anon): 180776 kB
Inactive(anon): 222944 kB
Active(file): 446156 kB
Inactive(file): 296272 kB
Unevictable: 0 kB
Mlocked: 0 kB
HighTotal: 7294672 kB
HighFree: 5704696 kB
LowTotal: 309068 kB
LowFree: 624916 kB

After logic memory remove:

MemTotal: 7079452 kB
MemFree: 5805976 kB
Buffers: 94372 kB
Cached: 872000 kB
SwapCached: 0 kB
Active: 626936 kB
Inactive: 519236 kB
Active(anon): 180780 kB
Inactive(anon): 222944 kB
Active(file): 446156 kB
Inactive(file): 296292 kB
Unevictable: 0 kB
Mlocked: 0 kB
HighTotal: 7294672 kB
HighFree: 5181024 kB
LowTotal: 4294752076 kB
LowFree: 624952 kB

Reviewed-by: Michal Hocko <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
mm/page_alloc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 98cbdf6..23b921f 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6140,6 +6140,8 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
list_del(&page->lru);
rmv_page_order(page);
zone->free_area[order].nr_free--;
+ if (PageHighMem(page))
+ totalhigh_pages -= 1 << order;
for (i = 0; i < (1 << order); i++)
SetPageReserved((page+i));
pfn += (1 << order);
--
1.8.1.2


2013-05-26 05:59:40

by Wanpeng Li

[permalink] [raw]
Subject: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
it was born, this patch disable memory hotremove when 32bit at compile
time.

Suggested-by: KOSAKI Motohiro <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
mm/Kconfig | 1 +
1 file changed, 1 insertion(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index e742d06..ada9569 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -184,6 +184,7 @@ config MEMORY_HOTREMOVE
bool "Allow for memory hot remove"
select MEMORY_ISOLATION
select HAVE_BOOTMEM_INFO_NODE if X86_64
+ depends on 64BIT
depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE
depends on MIGRATION

--
1.8.1.2

2013-05-26 05:59:38

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 5/6] mm/pageblock: remove get/set_pageblock_flags

Changelog:
v1 -> v2:
* add Michal reviewed-by

get_pageblock_flags and set_pageblock_flags are not used any
more, this patch remove them.

Reviewed-by: Michal Hocko <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
include/linux/pageblock-flags.h | 6 ------
1 file changed, 6 deletions(-)

diff --git a/include/linux/pageblock-flags.h b/include/linux/pageblock-flags.h
index be655e4..2ee8cd2 100644
--- a/include/linux/pageblock-flags.h
+++ b/include/linux/pageblock-flags.h
@@ -80,10 +80,4 @@ void set_pageblock_flags_group(struct page *page, unsigned long flags,
PB_migrate_skip)
#endif /* CONFIG_COMPACTION */

-#define get_pageblock_flags(page) \
- get_pageblock_flags_group(page, 0, PB_migrate_end)
-#define set_pageblock_flags(page, flags) \
- set_pageblock_flags_group(page, flags, \
- 0, PB_migrate_end)
-
#endif /* PAGEBLOCK_FLAGS_H */
--
1.8.1.2

2013-05-26 05:59:34

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 6/6] mm/hugetlb: remove hugetlb_prefault

Changelog:
v1 -> v2:
* add Michal reviewed-by

hugetlb_prefault are not used any more, this patch remove it.

Reviewed-by: Michal Hocko <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
include/linux/hugetlb.h | 2 --
1 file changed, 2 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index 6b4890f..a811149 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -55,7 +55,6 @@ void __unmap_hugepage_range_final(struct mmu_gather *tlb,
void __unmap_hugepage_range(struct mmu_gather *tlb, struct vm_area_struct *vma,
unsigned long start, unsigned long end,
struct page *ref_page);
-int hugetlb_prefault(struct address_space *, struct vm_area_struct *);
void hugetlb_report_meminfo(struct seq_file *);
int hugetlb_report_node_meminfo(int, char *);
void hugetlb_show_meminfo(void);
@@ -110,7 +109,6 @@ static inline unsigned long hugetlb_total_pages(void)
#define follow_hugetlb_page(m,v,p,vs,a,b,i,w) ({ BUG(); 0; })
#define follow_huge_addr(mm, addr, write) ERR_PTR(-EINVAL)
#define copy_hugetlb_page_range(src, dst, vma) ({ BUG(); 0; })
-#define hugetlb_prefault(mapping, vma) ({ BUG(); 0; })
static inline void hugetlb_report_meminfo(struct seq_file *m)
{
}
--
1.8.1.2

2013-05-26 05:59:30

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 4/6] mm/hugetlb: use already exist interface huge_page_shift

Changelog:
v1 -> v2:
* update alloc_bootmem_huge_page in powerpc
* add Michal reviewed-by

Use already exist interface huge_page_shift instead of h->order + PAGE_SHIFT.

Reviewed-by: Michal Hocko <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
arch/powerpc/mm/hugetlbpage.c | 2 +-
mm/hugetlb.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 237c8e5..cafec93 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -357,7 +357,7 @@ void add_gpage(u64 addr, u64 page_size, unsigned long number_of_pages)
int alloc_bootmem_huge_page(struct hstate *hstate)
{
struct huge_bootmem_page *m;
- int idx = shift_to_mmu_psize(hstate->order + PAGE_SHIFT);
+ int idx = shift_to_mmu_psize(huge_page_shift(hstate));
int nr_gpages = gpage_freearray[idx].nr_gpages;

if (nr_gpages == 0)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index f8feeec..b6ff0ee 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -319,7 +319,7 @@ unsigned long vma_kernel_pagesize(struct vm_area_struct *vma)

hstate = hstate_vma(vma);

- return 1UL << (hstate->order + PAGE_SHIFT);
+ return 1UL << huge_page_shift(hstate);
}
EXPORT_SYMBOL_GPL(vma_kernel_pagesize);

--
1.8.1.2

2013-05-26 05:59:26

by Wanpeng Li

[permalink] [raw]
Subject: [PATCH v3 2/6] mm/memory_hotplug: remove memory_add_physaddr_to_nid

memory_add_physaddr_to_nid is not used any more, this patch remove it.

Signed-off-by: Wanpeng Li <[email protected]>
---
arch/x86/mm/numa.c | 15 ---------------
1 file changed, 15 deletions(-)

diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
index a71c4e2..d470a54 100644
--- a/arch/x86/mm/numa.c
+++ b/arch/x86/mm/numa.c
@@ -803,18 +803,3 @@ const struct cpumask *cpumask_of_node(int node)
EXPORT_SYMBOL(cpumask_of_node);

#endif /* !CONFIG_DEBUG_PER_CPU_MAPS */
-
-#ifdef CONFIG_MEMORY_HOTPLUG
-int memory_add_physaddr_to_nid(u64 start)
-{
- struct numa_meminfo *mi = &numa_meminfo;
- int nid = mi->blk[0].nid;
- int i;
-
- for (i = 0; i < mi->nr_blks; i++)
- if (mi->blk[i].start <= start && mi->blk[i].end > start)
- nid = mi->blk[i].nid;
- return nid;
-}
-EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
-#endif
--
1.8.1.2

2013-05-26 08:59:44

by Michal Hocko

[permalink] [raw]
Subject: Re: [PATCH v3 2/6] mm/memory_hotplug: remove memory_add_physaddr_to_nid

On Sun 26-05-13 13:58:37, Wanpeng Li wrote:
> memory_add_physaddr_to_nid is not used any more, this patch remove it.

git grep disagrees.
git grep "= *\<memory_add_physaddr_to_nid\>" mmotm
mmotm:drivers/acpi/acpi_memhotplug.c: node = memory_add_physaddr_to_nid(info->start_addr);
mmotm:drivers/acpi/acpi_memhotplug.c: nid = memory_add_physaddr_to_nid(info->start_addr);
mmotm:drivers/base/memory.c: nid = memory_add_physaddr_to_nid(phys_addr);
mmotm:drivers/xen/balloon.c: nid = memory_add_physaddr_to_nid(hotplug_start_paddr);

>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> arch/x86/mm/numa.c | 15 ---------------
> 1 file changed, 15 deletions(-)
>
> diff --git a/arch/x86/mm/numa.c b/arch/x86/mm/numa.c
> index a71c4e2..d470a54 100644
> --- a/arch/x86/mm/numa.c
> +++ b/arch/x86/mm/numa.c
> @@ -803,18 +803,3 @@ const struct cpumask *cpumask_of_node(int node)
> EXPORT_SYMBOL(cpumask_of_node);
>
> #endif /* !CONFIG_DEBUG_PER_CPU_MAPS */
> -
> -#ifdef CONFIG_MEMORY_HOTPLUG
> -int memory_add_physaddr_to_nid(u64 start)
> -{
> - struct numa_meminfo *mi = &numa_meminfo;
> - int nid = mi->blk[0].nid;
> - int i;
> -
> - for (i = 0; i < mi->nr_blks; i++)
> - if (mi->blk[i].start <= start && mi->blk[i].end > start)
> - nid = mi->blk[i].nid;
> - return nid;
> -}
> -EXPORT_SYMBOL_GPL(memory_add_physaddr_to_nid);
> -#endif
> --
> 1.8.1.2
>

--
Michal Hocko
SUSE Labs

2013-05-26 09:01:02

by Michal Hocko

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

On Sun 26-05-13 13:58:38, Wanpeng Li wrote:
> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
> it was born,

Why? any reference? This reasoning is really weak.

> this patch disable memory hotremove when 32bit at compile
> time.
>
> Suggested-by: KOSAKI Motohiro <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> mm/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/mm/Kconfig b/mm/Kconfig
> index e742d06..ada9569 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -184,6 +184,7 @@ config MEMORY_HOTREMOVE
> bool "Allow for memory hot remove"
> select MEMORY_ISOLATION
> select HAVE_BOOTMEM_INFO_NODE if X86_64
> + depends on 64BIT
> depends on MEMORY_HOTPLUG && ARCH_ENABLE_MEMORY_HOTREMOVE
> depends on MIGRATION
>
> --
> 1.8.1.2
>

--
Michal Hocko
SUSE Labs

2013-05-26 11:49:56

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] mm/memory-hotplug: fix lowmem count overflow when offline pages

On Sun, May 26, 2013 at 1:58 AM, Wanpeng Li <[email protected]> wrote:
> Changelog:
> v1 -> v2:
> * show number of HighTotal before hotremove
> * remove CONFIG_HIGHMEM
> * cc stable kernels
> * add Michal reviewed-by
>
> Logic memory-remove code fails to correctly account the Total High Memory
> when a memory block which contains High Memory is offlined as shown in the
> example below. The following patch fixes it.
>
> Stable for 2.6.24+.
>
> Before logic memory remove:
>
> MemTotal: 7603740 kB
> MemFree: 6329612 kB
> Buffers: 94352 kB
> Cached: 872008 kB
> SwapCached: 0 kB
> Active: 626932 kB
> Inactive: 519216 kB
> Active(anon): 180776 kB
> Inactive(anon): 222944 kB
> Active(file): 446156 kB
> Inactive(file): 296272 kB
> Unevictable: 0 kB
> Mlocked: 0 kB
> HighTotal: 7294672 kB
> HighFree: 5704696 kB
> LowTotal: 309068 kB
> LowFree: 624916 kB
>
> After logic memory remove:
>
> MemTotal: 7079452 kB
> MemFree: 5805976 kB
> Buffers: 94372 kB
> Cached: 872000 kB
> SwapCached: 0 kB
> Active: 626936 kB
> Inactive: 519236 kB
> Active(anon): 180780 kB
> Inactive(anon): 222944 kB
> Active(file): 446156 kB
> Inactive(file): 296292 kB
> Unevictable: 0 kB
> Mlocked: 0 kB
> HighTotal: 7294672 kB
> HighFree: 5181024 kB
> LowTotal: 4294752076 kB
> LowFree: 624952 kB
>
> Reviewed-by: Michal Hocko <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 98cbdf6..23b921f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6140,6 +6140,8 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
> list_del(&page->lru);
> rmv_page_order(page);
> zone->free_area[order].nr_free--;
> + if (PageHighMem(page))
> + totalhigh_pages -= 1 << order;
> for (i = 0; i < (1 << order); i++)
> SetPageReserved((page+i));
> pfn += (1 << order);

Hm. I already NAKed and you didn't answered my question. isn't it?

2013-05-26 11:59:05

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

>> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
>> it was born,
>
> Why? any reference? This reasoning is really weak.

I have no seen any highmem support in memory hotplug code and I don't think this
patch fixes all 32bit highmem issue. If anybody are interesting to
support it, it is good thing. But in fact, _now_ it is broken when
enable HIGHMEM.
So, I just want to mark broken until someone want to support highmem
and verify overall.

And, yes, this patch is no good. Kconfig doesn't describe why disable
when highmem.
So,

depends on 64BIT || !HIGHMEM || BROKEN

maybe clear documentation more.

2013-05-26 12:45:18

by Hush Bensen

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

于 2013/5/26 19:58, KOSAKI Motohiro 写道:
>>> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
>>> it was born,
>> Why? any reference? This reasoning is really weak.
> I have no seen any highmem support in memory hotplug code and I don't think this
> patch fixes all 32bit highmem issue. If anybody are interesting to
> support it, it is good thing. But in fact, _now_ it is broken when
> enable HIGHMEM.

But online/offline memory can work well when enable HIGHMEM, isn't it?

> So, I just want to mark broken until someone want to support highmem
> and verify overall.
>
> And, yes, this patch is no good. Kconfig doesn't describe why disable
> when highmem.
> So,
>
> depends on 64BIT || !HIGHMEM || BROKEN
>
> maybe clear documentation more.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [email protected]. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

2013-05-26 13:07:15

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

On Sun, May 26, 2013 at 8:45 AM, Hush Bensen <[email protected]> wrote:
> 于 2013/5/26 19:58, KOSAKI Motohiro 写道:
>
>>>> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
>>>> it was born,
>>>
>>> Why? any reference? This reasoning is really weak.
>>
>> I have no seen any highmem support in memory hotplug code and I don't
>> think this
>> patch fixes all 32bit highmem issue. If anybody are interesting to
>> support it, it is good thing. But in fact, _now_ it is broken when
>> enable HIGHMEM.
>
>
> But online/offline memory can work well when enable HIGHMEM, isn't it?

If you are lucky.

2013-05-26 13:49:33

by Hush Bensen

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

于 2013/5/26 21:06, KOSAKI Motohiro 写道:
> On Sun, May 26, 2013 at 8:45 AM, Hush Bensen <[email protected]> wrote:
>> 于 2013/5/26 19:58, KOSAKI Motohiro 写道:
>>
>>>>> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
>>>>> it was born,
>>>> Why? any reference? This reasoning is really weak.
>>> I have no seen any highmem support in memory hotplug code and I don't
>>> think this
>>> patch fixes all 32bit highmem issue. If anybody are interesting to
>>> support it, it is good thing. But in fact, _now_ it is broken when
>>> enable HIGHMEM.
>>
>> But online/offline memory can work well when enable HIGHMEM, isn't it?
> If you are lucky.
I think it can work well on my x86_32 with highmem enable box.

2013-05-26 14:26:50

by Jiang Liu

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] mm/memory-hotplug: fix lowmem count overflow when offline pages

On Sun 26 May 2013 01:58:36 PM CST, Wanpeng Li wrote:
> Changelog:
> v1 -> v2:
> * show number of HighTotal before hotremove
> * remove CONFIG_HIGHMEM
> * cc stable kernels
> * add Michal reviewed-by
>
> Logic memory-remove code fails to correctly account the Total High Memory
> when a memory block which contains High Memory is offlined as shown in the
> example below. The following patch fixes it.
>
> Stable for 2.6.24+.
>
> Before logic memory remove:
>
> MemTotal: 7603740 kB
> MemFree: 6329612 kB
> Buffers: 94352 kB
> Cached: 872008 kB
> SwapCached: 0 kB
> Active: 626932 kB
> Inactive: 519216 kB
> Active(anon): 180776 kB
> Inactive(anon): 222944 kB
> Active(file): 446156 kB
> Inactive(file): 296272 kB
> Unevictable: 0 kB
> Mlocked: 0 kB
> HighTotal: 7294672 kB
> HighFree: 5704696 kB
> LowTotal: 309068 kB
> LowFree: 624916 kB
>
> After logic memory remove:
>
> MemTotal: 7079452 kB
> MemFree: 5805976 kB
> Buffers: 94372 kB
> Cached: 872000 kB
> SwapCached: 0 kB
> Active: 626936 kB
> Inactive: 519236 kB
> Active(anon): 180780 kB
> Inactive(anon): 222944 kB
> Active(file): 446156 kB
> Inactive(file): 296292 kB
> Unevictable: 0 kB
> Mlocked: 0 kB
> HighTotal: 7294672 kB
> HighFree: 5181024 kB
> LowTotal: 4294752076 kB
> LowFree: 624952 kB
>
> Reviewed-by: Michal Hocko <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> mm/page_alloc.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 98cbdf6..23b921f 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6140,6 +6140,8 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
> list_del(&page->lru);
> rmv_page_order(page);
> zone->free_area[order].nr_free--;
> + if (PageHighMem(page))
> + totalhigh_pages -= 1 << order;
> for (i = 0; i < (1 << order); i++)
> SetPageReserved((page+i));
> pfn += (1 << order);

Hi Wanpeng,
The memory hotplug code adjusts totalram_pages,
totalhigh_pages, zone->present_pages
and zone->managed_pages all in memory_hotplug.c, so suggest to move
this into memory_hotplug.c
too.
One of my patch fixes this issue in another way, please refer
to:
http://marc.info/?l=linux-mm&m=136957578620221&w=2
Regards!
Gerry

2013-05-26 18:09:39

by Michal Hocko

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

On Sun 26-05-13 07:58:42, KOSAKI Motohiro wrote:
> >> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
> >> it was born,
> >
> > Why? any reference? This reasoning is really weak.
>
> I have no seen any highmem support in memory hotplug code and I don't think this
> patch fixes all 32bit highmem issue. If anybody are interesting to
> support it, it is good thing. But in fact, _now_ it is broken when
> enable HIGHMEM.
> So, I just want to mark broken until someone want to support highmem
> and verify overall.
>
> And, yes, this patch is no good. Kconfig doesn't describe why disable
> when highmem.
> So,
>
> depends on 64BIT || !HIGHMEM || BROKEN
>
> maybe clear documentation more.

I have no objection to disbale the feature for HIGHMEM configurations I
was merely complaining that the patch didn't describe _why_.

--
Michal Hocko
SUSE Labs

2013-05-26 18:12:14

by Michal Hocko

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

On Sun 26-05-13 17:06:17, Wanpeng Li wrote:
> On Sun, May 26, 2013 at 11:00:54AM +0200, Michal Hocko wrote:
> >On Sun 26-05-13 13:58:38, Wanpeng Li wrote:
> >> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
> >> it was born,
> >
> >Why? any reference? This reasoning is really weak.
> >
>
> http://marc.info/?l=linux-mm&m=136953099010171&w=2

I have seen the email but that email just states that the feature is
broken. Maybe it is obvious to you _what_ is actually broken but it
doesn't need to be others especially those who would be reading such
changelog later. So if you consider this configuration broken then be
specific what is broken.

--
Michal Hocko
SUSE Labs

2013-05-26 22:02:58

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

(5/26/13 2:09 PM), Michal Hocko wrote:
> On Sun 26-05-13 07:58:42, KOSAKI Motohiro wrote:
>>>> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
>>>> it was born,
>>>
>>> Why? any reference? This reasoning is really weak.
>>
>> I have no seen any highmem support in memory hotplug code and I don't think this
>> patch fixes all 32bit highmem issue. If anybody are interesting to
>> support it, it is good thing. But in fact, _now_ it is broken when
>> enable HIGHMEM.
>> So, I just want to mark broken until someone want to support highmem
>> and verify overall.
>>
>> And, yes, this patch is no good. Kconfig doesn't describe why disable
>> when highmem.
>> So,
>>
>> depends on 64BIT || !HIGHMEM || BROKEN
>>
>> maybe clear documentation more.
>
> I have no objection to disbale the feature for HIGHMEM configurations I
> was merely complaining that the patch didn't describe _why_.

No worry. I withdrew the claim because several people now willing to contribute
32bit highmem improvement. I don't want to block them.

2013-05-27 06:42:29

by KOSAKI Motohiro

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] mm/memory-hotplug: fix lowmem count overflow when offline pages

(5/26/13 8:02 PM), Wanpeng Li wrote:
> On Sun, May 26, 2013 at 07:49:33AM -0400, KOSAKI Motohiro wrote:
>> On Sun, May 26, 2013 at 1:58 AM, Wanpeng Li <[email protected]> wrote:
>>> Changelog:
>>> v1 -> v2:
>>> * show number of HighTotal before hotremove
>>> * remove CONFIG_HIGHMEM
>>> * cc stable kernels
>>> * add Michal reviewed-by
>>>
>>> Logic memory-remove code fails to correctly account the Total High Memory
>>> when a memory block which contains High Memory is offlined as shown in the
>>> example below. The following patch fixes it.
>>>
>>> Stable for 2.6.24+.
>>>
>>> Before logic memory remove:
>>>
>>> MemTotal: 7603740 kB
>>> MemFree: 6329612 kB
>>> Buffers: 94352 kB
>>> Cached: 872008 kB
>>> SwapCached: 0 kB
>>> Active: 626932 kB
>>> Inactive: 519216 kB
>>> Active(anon): 180776 kB
>>> Inactive(anon): 222944 kB
>>> Active(file): 446156 kB
>>> Inactive(file): 296272 kB
>>> Unevictable: 0 kB
>>> Mlocked: 0 kB
>>> HighTotal: 7294672 kB
>>> HighFree: 5704696 kB
>>> LowTotal: 309068 kB
>>> LowFree: 624916 kB
>>>
>>> After logic memory remove:
>>>
>>> MemTotal: 7079452 kB
>>> MemFree: 5805976 kB
>>> Buffers: 94372 kB
>>> Cached: 872000 kB
>>> SwapCached: 0 kB
>>> Active: 626936 kB
>>> Inactive: 519236 kB
>>> Active(anon): 180780 kB
>>> Inactive(anon): 222944 kB
>>> Active(file): 446156 kB
>>> Inactive(file): 296292 kB
>>> Unevictable: 0 kB
>>> Mlocked: 0 kB
>>> HighTotal: 7294672 kB
>>> HighFree: 5181024 kB
>>> LowTotal: 4294752076 kB
>>> LowFree: 624952 kB
>>>
>>> Reviewed-by: Michal Hocko <[email protected]>
>>> Signed-off-by: Wanpeng Li <[email protected]>
>>> ---
>>> mm/page_alloc.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>>> index 98cbdf6..23b921f 100644
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -6140,6 +6140,8 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
>>> list_del(&page->lru);
>>> rmv_page_order(page);
>>> zone->free_area[order].nr_free--;
>>> + if (PageHighMem(page))
>>> + totalhigh_pages -= 1 << order;
>>> for (i = 0; i < (1 << order); i++)
>>> SetPageReserved((page+i));
>>> pfn += (1 << order);
>>
>> Hm. I already NAKed and you didn't answered my question. isn't it?
>
> Jiang makes his effort to support highmem for memory hotremove, he also
> fix this bug, http://marc.info/?l=linux-mm&m=136957578620221&w=2

OK, go ahead.




2013-05-27 06:48:32

by Michal Hocko

[permalink] [raw]
Subject: Re: [patch v2 3/6] mm/memory_hotplug: Disable memory hotremove for 32bit

On Mon 27-05-13 07:51:38, Wanpeng Li wrote:
> On Sun, May 26, 2013 at 08:12:09PM +0200, Michal Hocko wrote:
> >On Sun 26-05-13 17:06:17, Wanpeng Li wrote:
> >> On Sun, May 26, 2013 at 11:00:54AM +0200, Michal Hocko wrote:
> >> >On Sun 26-05-13 13:58:38, Wanpeng Li wrote:
> >> >> As KOSAKI Motohiro mentioned, memory hotplug don't support 32bit since
> >> >> it was born,
> >> >
> >> >Why? any reference? This reasoning is really weak.
> >> >
> >>
> >> http://marc.info/?l=linux-mm&m=136953099010171&w=2
> >
> >I have seen the email but that email just states that the feature is
> >broken. Maybe it is obvious to you _what_ is actually broken but it
> >doesn't need to be others especially those who would be reading such
> >changelog later. So if you consider this configuration broken then be
> >specific what is broken.
> >
>
> Sorry for the not enough information. KOSAKI explain more here:
> http://marc.info/?l=linux-mm&m=136958040921274&w=2

There are still just general claims that _something_ is not highmem
aware.

Anyway, as it seems that there are some attempts to revive this code
then this discussion is moot. But, just for the future, make sure you
are really specific when you claim that something is broken. Somebody
said it was broken is _not_ a reasoning that would fly.

--
Michal Hocko
SUSE Labs

2013-05-28 03:38:09

by Ben Hutchings

[permalink] [raw]
Subject: Re: [PATCH v3 1/6] mm/memory-hotplug: fix lowmem count overflow when offline pages

On Sun, 2013-05-26 at 13:58 +0800, Wanpeng Li wrote:
> Changelog:
> v1 -> v2:
> * show number of HighTotal before hotremove
> * remove CONFIG_HIGHMEM
> * cc stable kernels
> * add Michal reviewed-by
>
> Logic memory-remove code fails to correctly account the Total High Memory
> when a memory block which contains High Memory is offlined as shown in the
> example below. The following patch fixes it.
>
> Stable for 2.6.24+.
[...]
> Reviewed-by: Michal Hocko <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
[...]

This is not the correct way to request changes for stable. See
Documentation/stable_kernel_rules.txt

Ben.

--
Ben Hutchings
If at first you don't succeed, you're doing about average.


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part