Re: [PATCH] mm: memory hotplug: Check if pages are correctly reserved on a per-section basis

David Rientjes
2011-10-11T04:26:09Z

On Mon, 10 Oct 2011, Andrew Morton wrote:

> 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 + i))

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;