Thread (12 messages) 12 messages, 5 authors, 2012-10-19

Re: [PATCH v2 2/5] memory-hotplug: update mce_bad_pages when removing the memory

From: Dave Hansen <hidden>
Date: 2012-10-17 15:12:10
Also in: lkml

Hi Wen,
+#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.
quoted hunk ↗ jump to hunk
+	for (i = 0; i < PAGES_PER_SECTION; i++) {
+		if (PageHWPoison(&memmap[i])) {
+			atomic_long_sub(1, &mce_bad_pages);
+			ClearPageHWPoison(&memmap[i]);
+		}
+	}
+}
+#endif
+
 void sparse_remove_one_section(struct zone *zone, struct mem_section *ms)
 {
 	struct page *memmap = NULL;
@@ -786,6 +803,10 @@ void sparse_remove_one_section(struct zone *zone, struct mem_section *ms)
 		ms->pageblock_flags = NULL;
 	}

+#ifdef CONFIG_MEMORY_FAILURE
+	clear_hwpoisoned_pages(memmap, PAGES_PER_SECTION);
+#endif
+
 	free_section_usemap(memmap, usemap);
 }
 #endif
But why put the call outside the  "if (ms->section_mem_map)" block?  If
you put it inside, then you don't have to check for !memmap in
clear_hwpoisoned_pages().

Also, we really frown on #ifdefs scattered throughout code.  I'd suggest
either:

+static void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
+{
+#ifdef CONFIG_MEMORY_FAILURE
... existing code
+#endif /* CONFIG_MEMORY_FAILURE */
+}

or

+#ifdef CONFIG_MEMORY_FAILURE
+static void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
+{
... existing code
+}
+#else
+static void clear_hwpoisoned_pages(struct page *memmap, int nr_pages)
+{}
+#endif /* CONFIG_MEMORY_FAILURE */

and keep the #ifdef out of sparse_remove_one_section().

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help