On 10/18/2012 03:20 PM, Andrew Morton wrote: > On Wed, 17 Oct 2012 08:09:55 -0700 > Dave Hansen <d...@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, ...); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/