Messages in this thread | | | Date | Fri, 19 Oct 2012 06:42:46 -0700 | From | Dave Hansen <> | Subject | Re: [PATCH v2 2/5] memory-hotplug: update mce_bad_pages when removing the memory |
| |
On 10/18/2012 03:20 PM, Andrew Morton wrote: > On Wed, 17 Oct 2012 08:09:55 -0700 > Dave Hansen <dave@linux.vnet.ibm.com> wrote: >>> +#ifdef CONFIG_MEMORY_FAILURE >>> +static void clear_hwpoisoned_pages(struct page *memmap, int nr_pages) >>> +{ >>> + int i; >>> + >>> + if (!memmap) >>> + return; >> >> I guess free_section_usemap() does the same thing. > > What does this observation mean?
sparse_remove_one_section() has an if(ms->section_mem_map) statement. Inside that if() block is the only place in the function where 'memmap' can get set.
Currently, sparse_remove_one_section() calls in to free_section_usemap() ouside of that if() block. With this patch new call to clear_hwpoisoned_pages() is done in the same place, both passing 'memmap'.
However, both free_section_usemap() and clear_hwpoisoned_pages() check 'memmap' for NULL and immediately return if so. That's a bit silly since it could hide garbage coming back from sparse_decode_mem_map(). Seems like we should just call them both inside that if() block, or reorganize sparse_remove_one_section(), maybe like this:
void sparse_remove_one_section(struct zone *zone, struct mem_section *ms) { struct page *memmap = NULL; unsigned long *usemap = NULL;
if (!ms->section_mem_map) return;
usemap = ms->pageblock_flags; memmap = sparse_decode_mem_map(ms->section_mem_map, __section_nr(ms)); ms->section_mem_map = 0; ms->pageblock_flags = NULL;
free_section_usemap(memmap, usemap); clear_hwpoisoned_pages(usemap, ...); }
| |