From: David Hildenbrand <david@redhat.com>
To: Wei Yang <richard.weiyang@gmail.com>
Cc: Michal Hocko <mhocko@suse.com>,
osalvador@suse.de, akpm@linux-foundation.org, linux-mm@kvack.org
Subject: Re: [PATCH v2] mm, hotplug: move init_currently_empty_zone() under zone_span_lock protection
Date: Mon, 26 Nov 2018 10:24:26 +0100 [thread overview]
Message-ID: <424dd9eb-7295-5cf4-98b2-7a1bfd53b32e@redhat.com> (raw)
In-Reply-To: <20181126014451.grrdei2luujiqwrh@master>
On 26.11.18 02:44, Wei Yang wrote:
> On Fri, Nov 23, 2018 at 09:46:52AM +0100, David Hildenbrand wrote:
>> On 23.11.18 09:42, Michal Hocko wrote:
>>> On Thu 22-11-18 16:26:40, David Hildenbrand wrote:
>>>> On 22.11.18 11:12, Wei Yang wrote:
>>>>> During online_pages phase, pgdat->nr_zones will be updated in case this
>>>>> zone is empty.
>>>>>
>>>>> Currently the online_pages phase is protected by the global lock
>>>>> mem_hotplug_begin(), which ensures there is no contention during the
>>>>> update of nr_zones. But this global lock introduces scalability issues.
>>>>>
>>>>> This patch is a preparation for removing the global lock during
>>>>> online_pages phase. Also this patch changes the documentation of
>>>>> node_size_lock to include the protectioin of nr_zones.
>>>>
>>>> I looked into locking recently, and there is more to it.
>>>>
>>>> Please read:
>>>>
>>>> commit dee6da22efac451d361f5224a60be2796d847b51
>>>> Author: David Hildenbrand <david@redhat.com>
>>>> Date: Tue Oct 30 15:10:44 2018 -0700
>>>>
>>>> memory-hotplug.rst: add some details about locking internals
>>>>
>>>> Let's document the magic a bit, especially why device_hotplug_lock is
>>>> required when adding/removing memory and how it all play together with
>>>> requests to online/offline memory from user space.
>>>>
>>>> Short summary: Onlining/offlining of memory requires the device_hotplug_lock
>>>> as of now.
>>>
>>> Well, I would tend to disagree here. You might be describing the current
>>> state of art but the device_hotplug_lock doesn't make much sense for the
>>> memory hotplug in principle. There is absolutely nothing in the core MM
>>
>> There are collisions with CPU hotplug that require this lock (when nodes
>> come and go as far as I remember). And there is the problematic lock
>> inversion that can happen when adding/remving memory. This all has to be
>> sorted out, we'll have to see if we really need it for
>> onlining/offlining, though, however ...
>>
>
> Seems I get a little understanding on this part.
>
> There are two hotplug:
> * CPU hotplug
> * Memory hotplug.
>
> There are two phase for Memory hotplug:
> * physical add/remove
> * logical online/offline
>
> All of them are protected by device_hotplug_lock now, so we need to be
> careful to release this in any case. Is my understanding correct?
Yes, e.g. the acpi driver always held the device_hotplug_lock (due to
possible problems with concurrent cpu/memory hot(un)plug). Onlining
offlining of devices (including cpu/memory) from user space always held
the device_hotplug_lock. So this part was executed sequentially for a
long time.
I recently made sure that any adding/removing/onlining/offlining
correctly grabs the device_hotplug_lock AND the mem_hotplug_lock in any
case (because it was inconsistent and broken), so it is all executed
sequentially.
So when getting rid of mem_hotplug_lock we only have to care about all
users that don't take the device_hotplug_lock.
>
>>> that would require this lock. The current state just uses a BKL in some
>>> sense and we really want to get rid of that longterm. This patch is a tiny
>>> step in that direction and I suspect many more will need to come on the
>>> way. We really want to end up with a clear scope of each lock being
>>> taken. A project for a brave soul...
>>
>> ... for now I don't consider "optimize for parallel
>> onlining/offlining/adding/removing of memory and cpus" really necessary.
>> What is necessary indeed is to not slowdown the whole system just
>> because some memory is coming/going. Therefore I agree, this patch is a
>> step into the right direction.
>>
>
> Agree.
>
> The target is to accelerate the hot-plug without slow down the normal
> process.
Indeed!
--
Thanks,
David / dhildenb
next prev parent reply other threads:[~2018-11-26 9:24 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-20 1:48 [PATCH] mm, hotplug: protect nr_zones with pgdat_resize_lock() Wei Yang
2018-11-20 7:31 ` Michal Hocko
2018-11-20 7:58 ` osalvador
2018-11-20 8:48 ` Michal Hocko
2018-11-21 2:52 ` Wei Yang
2018-11-21 7:15 ` Michal Hocko
2018-11-22 1:52 ` Wei Yang
2018-11-22 8:39 ` Michal Hocko
2018-11-26 2:28 ` Wei Yang
2018-11-26 8:16 ` Michal Hocko
2018-11-26 9:06 ` Wei Yang
2018-11-26 10:03 ` Michal Hocko
2018-11-27 0:18 ` Wei Yang
2018-11-27 3:12 ` Wei Yang
2018-11-27 13:16 ` Michal Hocko
2018-11-27 23:56 ` Wei Yang
2018-11-21 8:24 ` osalvador
2018-11-21 2:44 ` Wei Yang
2018-11-21 7:14 ` Michal Hocko
2018-11-22 10:12 ` [PATCH v2] mm, hotplug: move init_currently_empty_zone() under zone_span_lock protection Wei Yang
2018-11-22 10:15 ` Wei Yang
2018-11-22 10:29 ` Michal Hocko
2018-11-22 14:27 ` Wei Yang
2018-11-22 10:37 ` osalvador
2018-11-22 14:28 ` Wei Yang
2018-11-22 15:26 ` David Hildenbrand
2018-11-22 21:28 ` Wei Yang
2018-11-22 21:53 ` David Hildenbrand
2018-11-22 23:53 ` Wei Yang
2018-11-23 8:42 ` Michal Hocko
2018-11-23 8:46 ` David Hildenbrand
2018-11-26 1:44 ` Wei Yang
2018-11-26 9:24 ` David Hildenbrand [this message]
2018-11-27 0:23 ` Wei Yang
2018-11-30 6:58 ` [PATCH v3] " Wei Yang
2018-11-30 9:30 ` David Hildenbrand
2018-12-01 0:27 ` Wei Yang
2018-12-03 10:09 ` David Hildenbrand
2018-12-03 20:37 ` Wei Yang
2018-12-03 20:50 ` [PATCH v4] " Wei Yang
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=424dd9eb-7295-5cf4-98b2-7a1bfd53b32e@redhat.com \
--to=david@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=osalvador@suse.de \
--cc=richard.weiyang@gmail.com \
/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).