mm, memory_hotplug: deobfuscate migration part of offlining
authorMichal Hocko <mhocko@suse.com>
Fri, 28 Dec 2018 08:38:32 +0000 (00:38 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 28 Dec 2018 20:11:50 +0000 (12:11 -0800)
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 <mhocko@suse.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Oscar Salvador <osalvador@suse.de>
Cc: Hugh Dickins <hughd@google.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Cc: William Kucharski <william.kucharski@oracle.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
mm/memory_hotplug.c

index 2d589d9f9f9e6de6b2ba4c2d2e15b906de68414c..b9a667d36c554afc46c2d337a2712e6ca8664fb5 100644 (file)
@@ -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. */