From: Michal Hocko <mhocko@kernel.org> To: <linux-mm@kvack.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Oscar Salvador <OSalvador@suse.com>, Pavel Tatashin <pasha.tatashin@oracle.com>, David Hildenbrand <david@redhat.com>, LKML <linux-kernel@vger.kernel.org>, Michal Hocko <mhocko@suse.com> Subject: [RFC PATCH 2/3] mm, memory_hotplug: deobfuscate migration part of offlining Date: Tue, 20 Nov 2018 14:43:22 +0100 [thread overview] Message-ID: <20181120134323.13007-3-mhocko@kernel.org> (raw) In-Reply-To: <20181120134323.13007-1-mhocko@kernel.org> From: Michal Hocko <mhocko@suse.com> Memory migration might fail during offlining and we keep retrying in that case. This is currently obfuscate 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. Signed-off-by: Michal Hocko <mhocko@suse.com> --- mm/memory_hotplug.c | 60 +++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 6263c8cd4491..9cd161db3061 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1591,38 +1591,40 @@ 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;) + { + /* start memory hot removal */ + ret = -EINTR; + if (signal_pending(current)) { + 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(start_pfn, end_pfn); - if (pfn) { /* We have movable pages */ - ret = do_migrate_range(pfn, end_pfn); - goto repeat; - } + pfn = scan_movable_pages(pfn, end_pfn); + if (pfn) { + /* TODO fatal migration failures should bail out */ + do_migrate_range(pfn, end_pfn); + } + } + + /* + * 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.19.1
WARNING: multiple messages have this Message-ID (diff)
From: Michal Hocko <mhocko@kernel.org> To: linux-mm@kvack.org Cc: Andrew Morton <akpm@linux-foundation.org>, Oscar Salvador <OSalvador@suse.com>, Pavel Tatashin <pasha.tatashin@oracle.com>, David Hildenbrand <david@redhat.com>, LKML <linux-kernel@vger.kernel.org>, Michal Hocko <mhocko@suse.com> Subject: [RFC PATCH 2/3] mm, memory_hotplug: deobfuscate migration part of offlining Date: Tue, 20 Nov 2018 14:43:22 +0100 [thread overview] Message-ID: <20181120134323.13007-3-mhocko@kernel.org> (raw) In-Reply-To: <20181120134323.13007-1-mhocko@kernel.org> From: Michal Hocko <mhocko@suse.com> Memory migration might fail during offlining and we keep retrying in that case. This is currently obfuscate 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. Signed-off-by: Michal Hocko <mhocko@suse.com> --- mm/memory_hotplug.c | 60 +++++++++++++++++++++++---------------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c index 6263c8cd4491..9cd161db3061 100644 --- a/mm/memory_hotplug.c +++ b/mm/memory_hotplug.c @@ -1591,38 +1591,40 @@ 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;) + { + /* start memory hot removal */ + ret = -EINTR; + if (signal_pending(current)) { + 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(start_pfn, end_pfn); - if (pfn) { /* We have movable pages */ - ret = do_migrate_range(pfn, end_pfn); - goto repeat; - } + pfn = scan_movable_pages(pfn, end_pfn); + if (pfn) { + /* TODO fatal migration failures should bail out */ + do_migrate_range(pfn, end_pfn); + } + } + + /* + * 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.19.1
next prev parent reply other threads:[~2018-11-20 13:43 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-20 13:43 [RFC PATCH 0/3] few memory offlining enhancements Michal Hocko 2018-11-20 13:43 ` Michal Hocko 2018-11-20 13:43 ` [RFC PATCH 1/3] mm, memory_hotplug: try to migrate full section worth of pages Michal Hocko 2018-11-20 13:43 ` Michal Hocko 2018-11-20 14:18 ` David Hildenbrand 2018-11-20 14:25 ` Michal Hocko 2018-11-20 14:27 ` David Hildenbrand 2018-11-20 14:33 ` Pavel Tatashin 2018-11-20 14:51 ` osalvador 2018-11-20 15:00 ` Michal Hocko 2018-11-20 13:43 ` Michal Hocko [this message] 2018-11-20 13:43 ` [RFC PATCH 2/3] mm, memory_hotplug: deobfuscate migration part of offlining Michal Hocko 2018-11-20 14:26 ` David Hildenbrand 2018-11-20 14:34 ` Michal Hocko 2018-11-20 14:34 ` David Hildenbrand 2018-11-20 15:13 ` osalvador 2018-11-20 15:18 ` Michal Hocko 2018-11-20 13:43 ` [RFC PATCH 3/3] mm, fault_around: do not take a reference to a locked page Michal Hocko 2018-11-20 13:43 ` Michal Hocko 2018-11-20 14:07 ` Kirill A. Shutemov 2018-11-20 14:12 ` Michal Hocko 2018-11-20 14:17 ` Kirill A. Shutemov 2018-11-20 14:25 ` Michal Hocko 2018-11-21 4:51 ` William Kucharski 2018-11-21 7:07 ` Michal Hocko 2018-11-20 14:33 ` David Hildenbrand 2018-11-21 1:47 ` Hugh Dickins 2018-11-21 7:11 ` Michal Hocko 2018-11-22 2:27 ` Hugh Dickins 2018-11-22 9:05 ` Michal Hocko 2018-11-23 20:22 ` Hugh Dickins
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20181120134323.13007-3-mhocko@kernel.org \ --to=mhocko@kernel.org \ --cc=OSalvador@suse.com \ --cc=akpm@linux-foundation.org \ --cc=david@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=mhocko@suse.com \ --cc=pasha.tatashin@oracle.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.