[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20080616104434.GG17016@shadowen.org>
Date: Mon, 16 Jun 2008 11:44:34 +0100
From: Andy Whitcroft <apw@...dowen.org>
To: Yasunori Goto <y-goto@...fujitsu.com>
Cc: Badari Pulavarty <pbadari@...ibm.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Linux Kernel ML <linux-kernel@...r.kernel.org>,
linux-mm <linux-mm@...ck.org>, Yinghai Lu <yhlu.kernel@...il.com>
Subject: Re: [Patch 005/005](memory hotplug) free memmaps allocated by bootmem
On Mon, Apr 07, 2008 at 09:50:18PM +0900, Yasunori Goto wrote:
> This patch is to free memmaps which is allocated by bootmem.
>
> Freeing usemap is not necessary. The pages of usemap may be necessary
> for other sections.
>
> If removing section is last section on the node,
> its section is the final user of usemap page.
> (usemaps are allocated on its section by previous patch.)
> But it shouldn't be freed too, because the section must be
> logical offline state which all pages are isolated against page allocater.
> If it is freed, page alloctor may use it which will be removed
> physically soon. It will be disaster.
> So, this patch keeps it as it is.
>
>
> Signed-off-by: Yasunori Goto <y-goto@...fujitsu.com>
>
> ---
> mm/internal.h | 3 +--
> mm/memory_hotplug.c | 11 +++++++++++
> mm/page_alloc.c | 2 +-
> mm/sparse.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
> 4 files changed, 60 insertions(+), 7 deletions(-)
>
> Index: current/mm/sparse.c
> ===================================================================
> --- current.orig/mm/sparse.c 2008-04-07 20:13:25.000000000 +0900
> +++ current/mm/sparse.c 2008-04-07 20:27:20.000000000 +0900
> @@ -8,6 +8,7 @@
> #include <linux/module.h>
> #include <linux/spinlock.h>
> #include <linux/vmalloc.h>
> +#include "internal.h"
> #include <asm/dma.h>
> #include <asm/pgalloc.h>
> #include <asm/pgtable.h>
> @@ -360,6 +361,9 @@
> {
> return; /* XXX: Not implemented yet */
> }
> +static void free_map_bootmem(struct page *page, unsigned long nr_pages)
> +{
> +}
> #else
> static struct page *__kmalloc_section_memmap(unsigned long nr_pages)
> {
> @@ -397,17 +401,47 @@
> free_pages((unsigned long)memmap,
> get_order(sizeof(struct page) * nr_pages));
> }
> +
> +static void free_map_bootmem(struct page *page, unsigned long nr_pages)
> +{
> + unsigned long maps_section_nr, removing_section_nr, i;
> + int magic;
> +
> + for (i = 0; i < nr_pages; i++, page++) {
> + magic = atomic_read(&page->_mapcount);
> +
> + BUG_ON(magic == NODE_INFO);
Are we sure the node area was big enough to never allocate usemap's into
it and change the magic to MIX? I saw you make the section information
page sized but not the others.
> +
> + maps_section_nr = pfn_to_section_nr(page_to_pfn(page));
> + removing_section_nr = page->private;
> +
> + /*
> + * When this function is called, the removing section is
> + * logical offlined state. This means all pages are isolated
> + * from page allocator. If removing section's memmap is placed
> + * on the same section, it must not be freed.
> + * If it is freed, page allocator may allocate it which will
> + * be removed physically soon.
> + */
> + if (maps_section_nr != removing_section_nr)
> + put_page_bootmem(page);
Would the section memmap have its own get_page_bootmem reference here?
Would that not protect it from release?
> + }
> +}
> #endif /* CONFIG_SPARSEMEM_VMEMMAP */
>
> static void free_section_usemap(struct page *memmap, unsigned long *usemap)
> {
> + struct page *usemap_page;
> + unsigned long nr_pages;
> +
> if (!usemap)
> return;
>
> + usemap_page = virt_to_page(usemap);
> /*
> * Check to see if allocation came from hot-plug-add
> */
> - if (PageSlab(virt_to_page(usemap))) {
> + if (PageSlab(usemap_page)) {
> kfree(usemap);
> if (memmap)
> __kfree_section_memmap(memmap, PAGES_PER_SECTION);
> @@ -415,10 +449,19 @@
> }
>
> /*
> - * TODO: Allocations came from bootmem - how do I free up ?
> + * The usemap came from bootmem. This is packed with other usemaps
> + * on the section which has pgdat at boot time. Just keep it as is now.
> */
> - printk(KERN_WARNING "Not freeing up allocations from bootmem "
> - "- leaking memory\n");
> +
> + if (memmap) {
> + struct page *memmap_page;
> + memmap_page = virt_to_page(memmap);
> +
> + nr_pages = PAGE_ALIGN(PAGES_PER_SECTION * sizeof(struct page))
> + >> PAGE_SHIFT;
> +
> + free_map_bootmem(memmap_page, nr_pages);
> + }
> }
>
> /*
> Index: current/mm/page_alloc.c
> ===================================================================
> --- current.orig/mm/page_alloc.c 2008-04-07 20:12:55.000000000 +0900
> +++ current/mm/page_alloc.c 2008-04-07 20:13:29.000000000 +0900
> @@ -568,7 +568,7 @@
> /*
> * permit the bootmem allocator to evade page validation on high-order frees
> */
> -void __init __free_pages_bootmem(struct page *page, unsigned int order)
> +void __free_pages_bootmem(struct page *page, unsigned int order)
not __meminit or something?
> {
> if (order == 0) {
> __ClearPageReserved(page);
> Index: current/mm/internal.h
> ===================================================================
> --- current.orig/mm/internal.h 2008-04-07 20:12:55.000000000 +0900
> +++ current/mm/internal.h 2008-04-07 20:13:29.000000000 +0900
> @@ -34,8 +34,7 @@
> atomic_dec(&page->_count);
> }
>
> -extern void __init __free_pages_bootmem(struct page *page,
> - unsigned int order);
> +extern void __free_pages_bootmem(struct page *page, unsigned int order);
>
> /*
> * function for dealing with page's order in buddy system.
> Index: current/mm/memory_hotplug.c
> ===================================================================
> --- current.orig/mm/memory_hotplug.c 2008-04-07 20:12:55.000000000 +0900
> +++ current/mm/memory_hotplug.c 2008-04-07 20:13:29.000000000 +0900
> @@ -199,6 +199,16 @@
> return register_new_memory(__pfn_to_section(phys_start_pfn));
> }
>
> +#ifdef CONFIG_SPARSEMEM_VMEMMAP
> +static int __remove_section(struct zone *zone, struct mem_section *ms)
> +{
> + /*
> + * XXX: Freeing memmap with vmemmap is not implement yet.
> + * This should be removed later.
> + */
> + return -EBUSY;
> +}
> +#else
> static int __remove_section(struct zone *zone, struct mem_section *ms)
> {
> unsigned long flags;
> @@ -217,6 +227,7 @@
> pgdat_resize_unlock(pgdat, &flags);
> return 0;
> }
> +#endif
>
> /*
> * Reasonably generic function for adding memory. It is
-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists