2011-05-02 21:19:32

by Daniel Kiper

[permalink] [raw]
Subject: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
It means that code depending on CONFIG_FLATMEM in online_page()
is never compiled. Remove it because it is not needed anymore.

Signed-off-by: Daniel Kiper <[email protected]>
---
mm/memory_hotplug.c | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 9ca1d60..a807ccb 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -374,10 +374,6 @@ void online_page(struct page *page)
totalhigh_pages++;
#endif

-#ifdef CONFIG_FLATMEM
- max_mapnr = max(pfn, max_mapnr);
-#endif
-
ClearPageReserved(page);
init_page_count(page);
__free_page(page);
--
1.5.6.5


2011-05-02 21:25:13

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Mon, 2011-05-02 at 23:19 +0200, Daniel Kiper wrote:
> Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
> It means that code depending on CONFIG_FLATMEM in online_page()
> is never compiled. Remove it because it is not needed anymore.

It's subtle, but I don't think that's true. We had another hotplug mode
for x86_64 before folks were comfortable turning SPARSEMEM on for the
whole architecture. It was quite possible to have memory hotplug
without sparsemem in that case. I think Keith Mannthey did some of that
code if I remember right.

But, I'm not sure how much of that stayed in distros versus made it
upstream. In any case, you might want to chase down the
X86_64_ACPI_NUMA bit to make sure it can't be used with FLATMEM ever.

config MEMORY_HOTPLUG
bool "Allow for memory hot-add"
depends on SPARSEMEM || X86_64_ACPI_NUMA
depends on HOTPLUG && ARCH_ENABLE_MEMORY_HOTPLUG
depends on (IA64 || X86 || PPC_BOOK3S_64 || SUPERH || S390)

-- Dave

2011-05-02 22:20:23

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Mon, May 02, 2011 at 02:25:04PM -0700, Dave Hansen wrote:
> On Mon, 2011-05-02 at 23:19 +0200, Daniel Kiper wrote:
> > Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
> > It means that code depending on CONFIG_FLATMEM in online_page()
> > is never compiled. Remove it because it is not needed anymore.
>
> It's subtle, but I don't think that's true. We had another hotplug mode
> for x86_64 before folks were comfortable turning SPARSEMEM on for the
> whole architecture. It was quite possible to have memory hotplug
> without sparsemem in that case. I think Keith Mannthey did some of that
> code if I remember right.
>
> But, I'm not sure how much of that stayed in distros versus made it
> upstream. In any case, you might want to chase down the
> X86_64_ACPI_NUMA bit to make sure it can't be used with FLATMEM ever.
>
> config MEMORY_HOTPLUG
> bool "Allow for memory hot-add"
> depends on SPARSEMEM || X86_64_ACPI_NUMA
> depends on HOTPLUG && ARCH_ENABLE_MEMORY_HOTPLUG
> depends on (IA64 || X86 || PPC_BOOK3S_64 || SUPERH || S390)

IIRC some time ago it was possible to enable memory hotplug with
CONFIG_FLATMEM. That is why I looked for any dependencies of memory
hoplug code on CONFIG_FLATMEM in current Linux Kernel source. I could
not find anything and that is why I published this patch. However,
maybe I missed something.

Daniel

2011-05-02 22:48:57

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Tue, 2011-05-03 at 00:18 +0200, Daniel Kiper wrote:
>
> > config MEMORY_HOTPLUG
> > bool "Allow for memory hot-add"
> > depends on SPARSEMEM || X86_64_ACPI_NUMA
> > depends on HOTPLUG && ARCH_ENABLE_MEMORY_HOTPLUG
> > depends on (IA64 || X86 || PPC_BOOK3S_64 || SUPERH || S390)
>
> IIRC some time ago it was possible to enable memory hotplug with
> CONFIG_FLATMEM. That is why I looked for any dependencies of memory
> hoplug code on CONFIG_FLATMEM in current Linux Kernel source. I could
> not find anything and that is why I published this patch. However,
> maybe I missed something.

I can't find any immediately apparent case, either. Guess that's what
LKML is for. :)

Acked-by: Dave Hansen <[email protected]>

-- Dave

2011-05-11 22:48:05

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Mon, 2 May 2011, Daniel Kiper wrote:

> Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
> It means that code depending on CONFIG_FLATMEM in online_page()
> is never compiled. Remove it because it is not needed anymore.
>
> Signed-off-by: Daniel Kiper <[email protected]>

The code you're patching depends on CONFIG_MEMORY_HOTPLUG_SPARSE, so this
is valid. The changelog should be updated to reflect that, however.

Acked-by: David Rientjes <[email protected]>

2011-05-12 10:26:58

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Wed, May 11, 2011 at 03:47:49PM -0700, David Rientjes wrote:
> On Mon, 2 May 2011, Daniel Kiper wrote:
>
> > Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
> > It means that code depending on CONFIG_FLATMEM in online_page()
> > is never compiled. Remove it because it is not needed anymore.
> >
> > Signed-off-by: Daniel Kiper <[email protected]>
>
> The code you're patching depends on CONFIG_MEMORY_HOTPLUG_SPARSE, so this
> is valid. The changelog should be updated to reflect that, however.
>
> Acked-by: David Rientjes <[email protected]>

No problem, however, this bundle of patches was added to the -mm tree.
In this situation should I repost whole bundle with relevant changes
or post only those two patches requested by you ??? For which tree
should I prepare new version of patches ???

Daniel

2011-05-12 19:26:37

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Thu, 12 May 2011, Daniel Kiper wrote:

> > > Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
> > > It means that code depending on CONFIG_FLATMEM in online_page()
> > > is never compiled. Remove it because it is not needed anymore.
> > >
> > > Signed-off-by: Daniel Kiper <[email protected]>
> >
> > The code you're patching depends on CONFIG_MEMORY_HOTPLUG_SPARSE, so this
> > is valid. The changelog should be updated to reflect that, however.
> >
> > Acked-by: David Rientjes <[email protected]>
>
> No problem, however, this bundle of patches was added to the -mm tree.
> In this situation should I repost whole bundle with relevant changes
> or post only those two patches requested by you ??? For which tree
> should I prepare new version of patches ???
>

No, I would just reply to the email notification you received when the
patch went into -mm saying that the changelog should be adjusted to read
something like

online_pages() is only compiled for CONFIG_MEMORY_HOTPLUG_SPARSE,
so there is no need to support CONFIG_FLATMEM code within it.

This patch removes code that is never used.

2011-05-16 08:00:42

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Thu, May 12, 2011 at 12:26:22PM -0700, David Rientjes wrote:
> On Thu, 12 May 2011, Daniel Kiper wrote:
>
> > > > Memory hotplug code strictly depends on CONFIG_SPARSEMEM.
> > > > It means that code depending on CONFIG_FLATMEM in online_page()
> > > > is never compiled. Remove it because it is not needed anymore.
> > > >
> > > > Signed-off-by: Daniel Kiper <[email protected]>
> > >
> > > The code you're patching depends on CONFIG_MEMORY_HOTPLUG_SPARSE, so this
> > > is valid. The changelog should be updated to reflect that, however.
> > >
> > > Acked-by: David Rientjes <[email protected]>
> >
> > No problem, however, this bundle of patches was added to the -mm tree.
> > In this situation should I repost whole bundle with relevant changes
> > or post only those two patches requested by you ??? For which tree
> > should I prepare new version of patches ???
> >
>
> No, I would just reply to the email notification you received when the
> patch went into -mm saying that the changelog should be adjusted to read
> something like
>
> online_pages() is only compiled for CONFIG_MEMORY_HOTPLUG_SPARSE,
> so there is no need to support CONFIG_FLATMEM code within it.
>
> This patch removes code that is never used.

Please look into attachments.

If you have any questions please drop me a line.

Daniel


Attachments:
(No filename) (1.29 kB)
mm-enable-set_page_section-only-if-config_sparsemem-and-config_sparsemem_vmemmap.patch (4.94 kB)
mm-remove-dependency-on-config_flatmem-from-online_page.patch (3.92 kB)
Download all attachments

2011-05-16 20:32:27

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Mon, 16 May 2011, Daniel Kiper wrote:

> > No, I would just reply to the email notification you received when the
> > patch went into -mm saying that the changelog should be adjusted to read
> > something like
> >
> > online_pages() is only compiled for CONFIG_MEMORY_HOTPLUG_SPARSE,
> > so there is no need to support CONFIG_FLATMEM code within it.
> >
> > This patch removes code that is never used.
>
> Please look into attachments.
>
> If you have any questions please drop me a line.
>

Not sure why you've attached the emails from the mm-commits mailing list.
I'll respond to the commits with with my suggestions for how the changelog
should be fixed.

Thanks.

2011-05-17 08:49:38

by Daniel Kiper

[permalink] [raw]
Subject: Re: [PATCH 1/4] mm: Remove dependency on CONFIG_FLATMEM from online_page()

On Mon, May 16, 2011 at 01:32:19PM -0700, David Rientjes wrote:
> On Mon, 16 May 2011, Daniel Kiper wrote:
>
> > > No, I would just reply to the email notification you received when the
> > > patch went into -mm saying that the changelog should be adjusted to read
> > > something like
> > >
> > > online_pages() is only compiled for CONFIG_MEMORY_HOTPLUG_SPARSE,
> > > so there is no need to support CONFIG_FLATMEM code within it.
> > >
> > > This patch removes code that is never used.
> >
> > Please look into attachments.
> >
> > If you have any questions please drop me a line.
>
> Not sure why you've attached the emails from the mm-commits mailing list.

I attached emails from the mm-commits mailing list because
I understood that you need them to correct changelogs.

> I'll respond to the commits with with my suggestions for how the changelog
> should be fixed.

I saw your replies. Thank you for your help.

Daniel