[PATCH -next] mm/hotplug: silence a lockdep splat with printk()

* [PATCH -next] mm/hotplug: silence a lockdep splat with printk()
@ 2020-01-14 20:11 Qian Cai
  2020-01-14 20:20 ` David Hildenbrand
  0 siblings, 1 reply; 14+ messages in thread
From: Qian Cai @ 2020-01-14 20:11 UTC (permalink / raw)
  To: akpm
  Cc: mhocko, sergey.senozhatsky.work, pmladek, rostedt, peterz, david,
	linux-mm, linux-kernel, Qian Cai

Similar to the recent commit [1] merged into the random and -next trees,
it is not a good idea to call printk() with zone->lock held. The
standard fix is to use printk_deferred() in those places, but memory
offline will call dump_page() which need to defer after the lock. While
at it, remove a similar but unnecessary debug printk() as well.

[1] https://lore.kernel.org/lkml/1573679785-21068-1-git-send-email-cai@lca.pw/

Signed-off-by: Qian Cai <cai@lca.pw>
---
 include/linux/page-isolation.h |  2 +-
 mm/memory_hotplug.c            |  2 +-
 mm/page_alloc.c                | 12 +++++-------
 mm/page_isolation.c            | 10 +++++++++-
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/linux/page-isolation.h b/include/linux/page-isolation.h
index 148e65a9c606..5d8ba078006f 100644
--- a/include/linux/page-isolation.h
+++ b/include/linux/page-isolation.h
@@ -34,7 +34,7 @@ static inline bool is_migrate_isolate(int migratetype)
 #define REPORT_FAILURE	0x2
 
 bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
-			 int flags);
+			 int flags, char *dump);
 void set_pageblock_migratetype(struct page *page, int migratetype);
 int move_freepages_block(struct zone *zone, struct page *page,
 				int migratetype, int *num_movable);
diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
index 7a6de9b0dcab..f10928538fa3 100644
--- a/mm/memory_hotplug.c
+++ b/mm/memory_hotplug.c
@@ -1149,7 +1149,7 @@ static bool is_pageblock_removable_nolock(unsigned long pfn)
 		return false;
 
 	return !has_unmovable_pages(zone, page, MIGRATE_MOVABLE,
-				    MEMORY_OFFLINE);
+				    MEMORY_OFFLINE, NULL);
 }
 
 /* Checks if this range of memory is likely to be hot-removable. */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e56cd1f33242..b6bec3925e80 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -8204,7 +8204,7 @@ void *__init alloc_large_system_hash(const char *tablename,
  * race condition. So you can't expect this function should be exact.
  */
 bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
-			 int flags)
+			 int flags, char *dump)
 {
 	unsigned long iter = 0;
 	unsigned long pfn = page_to_pfn(page);
@@ -8305,8 +8305,10 @@ bool has_unmovable_pages(struct zone *zone, struct page *page, int migratetype,
 	return false;
 unmovable:
 	WARN_ON_ONCE(zone_idx(zone) == ZONE_MOVABLE);
-	if (flags & REPORT_FAILURE)
-		dump_page(pfn_to_page(pfn + iter), reason);
+	if (flags & REPORT_FAILURE) {
+		page = pfn_to_page(pfn + iter);
+		strscpy(dump, reason, 64);
+	}
 	return true;
 }
 
@@ -8711,10 +8713,6 @@ __offline_isolated_pages(unsigned long start_pfn, unsigned long end_pfn)
 		BUG_ON(!PageBuddy(page));
 		order = page_order(page);
 		offlined_pages += 1 << order;
-#ifdef CONFIG_DEBUG_VM
-		pr_info("remove from free list %lx %d %lx\n",
-			pfn, 1 << order, end_pfn);
-#endif
 		del_page_from_free_area(page, &zone->free_area[order]);
 		pfn += (1 << order);
 	}
diff --git a/mm/page_isolation.c b/mm/page_isolation.c
index 1f8b9dfecbe8..ce0fe3c1ceff 100644
--- a/mm/page_isolation.c
+++ b/mm/page_isolation.c
@@ -20,6 +20,7 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	struct zone *zone;
 	unsigned long flags;
 	int ret = -EBUSY;
+	char dump[64];
 
 	zone = page_zone(page);
 
@@ -37,7 +38,8 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	 * FIXME: Now, memory hotplug doesn't call shrink_slab() by itself.
 	 * We just check MOVABLE pages.
 	 */
-	if (!has_unmovable_pages(zone, page, migratetype, isol_flags)) {
+	if (!has_unmovable_pages(zone, page, migratetype, isol_flags,
+				 dump)) {
 		unsigned long nr_pages;
 		int mt = get_pageblock_migratetype(page);
 
@@ -54,6 +56,12 @@ static int set_migratetype_isolate(struct page *page, int migratetype, int isol_
 	spin_unlock_irqrestore(&zone->lock, flags);
 	if (!ret)
 		drain_all_pages(zone);
+	else if (isol_flags & REPORT_FAILURE)
+		/*
+		 * printk() with zone->lock held will guarantee to trigger a
+		 * lockdep splat, so defer it here.
+		 */
+		dump_page(page, dump);
 	return ret;
 }
 
-- 
2.21.0 (Apple Git-122.2)


^ permalink raw reply related	[flat|nested] 14+ messages in thread