From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt0-f197.google.com (mail-qt0-f197.google.com [209.85.216.197]) by kanga.kvack.org (Postfix) with ESMTP id 2E50C6B0527 for ; Tue, 1 Aug 2017 07:30:52 -0400 (EDT) Received: by mail-qt0-f197.google.com with SMTP id t37so6091275qtg.6 for ; Tue, 01 Aug 2017 04:30:52 -0700 (PDT) Received: from mx1.redhat.com (mx1.redhat.com. [209.132.183.28]) by mx.google.com with ESMTPS id d140si13672555qkb.223.2017.08.01.04.30.50 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 01 Aug 2017 04:30:51 -0700 (PDT) Date: Tue, 1 Aug 2017 13:30:39 +0200 From: Igor Mammedov Subject: Re: [RFC PATCH 0/5] mm, memory_hotplug: allocate memmap from hotadded memory Message-ID: <20170801133039.6293f108@nial.brq.redhat.com> In-Reply-To: <20170731195830.0d0ebf2f@thinkpad> References: <20170726083333.17754-1-mhocko@kernel.org> <20170726210657.GE21717@redhat.com> <20170727065652.GE20970@dhcp22.suse.cz> <20170728121941.GL2274@dhcp22.suse.cz> <20170731143521.5809a6ca@thinkpad> <20170731125319.GA4829@dhcp22.suse.cz> <20170731170459.613d5cbd@thinkpad> <20170731155350.GA1189@dhcp22.suse.cz> <20170731195830.0d0ebf2f@thinkpad> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: Gerald Schaefer Cc: Michal Hocko , Jerome Glisse , linux-mm@kvack.org, Andrew Morton , Mel Gorman , Vlastimil Babka , Andrea Arcangeli , Reza Arbab , Yasuaki Ishimatsu , qiuxishi@huawei.com, Kani Toshimitsu , slaoub@gmail.com, Joonsoo Kim , Andi Kleen , Daniel Kiper , Vitaly Kuznetsov , LKML , Benjamin Herrenschmidt , Catalin Marinas , Dan Williams , Fenghua Yu , Heiko Carstens , "H. Peter Anvin" , Ingo Molnar , Martin Schwidefsky , Michael Ellerman , Paul Mackerras , Thomas Gleixner , Tony Luck , Will Deacon On Mon, 31 Jul 2017 19:58:30 +0200 Gerald Schaefer wrote: > On Mon, 31 Jul 2017 17:53:50 +0200 > Michal Hocko wrote: > > > On Mon 31-07-17 17:04:59, Gerald Schaefer wrote: > > > On Mon, 31 Jul 2017 14:53:19 +0200 > > > Michal Hocko wrote: > > > > > > > On Mon 31-07-17 14:35:21, Gerald Schaefer wrote: > > > > > On Fri, 28 Jul 2017 14:19:41 +0200 > > > > > Michal Hocko wrote: > > > > > > > > > > > On Thu 27-07-17 08:56:52, Michal Hocko wrote: > > > > > > > On Wed 26-07-17 17:06:59, Jerome Glisse wrote: > > > > > > > [...] > > > > > > > > This does not seems to be an opt-in change ie if i am reading patch 3 > > > > > > > > correctly if an altmap is not provided to __add_pages() you fallback > > > > > > > > to allocating from begining of zone. This will not work with HMM ie > > > > > > > > device private memory. So at very least i would like to see some way > > > > > > > > to opt-out of this. Maybe a new argument like bool forbid_altmap ? > > > > > > > > > > > > > > OK, I see! I will think about how to make a sane api for that. > > > > > > > > > > > > This is what I came up with. s390 guys mentioned that I cannot simply > > > > > > use the new range at this stage yet. This will need probably some other > > > > > > changes but I guess we want an opt-in approach with an arch veto in general. > > > > > > > > > > > > So what do you think about the following? Only x86 is update now and I > > > > > > will split it into two parts but the idea should be clear at least. > > > > > > > > > > This looks good, and the kernel will also boot again on s390 when applied > > > > > on top of the other 5 patches (plus adding the s390 part here). > > > > > > > > Thanks for testing Gerald! I am still undecided whether the arch code > > > > should veto MHP_RANGE_ACCESSIBLE if it cannot be supported or just set > > > > it when it is supported. My last post did the later but the first one > > > > sounds like a more clear API to me. I will keep thinking about it. > > > > > > > > Anyway, did you have any chance to consider mapping the new physical > > > > memory range inside arch_add_memory rather than during online on s390? > > > > > > Well, it still looks like we cannot do w/o splitting up add_memory(): > > > 1) (only) set up section map during our initial memory detection, w/o > > > allocating struct pages, so that the sysfs entries get created also for > > > our offline memory (or else we have no way to online it later) > > > 2) set up vmemmap and allocate struct pages with your new altmap approach > > > during our MEM_GOING_ONLINE callback, because only now the memory is really > > > accessible > > > > As I've tried to mentioned in my other response. This is not possible > > because there are memory hotplug usecases which never do an explicit > > online. > > Of course the default behaviour should not change, we only need an option > to do the "2-stage-approach". E.g. we would call __add_pages() from our > MEM_GOING_ONLINE handler, and not from arch_add_memory() as before, but > then we would need some way to add memory sections (for generating sysfs > memory blocks) only, without allocating struct pages. See also below. > > > > > I am sorry to ask again. But why exactly cannot we make the range > > accessible from arch_add_memory on s390? > > We have no acpi or other events to indicate new memory, both online and > offline memory needs to be (hypervisor) defined upfront, and then we want > to be able to use memory hotplug for ballooning during runtime. > > Making the range accessible is equivalent to a hypervisor call that assigns > the memory to the guest. The problem with arch_add_memory() is now that > this gets called from add_memory(), which we call during initial memory > detection for the offline memory ranges. At that time, assigning all > offline memory to the guest, and thus making it accessible, would break > the ballooning usecase (even if it is still offline in Linux, the > hypervisor could not use it for other guests any more). > > The main difference to other architectures is that we can not simply > call add_memory() (and thus arch_add_memory()) at the time when the > offline memory is actually supposed to get online (e.g. triggered by acpi). > We rather need to somehow make sure that the offline memory is detected > early, and sysfs entries are created for it, so that it can be set online > later on demand. > > Maybe our design to use add_memory() for offline ranges during memory > detection was wrong, or overkill, since we actually only need to establish > a memory section, if I understood the sysfs code right. But I currently > see no other way to make sure that we get the sysfs attributes. And of > course the presence of users that work on offline struct pages, like > valid_zones, is also not helpful. > > > > > > Besides the obvious problem that this would need a new interface, there is > > > also the problem that (at least) show_valid_zones() in drivers/base/memory.c > > > operates on struct pages from _offline_ memory, for its page_zone() checks. > > > This will not work well if we have no struct pages for offline memory ... > > > > Yes. > > > > > BTW, the latter may also be a issue with your rework on any architecture. > > > Not sure if I understood it correctly, but the situation on s390 (i.e. > > > having offline memory blocks visible in sysfs) should be similar to > > > the scenario on x86, when you plug in memory, set it online in the acpi > > > handler, and then manually set it offline again via sysfs. Now the > > > memory is still visible in sysfs, and reading the valid_zones attribute > > > will trigger an access to struct pages for that memory. What if this > > > memory is now physically removed, in a race with such a struct page > > > access? > > > > The memmap goes away together with the whole section tear down. And we > > shouldn't have any users of any struct page by that time. Memblock sysfs > > should be down as well. I will go and double check whether there are any > > possible races. > > I was thinking of someone pulling out a DIMM whose range was (manually) > set offline before. It looks like (arch_)remove_memory() is not triggered > directly on setting it offline, but rather by an acpi event, probably after > physical memory removal. Order here a little bit different, first and ACPI event sent and processed by kernel (including removing ranges from sysfs) and only then kernel should call ACPI _EJ0 method on DIMM device when there shouldn't be any users left nor races should happen. > And that would mean that a user could just read > sysfs valid_zones in a loop, after setting it offline and before the > physical removal, thereby accessing struct pages in the offline range, > which would then race with the physical DIMM removal. > > However, as you can see, s390 memory hotplug works in a special way, > so I may have gotten the wrong picture of how it works on "normal" > architectures :-) > > Regards, > Gerald > -- 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: email@kvack.org