lkml.org 
[lkml]   [2011]   [Oct]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] mm: memory hotplug: Check if pages are correctly reserved on a per-section basis
    On 10/11/2011 02:27 AM, Mel Gorman wrote:
    > On Mon, Oct 10, 2011 at 04:28:13PM -0700, Andrew Morton wrote:
    >> On Mon, 10 Oct 2011 16:24:03 -0700
    >> Greg KH <greg@kroah.com> wrote:
    >>
    >>> On Mon, Oct 10, 2011 at 03:00:38PM -0700, Andrew Morton wrote:
    >>>> On Mon, 10 Oct 2011 08:11:19 +0100
    >>>> Mel Gorman <mgorman@suse.de> wrote:
    >>>>
    >>>>> It is expected that memory being brought online is PageReserved
    >>>>> similar to what happens when the page allocator is being brought up.
    >>>>> Memory is onlined in "memory blocks" which consist of one or more
    >>>>> sections. Unfortunately, the code that verifies PageReserved is
    >>>>> currently assuming that the memmap backing all these pages is virtually
    >>>>> contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set.
    >>>>> As a result, memory hot-add is failing on !VMEMMAP configurations
    >>>>> with the message;
    >>>>>
    >>>>> kernel: section number XXX page number 256 not reserved, was it already online?
    >>>>>
    >>>>> This patch updates the PageReserved check to lookup struct page once
    >>>>> per section to guarantee the correct struct page is being checked.
    >>>>>
    >>>>
    >>>> Nathan's earlier version of this patch is already in linux-next, via
    >>>> Greg. We should drop the old version and get the new one merged
    >>>> instead.
    >>>
    >>> Ok, care to send me what exactly needs to be reverted and what needs to
    >>> be added?
    >>
    >> Drop
    >>
    >> commit 54f23eb7ba7619de85d8edca6e5336bc33072dbd
    >> Author: Nathan Fontenot <nfont@austin.ibm.com>
    >> Date: Mon Sep 26 10:22:33 2011 -0500
    >>
    >> memory hotplug: Correct page reservation checking
    >>
    >> and replace it with start-of-this-thread.
    >>
    >> That's assuming that Mel's update passes Nathan's review and testing :)
    >
    > It passed review and testing with IBM based on a SUSE bug. I thought
    > Nathan's patch had been lost as it was posted to linuxppc-dev instead
    > of linux-mm. This rework was to improve the changelog and readability.
    >
    > David correctly pointed out a bug that passed testing because it was
    > still checking one page per section. As long as that page was reserved,
    > memory hot-add would go ahead. Here is a corrected version.
    >

    Previous patch passed testing, working on testing Mel's updated patch...

    -Nathan

    > Thanks
    >
    > ==== CUT HERE ====
    > mm: memory hotplug: Check if pages are correctly reserved on a per-section basis
    >
    > It is expected that memory being brought online is PageReserved
    > similar to what happens when the page allocator is being brought up.
    > Memory is onlined in "memory blocks" which consist of one or more
    > sections. Unfortunately, the code that verifies PageReserved is
    > currently assuming that the memmap backing all these pages is virtually
    > contiguous which is only the case when CONFIG_SPARSEMEM_VMEMMAP is set.
    > As a result, memory hot-add is failing on those configurations with
    > the message;
    >
    > kernel: section number XXX page number 256 not reserved, was it already online?
    >
    > This patch updates the PageReserved check to lookup struct page once
    > per section to guarantee the correct struct page is being checked.
    >
    > [Check pages within sections properly: rientjes@google.com]
    > [original patch by: nfont@linux.vnet.ibm.com]
    > Signed-off-by: Mel Gorman <mgorman@suse.de>
    > ---
    > drivers/base/memory.c | 58 +++++++++++++++++++++++++++++++++---------------
    > 1 files changed, 40 insertions(+), 18 deletions(-)
    >
    > diff --git a/drivers/base/memory.c b/drivers/base/memory.c
    > index 2840ed4..ffb69cd 100644
    > --- a/drivers/base/memory.c
    > +++ b/drivers/base/memory.c
    > @@ -224,13 +224,48 @@ int memory_isolate_notify(unsigned long val, void *v)
    > }
    >
    > /*
    > + * The probe routines leave the pages reserved, just as the bootmem code does.
    > + * Make sure they're still that way.
    > + */
    > +static bool pages_correctly_reserved(unsigned long start_pfn,
    > + unsigned long nr_pages)
    > +{
    > + int i, j;
    > + struct page *page;
    > + unsigned long pfn = start_pfn;
    > +
    > + /*
    > + * memmap between sections is not contiguous except with
    > + * SPARSEMEM_VMEMMAP. We lookup the page once per section
    > + * and assume memmap is contiguous within each section
    > + */
    > + for (i = 0; i < sections_per_block; i++, pfn += PAGES_PER_SECTION) {
    > + if (WARN_ON_ONCE(!pfn_valid(pfn)))
    > + return false;
    > + page = pfn_to_page(pfn);
    > +
    > + for (j = 0; j < PAGES_PER_SECTION; j++) {
    > + if (PageReserved(page + j))
    > + continue;
    > +
    > + printk(KERN_WARNING "section number %ld page number %d "
    > + "not reserved, was it already online?\n",
    > + pfn_to_section_nr(pfn), j);
    > +
    > + return false;
    > + }
    > + }
    > +
    > + return true;
    > +}
    > +
    > +/*
    > * MEMORY_HOTPLUG depends on SPARSEMEM in mm/Kconfig, so it is
    > * OK to have direct references to sparsemem variables in here.
    > */
    > static int
    > memory_block_action(unsigned long phys_index, unsigned long action)
    > {
    > - int i;
    > unsigned long start_pfn, start_paddr;
    > unsigned long nr_pages = PAGES_PER_SECTION * sections_per_block;
    > struct page *first_page;
    > @@ -238,26 +273,13 @@ memory_block_action(unsigned long phys_index, unsigned long action)
    >
    > first_page = pfn_to_page(phys_index << PFN_SECTION_SHIFT);
    >
    > - /*
    > - * The probe routines leave the pages reserved, just
    > - * as the bootmem code does. Make sure they're still
    > - * that way.
    > - */
    > - if (action == MEM_ONLINE) {
    > - for (i = 0; i < nr_pages; i++) {
    > - if (PageReserved(first_page+i))
    > - continue;
    > -
    > - printk(KERN_WARNING "section number %ld page number %d "
    > - "not reserved, was it already online?\n",
    > - phys_index, i);
    > - return -EBUSY;
    > - }
    > - }
    > -
    > switch (action) {
    > case MEM_ONLINE:
    > start_pfn = page_to_pfn(first_page);
    > +
    > + if (!pages_correctly_reserved(start_pfn, nr_pages))
    > + return -EBUSY;
    > +
    > ret = online_pages(start_pfn, nr_pages);
    > break;
    > case MEM_OFFLINE:
    >



    \
     
     \ /
      Last update: 2011-10-11 16:47    [W:0.037 / U:0.676 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site