On Wed 13-09-17 14:19:19, Vlastimil Babka wrote: > 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> Thanks. I will give it a day and repost the series. If somebody still have some concerns please speak up. -- Michal Hocko SUSE Labs -- 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>
