From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D7A6C4332B for ; Wed, 17 Mar 2021 14:24:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 755FA64F37 for ; Wed, 17 Mar 2021 14:24:03 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 755FA64F37 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=suse.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id CDF556B0073; Wed, 17 Mar 2021 10:24:02 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C8FF68D0003; Wed, 17 Mar 2021 10:24:02 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B42FA8D0002; Wed, 17 Mar 2021 10:24:02 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0139.hostedemail.com [216.40.44.139]) by kanga.kvack.org (Postfix) with ESMTP id 936E96B0073 for ; Wed, 17 Mar 2021 10:24:02 -0400 (EDT) Received: from smtpin07.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 352E3180AD822 for ; Wed, 17 Mar 2021 14:24:02 +0000 (UTC) X-FDA: 77929585524.07.FDC1F8B Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf10.hostedemail.com (Postfix) with ESMTP id 8AA67401F4F8 for ; Wed, 17 Mar 2021 14:08:55 +0000 (UTC) X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.221.27]) by mx2.suse.de (Postfix) with ESMTP id 51B69ACA8; Wed, 17 Mar 2021 14:08:54 +0000 (UTC) Date: Wed, 17 Mar 2021 15:08:51 +0100 From: Oscar Salvador To: David Hildenbrand Cc: Andrew Morton , Michal Hocko , Anshuman Khandual , Vlastimil Babka , Pavel Tatashin , linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 1/5] mm,memory_hotplug: Allocate memmap from the added memory range Message-ID: <20210317140847.GA20407@linux> References: <20210309175546.5877-1-osalvador@suse.de> <20210309175546.5877-2-osalvador@suse.de> <20210315102224.GA24699@linux> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 8AA67401F4F8 X-Stat-Signature: 3o7j1fyajmmqur87ijbkcqigpaqpmogt Received-SPF: none (suse.de>: No applicable sender policy available) receiver=imf10; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1615990135-582570 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Mar 16, 2021 at 06:45:17PM +0100, David Hildenbrand wrote: > > I find that cross reference to vmemmap code a little hard to digest. > > I would have assume that we don't have to care about PMDs in this > > code here at all. The vmemmap population code should handle that. > > > > I think I already mentioned that somewhere, I think it should be like this: > > > > a) vmemmap code should *never* populate more memory than requested for > > a single memory section when we are populating from the altmap. > > If that cannot be guaranteed for PMDs, then we have to fallback > > to populating base pages. Populating PMDs from an altmap with > > sizeof(struct page) == 64 is highly dangerous. I guess you meant sizeof(struct page) != 64 But other usecases of using altmap (ZONE_DEVICE stuff) might not care whether they have sub-populated PMDs when populating sections from altmap? Current vmemmap code populates PMD with PMD_SIZE if empty, and with basepages if there are still holes. > > Assume we have sizeof(struct page) == 56. A 128 MiB section > > spans 32768 pages - we need 32768 * sizeof(struct page) > > space for the vmemmap. > > With 64k pages we *can* use exactly one PMD. With 56k pages > > we need 448 individual (full!) pages for the vmemmap. > > > > IOW, we don't care how vmemmap code will do the mapping. > > vmemmap code has to get it right. IMHO, asserting it in > > this code is wrong. > > > > > > b) In this code, we really should only care about what > > memory onlining/offlining code can or can't do. > > We really only care that > > > > 1) size == memory_block_size_bytes() > > 2) remaining_size > > 3) IS_ALIGNED(remaining_size, pageblock_size); I agree with the above, but see below: > > Okay, please document the statement about single sections, that's > > important to understand what's happening. > > > > My take would be > > > > bool mhp_supports_memmap_on_memory(unsigned long size) > > { > > /* > > * Note: We calculate for a single memory section. The calculation > > */ > > unsigned long nr_vmemmap_pages = SECTION_SIZE / PAGE_SIZE; > > unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page); > > unsigned long remaining_size = size - vmemmap_size; While it might be true that we need to back off from populating with altmap in case PMDs are not going to be fully populated because of the size of the struct page (I am not still not sure though as I said above, other usecases might not care at all), I would go __for now__ with placing vmemmap_size == PMD_SIZE in the check below as well. If the check comes true, we know that we fully populate PMDs when populating sections, so the feature can be used. Then I commit to have a look whether we need to back off in vmemmap-populating code in case altmap && !NOT_FULLY_POPULATED_PMDS. What do you think? -- Oscar Salvador SUSE L3