On 09/13/2017 02:14 PM, Michal Hocko wrote: >>>> Do you think that the changelog should be more clear about this? >>> >>> It certainly wouldn't hurt :) >> >> So what do you think about the following wording: > > Ups, wrong patch > > > From 8639496a834b4a7c24972ec23b17e50f0d6a304c Mon Sep 17 00:00:00 2001 > From: Michal Hocko <mhocko@xxxxxxxx> > Date: Mon, 14 Aug 2017 10:46:12 +0200 > Subject: [PATCH 1/2] mm, memory_hotplug: do not fail offlining too early > > Memory offlining can fail just too eagerly under a heavy memory pressure. > > [ 5410.336792] page:ffffea22a646bd00 count:255 mapcount:252 mapping:ffff88ff926c9f38 index:0x3 > [ 5410.336809] flags: 0x9855fe40010048(uptodate|active|mappedtodisk) > [ 5410.336811] page dumped because: isolation failed > [ 5410.336813] page->mem_cgroup:ffff8801cd662000 > [ 5420.655030] memory offlining [mem 0x18b580000000-0x18b5ffffffff] failed > > Isolation has failed here because the page is not on LRU. Most probably > because it was on the pcp LRU cache or it has been removed from the LRU > already but it hasn't been freed yet. In both cases the page doesn't look > non-migrable so retrying more makes sense. > > __offline_pages seems rather cluttered when it comes to the retry > logic. We have 5 retries at maximum and a timeout. We could argue > whether the timeout makes sense but failing just because of a race when > somebody isoltes a page from LRU or puts it on a pcp LRU lists is just > wrong. It only takes it to race with a process which unmaps some pages > and remove them from the LRU list and we can fail the whole offline > because of something that is a temporary condition and actually not > harmful for the offline. > > Please note that unmovable pages should be already excluded during > start_isolate_page_range. We could argue that has_unmovable_pages is > racy and MIGRATE_MOVABLE check doesn't provide any hard guarantee either > but kernel zones (aka < ZONE_MOVABLE) will very likely detect unmovable > pages in most cases and movable zone shouldn't contain unmovable pages > at all. Some of those pages might be pinned but not for ever because > that would be a bug on its own. In any case the context is still > interruptible and so the userspace can easily bail out when the > operation takes too long. This is certainly better behavior than a > hardcoded retry loop which is racy. > > Fix this by removing the max retry count and only rely on the timeout > resp. interruption by a signal from the userspace. Also retry rather > than fail when check_pages_isolated sees some !free pages because those > could be a result of the race as well. > > Signed-off-by: Michal Hocko <mhocko@xxxxxxxx> Yeah, that's better, thanks. Acked-by: Vlastimil Babka <vbabka@xxxxxxx> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>
