From bb8965bd82fd4ed433a888f1383016ab3fa0d7de Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Fri, 28 Dec 2018 00:38:32 -0800 Subject: [PATCH] mm, memory_hotplug: deobfuscate migration part of offlining Memory migration might fail during offlining and we keep retrying in that case. This is currently obfuscated by goto retry loop. The code is hard to follow and as a result it is even suboptimal becase each retry round scans the full range from start_pfn even though we have successfully scanned/migrated [start_pfn, pfn] range already. This is all only because check_pages_isolated failure has to rescan the full range again. De-obfuscate the migration retry loop by promoting it to a real for loop. In fact remove the goto altogether by making it a proper double loop (yeah, gotos are nasty in this specific case). In the end we will get a slightly more optimal code which is better readable. [akpm@linux-foundation.org: reflow comments to 80 cols] Link: http://lkml.kernel.org/r/20181211142741.2607-3-mhocko@kernel.org Signed-off-by: Michal Hocko Reviewed-by: David Hildenbrand Reviewed-by: Oscar Salvador Cc: Hugh Dickins Cc: Jan Kara Cc: Kirill A. Shutemov Cc: Pavel Tatashin Cc: William Kucharski Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- mm/memory_hotplug.c | 62 ++++++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 2d589d9f9f9e..b9a667d36c55 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1607,38 +1607,42 @@ static int __ref __offline_pages(unsigned long start_pfn, goto failed_removal_isolated; } - pfn = start_pfn; -repeat: - /* start memory hot removal */ - ret = -EINTR; - if (signal_pending(current)) { - reason = "signal backoff"; - goto failed_removal_isolated; - } + do { + for (pfn = start_pfn; pfn;) { + if (signal_pending(current)) { + ret = -EINTR; + reason = "signal backoff"; + goto failed_removal_isolated; + } - cond_resched(); - lru_add_drain_all(); - drain_all_pages(zone); + cond_resched(); + lru_add_drain_all(); + drain_all_pages(zone); + + pfn = scan_movable_pages(pfn, end_pfn); + if (pfn) { + /* + * TODO: fatal migration failures should bail + * out + */ + do_migrate_range(pfn, end_pfn); + } + } - pfn = scan_movable_pages(start_pfn, end_pfn); - if (pfn) { /* We have movable pages */ - ret = do_migrate_range(pfn, end_pfn); - goto repeat; - } + /* + * Dissolve free hugepages in the memory block before doing + * offlining actually in order to make hugetlbfs's object + * counting consistent. + */ + ret = dissolve_free_huge_pages(start_pfn, end_pfn); + if (ret) { + reason = "failure to dissolve huge pages"; + goto failed_removal_isolated; + } + /* check again */ + offlined_pages = check_pages_isolated(start_pfn, end_pfn); + } while (offlined_pages < 0); - /* - * dissolve free hugepages in the memory block before doing offlining - * actually in order to make hugetlbfs's object counting consistent. - */ - ret = dissolve_free_huge_pages(start_pfn, end_pfn); - if (ret) { - reason = "failure to dissolve huge pages"; - goto failed_removal_isolated; - } - /* check again */ - offlined_pages = check_pages_isolated(start_pfn, end_pfn); - if (offlined_pages < 0) - goto repeat; pr_info("Offlined Pages %ld\n", offlined_pages); /* Ok, all of our target is isolated. We cannot do rollback at this point. */ -- 2.30.2