Commit 1723058e authored by Oscar Salvador's avatar Oscar Salvador Committed by Linus Torvalds

mm, memory_hotplug: don't bail out in do_migrate_range() prematurely

do_migrate_range() takes a memory range and tries to isolate the pages
to put them into a list.  This list will be later on used in
migrate_pages() to know the pages we need to migrate.

Currently, if we fail to isolate a single page, we put all already
isolated pages back to their LRU and we bail out from the function.
This is quite suboptimal, as this will force us to start over again
because scan_movable_pages will give us the same range.  If there is no
chance that we can isolate that page, we will loop here forever.

Issue debugged in [1] has proved that.  During the debugging of that
issue, it was noticed that if do_migrate_ranges() fails to isolate a
single page, we will just discard the work we have done so far and bail
out, which means that scan_movable_pages() will find again the same set
of pages.

Instead, we can just skip the error, keep isolating as much pages as
possible and then proceed with the call to migrate_pages().

This will allow us to do as much work as possible at once.

[1] https://lkml.org/lkml/2018/12/6/324

Michal said:

: I still think that this doesn't give us a whole picture.  Looping for
: ever is a bug.  Failing the isolation is quite possible and it should
: be a ephemeral condition (e.g.  a race with freeing the page or
: somebody else isolating the page for whatever reason).  And here comes
: the disadvantage of the current implementation.  We simply throw
: everything on the floor just because of a ephemeral condition.  The
: racy page_count check is quite dubious to prevent from that.

Link: http://lkml.kernel.org/r/20181211135312.27034-1-osalvador@suse.deSigned-off-by: default avatarOscar Salvador <osalvador@suse.de>
Acked-by: default avatarMichal Hocko <mhocko@suse.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Dan Williams <dan.j.williams@gmail.com>
Cc: Jan Kara <jack@suse.cz>
Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Cc: William Kucharski <william.kucharski@oracle.com>
Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 5eeb6335
...@@ -1344,7 +1344,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) ...@@ -1344,7 +1344,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
{ {
unsigned long pfn; unsigned long pfn;
struct page *page; struct page *page;
int not_managed = 0;
int ret = 0; int ret = 0;
LIST_HEAD(source); LIST_HEAD(source);
...@@ -1392,7 +1391,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) ...@@ -1392,7 +1391,6 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
else else
ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE); ret = isolate_movable_page(page, ISOLATE_UNEVICTABLE);
if (!ret) { /* Success */ if (!ret) { /* Success */
put_page(page);
list_add_tail(&page->lru, &source); list_add_tail(&page->lru, &source);
if (!__PageMovable(page)) if (!__PageMovable(page))
inc_node_page_state(page, NR_ISOLATED_ANON + inc_node_page_state(page, NR_ISOLATED_ANON +
...@@ -1401,22 +1399,10 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) ...@@ -1401,22 +1399,10 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
} else { } else {
pr_warn("failed to isolate pfn %lx\n", pfn); pr_warn("failed to isolate pfn %lx\n", pfn);
dump_page(page, "isolation failed"); dump_page(page, "isolation failed");
put_page(page);
/* Because we don't have big zone->lock. we should
check this again here. */
if (page_count(page)) {
not_managed++;
ret = -EBUSY;
break;
}
} }
put_page(page);
} }
if (!list_empty(&source)) { if (!list_empty(&source)) {
if (not_managed) {
putback_movable_pages(&source);
goto out;
}
/* Allocate a new page from the nearest neighbor node */ /* Allocate a new page from the nearest neighbor node */
ret = migrate_pages(&source, new_node_page, NULL, 0, ret = migrate_pages(&source, new_node_page, NULL, 0,
MIGRATE_SYNC, MR_MEMORY_HOTPLUG); MIGRATE_SYNC, MR_MEMORY_HOTPLUG);
...@@ -1429,7 +1415,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn) ...@@ -1429,7 +1415,7 @@ do_migrate_range(unsigned long start_pfn, unsigned long end_pfn)
putback_movable_pages(&source); putback_movable_pages(&source);
} }
} }
out:
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment