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=-3.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 5E2B3C4332E for ; Thu, 18 Mar 2021 10:38:56 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id CDFBE64F3B for ; Thu, 18 Mar 2021 10:38:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CDFBE64F3B 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 42DF86B0070; Thu, 18 Mar 2021 06:38:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3B64C6B0071; Thu, 18 Mar 2021 06:38:55 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 22FF26B0072; Thu, 18 Mar 2021 06:38:55 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0128.hostedemail.com [216.40.44.128]) by kanga.kvack.org (Postfix) with ESMTP id 02E206B0070 for ; Thu, 18 Mar 2021 06:38:54 -0400 (EDT) Received: from smtpin19.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id B1B4F1801612E for ; Thu, 18 Mar 2021 10:38:54 +0000 (UTC) X-FDA: 77932646988.19.40A5298 Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by imf19.hostedemail.com (Postfix) with ESMTP id 1F25C90009D6 for ; Thu, 18 Mar 2021 10:38:53 +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 BAD23AC1E; Thu, 18 Mar 2021 10:38:52 +0000 (UTC) Date: Thu, 18 Mar 2021 11:38:48 +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: References: <20210309175546.5877-1-osalvador@suse.de> <20210309175546.5877-2-osalvador@suse.de> <20210315102224.GA24699@linux> <20210317140847.GA20407@linux> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 1F25C90009D6 X-Stat-Signature: 1z18gx9f6wbrzd3noyeo6m3sjk9nndda Received-SPF: none (suse.de>: No applicable sender policy available) receiver=imf19; identity=mailfrom; envelope-from=""; helo=mx2.suse.de; client-ip=195.135.220.15 X-HE-DKIM-Result: none/none X-HE-Tag: 1616063933-738190 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 Thu, Mar 18, 2021 at 09:27:48AM +0100, Oscar Salvador wrote: > > If we check for > > > > IS_ALIGNED(nr_vmemmap_pages, PMD_SIZE), please add a proper TODO comment > > that this is most probably the wrong place to take care of this. > > Sure, I will stuff the check in there and place a big TODO comment so we > do not forget about addressing this issue the right way. Ok, I realized something while working on v5. Here is what I have right now: bool mhp_supports_memmap_on_memory(unsigned long size) { /* * Note: We calculate for a single memory section. The calculation * implicitly covers memory blocks that span multiple sections. * * Not all archs define SECTION_SIZE, but MIN_MEMORY_BLOCK_SIZE always * equals SECTION_SIZE, so use that instead. */ unsigned long nr_vmemmap_pages = MIN_MEMORY_BLOCK_SIZE / PAGE_SIZE; unsigned long vmemmap_size = nr_vmemmap_pages * sizeof(struct page); unsigned long remaining_size = size - vmemmap_size; /* * Besides having arch support and the feature enabled at runtime, we * need a few more assumptions to hold true: * * a) We span a single memory block: memory onlining/offlinin;g happens * in memory block granularity. We don't want the vmemmap of online * memory blocks to reside on offline memory blocks. In the future, * we might want to support variable-sized memory blocks to make the * feature more versatile. * * b) The vmemmap pages span complete PMDs: We don't want vmemmap code * to populate memory from the altmap for unrelated parts (i.e., * other memory blocks) * * c) The vmemmap pages (and thereby the pages that will be exposed to * the buddy) have to cover full pageblocks: memory onlining/offlining * code requires applicable ranges to be page-aligned, for example, to * set the migratetypes properly. * * TODO: Although we have a check here to make sure that vmemmap pages * fully populate a PMD, it is not the right place to check for * this. A much better solution involves improving vmemmap code * to fallback to base pages when trying to populate vmemmap using * altmap as an alternative source of memory, and we do not exactly * populate a single PMD. */ return memmap_on_memory && IS_ENABLED(CONFIG_MHP_MEMMAP_ON_MEMORY) && size == memory_block_size_bytes() && remaining_size && IS_ALIGNED(remaining_size, pageblock_size) && IS_ALIGNED(vmemmap_size, PMD_SIZE); } Assume we are on x86_64 to simplify the case. Above, nr_vmemmap_pages would be 32768 and vmemmap_size 2MB (exactly a PMD). Now, although correct, this nr_vmemmap_pages does not match with the altmap->alloc. static void * __meminit altmap_alloc_block_buf(unsigned long size, struct altmap) { ... ... nr_pfns = size >> PAGE_SHIFT; //size is PMD_SIZE altmap->alloc += nr_pfns; } altmap->alloc will be 512, 512 * 4K pages = 2MB. Of course, the reason they do not match is because in one case, we are saying a) how many pfns we need to cover a PMD_SIZE, while in the other case we say b) how many pages we need to cover SECTION_SIZE Then b) multiply for page_size to get the current size of it. So, I have mixed feeling about this. Would it be more clear to just do: bool mhp_supports_memmap_on_memory(unsigned long size) { /* * Note: We calculate for a single memory section. The calculation * implicitly covers memory blocks that span multiple sections. */ unsigned long nr_vmemmap_pages = PMD_SIZE / PAGE_SIZE; unsigned long vmemmap_size = nr_vmemmap_pages * PAGE_SIZE; unsigned long remaining_size = size - vmemmap_size; ... ... -- Oscar Salvador SUSE L3