Re: [RFC v2 PATCH] mm/hotplug: enable memory hotplug for non-lru movable pages
From: Yisheng Xie <hidden>
Date: 2017-01-30 15:24:42
Also in:
lkml
hi Michal, Thank you for reviewing and sorry for late reply. On 01/26/2017 05:43 PM, Michal Hocko wrote:
quoted hunk ↗ jump to hunk
On Wed 25-01-17 14:59:45, Yisheng Xie wrote: static unsigned long scan_movable_pages(unsigned long start, unsigned long end) {@@ -1531,6 +1531,16 @@ static unsigned long scan_movable_pages(unsigned long start, unsigned long end) pfn = round_up(pfn + 1, 1 << compound_order(page)) - 1; } + /* + * check __PageMovable in lock_page to avoid miss some + * non-lru movable pages at race condition. + */ + lock_page(page); + if (__PageMovable(page)) { + unlock_page(page); + return pfn; + } + unlock_page(page);This doesn't make any sense to me. __PageMovable can change right after you drop the lock so why the race matters? If we cannot tolerate races then the above doesn't work and if we can then taking the lock is pointless.
hmm, for PageLRU check may also race without lru-locki 1/4 ? I think it is ok to check __PageMovable without lock_page, here.
quoted
} } return 0;@@ -1600,21 +1610,25 @@ static struct page *new_node_page(struct page *page, unsigned long private, if (!get_page_unless_zero(page)) continue; /* - * We can skip free pages. And we can only deal with pages on - * LRU. + * We can skip free pages. And we can deal with pages on + * LRU and non-lru movable pages. */ - ret = isolate_lru_page(page); + if (PageLRU(page)) + ret = isolate_lru_page(page); + else + ret = !isolate_movable_page(page, ISOLATE_UNEVICTABLE);we really want to propagate the proper error code to the caller.
Yes , I make the same mistake again. Really sorry about that. Maybe I can rewrite the isolate_movable_page to let it return int as isolate_lru_page do in this patchset :) Thanks Yisheng Xie -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>