From: Vlastimil Babka <vbabka@suse.cz>
To: Michal Hocko <mhocko@kernel.org>
Cc: linux-mm@kvack.org, Andrew Morton <akpm@linux-foundation.org>,
Mel Gorman <mgorman@suse.de>,
Johannes Weiner <hannes@cmpxchg.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 7/9] mm, page_alloc: remove stop_machine from build_all_zonelists
Date: Thu, 20 Jul 2017 08:16:49 +0200 [thread overview]
Message-ID: <0d4c6dad-5a4f-fc67-d401-20e49805e773@suse.cz> (raw)
In-Reply-To: <20170714114509.GK2618@dhcp22.suse.cz>
On 07/14/2017 01:45 PM, Michal Hocko wrote:
> On Fri 14-07-17 13:43:21, Michal Hocko wrote:
>> On Fri 14-07-17 13:29:14, Vlastimil Babka wrote:
>>> On 07/14/2017 10:00 AM, Michal Hocko wrote:
>>>> From: Michal Hocko <mhocko@suse.com>
>>>>
>>>> build_all_zonelists has been (ab)using stop_machine to make sure that
>>>> zonelists do not change while somebody is looking at them. This is
>>>> is just a gross hack because a) it complicates the context from which
>>>> we can call build_all_zonelists (see 3f906ba23689 ("mm/memory-hotplug:
>>>> switch locking to a percpu rwsem")) and b) is is not really necessary
>>>> especially after "mm, page_alloc: simplify zonelist initialization".
>>>>
>>>> Updates of the zonelists happen very seldom, basically only when a zone
>>>> becomes populated during memory online or when it loses all the memory
>>>> during offline. A racing iteration over zonelists could either miss a
>>>> zone or try to work on one zone twice. Both of these are something we
>>>> can live with occasionally because there will always be at least one
>>>> zone visible so we are not likely to fail allocation too easily for
>>>> example.
>>>
>>> Given the experience with with cpusets and mempolicies, I would rather
>>> avoid the risk of allocation not seeing the only zone(s) that are
>>> allowed by its nodemask, and triggering premature OOM.
>>
>> I would argue, those are a different beast because they are directly
>> under control of not fully priviledged user and change between the empty
>> nodemask and cpusets very often. For this one to trigger we
>> would have to online/offline the last memory block in the zone very
>> often and that doesn't resemble a sensible usecase even remotely.
OK.
>>> So maybe the
>>> updates could be done in a way to avoid that, e.g. first append a copy
>>> of the old zonelist to the end, then overwrite and terminate with NULL.
>>> But if this requires any barriers or something similar on the iteration
>>> site, which is performance critical, then it's bad.
>>> Maybe a seqcount, that the iteration side only starts checking in the
>>> slowpath? Like we have with cpusets now.
>>> I know that Mel noted that stop_machine() also never had such guarantees
>>> to prevent this, but it could have made the chances smaller.
>>
>> I think we can come up with some scheme but is this really worth it
>> considering how unlikely the whole thing is? Well, if somebody hits a
>> premature OOM killer or allocations failures it would have to be along
>> with a heavy memory hotplug operations and then it would be quite easy
>> to spot what is going on and try to fix it. I would rather not
>> overcomplicate it, to be honest.
Fine, we can always add it later.
> And one more thing, Mel has already brought this up in his response.
> stop_machine haven't is very roughly same strenght wrt. double zone
> visit or a missed zone because we do not restart zonelist iteration.
I know, that's why I wrote "I know that Mel noted that stop_machine()
also never had such guarantees to prevent this, but it could have made
the chances smaller." But I don't have any good proof that your patch is
indeed making things worse, so let's apply and see...
next prev parent reply other threads:[~2017-07-20 6:16 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-14 7:59 [PATCH 0/9] cleanup zonelists initialization Michal Hocko
2017-07-14 7:59 ` [PATCH 1/9] mm, page_alloc: rip out ZONELIST_ORDER_ZONE Michal Hocko
2017-07-14 9:36 ` Mel Gorman
2017-07-14 10:47 ` Michal Hocko
2017-07-14 11:16 ` Mel Gorman
2017-07-14 11:38 ` Michal Hocko
2017-07-14 12:56 ` Mel Gorman
2017-07-14 13:01 ` Mel Gorman
2017-07-14 13:08 ` Michal Hocko
2017-07-19 9:33 ` Vlastimil Babka
2017-07-19 13:44 ` Michal Hocko
2017-07-14 7:59 ` [PATCH 2/9] mm, page_alloc: remove boot pageset initialization from memory hotplug Michal Hocko
2017-07-14 9:39 ` Mel Gorman
2017-07-19 13:15 ` Vlastimil Babka
2017-07-14 8:00 ` [PATCH 3/9] mm, page_alloc: do not set_cpu_numa_mem on empty nodes initialization Michal Hocko
2017-07-14 9:48 ` Mel Gorman
2017-07-14 10:50 ` Michal Hocko
2017-07-14 12:32 ` Mel Gorman
2017-07-14 12:39 ` Michal Hocko
2017-07-14 12:56 ` Mel Gorman
2017-07-19 13:19 ` Vlastimil Babka
2017-07-14 8:00 ` [PATCH 4/9] mm, memory_hotplug: drop zone from build_all_zonelists Michal Hocko
2017-07-19 13:33 ` Vlastimil Babka
2017-07-20 8:15 ` Michal Hocko
2017-07-14 8:00 ` [PATCH 5/9] mm, memory_hotplug: remove explicit build_all_zonelists from try_online_node Michal Hocko
2017-07-14 12:14 ` Michal Hocko
2017-07-20 6:13 ` Vlastimil Babka
2017-07-14 8:00 ` [PATCH 6/9] mm, page_alloc: simplify zonelist initialization Michal Hocko
2017-07-14 9:55 ` Mel Gorman
2017-07-14 10:51 ` Michal Hocko
2017-07-14 12:46 ` Mel Gorman
2017-07-14 13:02 ` Michal Hocko
2017-07-14 14:18 ` Mel Gorman
2017-07-17 6:06 ` Michal Hocko
2017-07-17 8:07 ` Mel Gorman
2017-07-17 8:19 ` Michal Hocko
2017-07-17 8:58 ` Mel Gorman
2017-07-17 9:15 ` Michal Hocko
2017-07-20 6:55 ` Vlastimil Babka
2017-07-20 7:19 ` Michal Hocko
2017-07-14 8:00 ` [PATCH 7/9] mm, page_alloc: remove stop_machine from build_all_zonelists Michal Hocko
2017-07-14 9:59 ` Mel Gorman
2017-07-14 11:00 ` Michal Hocko
2017-07-14 12:47 ` Mel Gorman
2017-07-14 11:29 ` Vlastimil Babka
2017-07-14 11:43 ` Michal Hocko
2017-07-14 11:45 ` Michal Hocko
2017-07-20 6:16 ` Vlastimil Babka [this message]
2017-07-20 7:24 ` Vlastimil Babka
2017-07-20 9:21 ` Michal Hocko
2017-07-14 8:00 ` [PATCH 8/9] mm, memory_hotplug: get rid of zonelists_mutex Michal Hocko
2017-07-14 8:00 ` [PATCH 9/9] mm, sparse, page_ext: drop ugly N_HIGH_MEMORY branches for allocations Michal Hocko
2017-07-20 8:04 ` Vlastimil Babka
2017-07-21 14:39 [PATCH -v1 0/9] cleanup zonelists initialization Michal Hocko
2017-07-21 14:39 ` [PATCH 7/9] mm, page_alloc: remove stop_machine from build_all_zonelists Michal Hocko
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=0d4c6dad-5a4f-fc67-d401-20e49805e773@suse.cz \
--to=vbabka@suse.cz \
--cc=akpm@linux-foundation.org \
--cc=hannes@cmpxchg.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mgorman@suse.de \
--cc=mhocko@kernel.org \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).