[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 7 Jul 2020 09:20:52 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: David Hildenbrand <david@...hat.com>, linux-mm@...ck.org
Cc: justin.he@....com, catalin.marinas@....com,
akpm@...ux-foundation.org, Will Deacon <will@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Tony Luck <tony.luck@...el.com>,
Fenghua Yu <fenghua.yu@...el.com>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Andy Lutomirski <luto@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
Mike Rapoport <rppt@...ux.ibm.com>,
Michal Hocko <mhocko@...e.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
Dan Williams <dan.j.williams@...el.com>,
Pavel Tatashin <pasha.tatashin@...een.com>,
linux-arm-kernel@...ts.infradead.org, linux-ia64@...r.kernel.org,
linux-riscv@...ts.infradead.org, x86@...nel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH V4 1/3] mm/sparsemem: Enable vmem_altmap support in
vmemmap_populate_basepages()
On 07/06/2020 02:33 PM, David Hildenbrand wrote:
>> return 0;
>> @@ -1505,7 +1505,7 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
>> int err;
>>
>> if (end - start < PAGES_PER_SECTION * sizeof(struct page))
>> - err = vmemmap_populate_basepages(start, end, node);
>> + err = vmemmap_populate_basepages(start, end, node, NULL);
>> else if (boot_cpu_has(X86_FEATURE_PSE))
>> err = vmemmap_populate_hugepages(start, end, node, altmap);
>> else if (altmap) {
>
> It's somewhat weird that we don't allocate basepages from altmap on x86
> (both for sub-sections and without PSE). I wonder if we can simply
> unlock that with your change. Especially, also handle the
> !X86_FEATURE_PSE case below properly with an altmap.
>
> a) all hw with PMEM has PSE - except special QEMU setups, so nobody
> cared to implement. For the sub-section special case, nobody cared about
> a handfull of memmap not ending up on the altmap. (but it's still wasted
> system memory IIRC).
>
> b) the pagetable overhead for small pages is not-neglectable and might
> result in similar issues as solved by the switch to altmap on very huge
> PMEM (with small amount of system RAM).
>
> I guess it is due to a).
Hmm, I assume these are some decisions that x86 platform will have to
make going forward in a subsequent patch as the third patch does for
the arm64 platform. But it is clearly beyond the scope of this patch
which never intended to change existing behavior on a given platform.
>
> [...]
>
>>
>> -pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node)
>> +pte_t * __meminit vmemmap_pte_populate(pmd_t *pmd, unsigned long addr, int node,
>> + struct vmem_altmap *altmap)
>> {
>> pte_t *pte = pte_offset_kernel(pmd, addr);
>> if (pte_none(*pte)) {
>> pte_t entry;
>> - void *p = vmemmap_alloc_block_buf(PAGE_SIZE, node);
>> + void *p;
>> +
>> + if (altmap)
>> + p = altmap_alloc_block_buf(PAGE_SIZE, altmap);
>> + else
>> + p = vmemmap_alloc_block_buf(PAGE_SIZE, node);
>> if (!p)
>> return NULL;
>
> I was wondering if
>
> if (altmap)
> p = altmap_alloc_block_buf(PAGE_SIZE, altmap);
> if (!p)
> p = vmemmap_alloc_block_buf(PAGE_SIZE, node);
> if (!p)
> return NULL
>
> Would make sense. But I guess this isn't really relevant in practice,
> because the altmap is usually sized properly.
>
> In general, LGTM.
Okay, I assume that no further changes are required here.
Powered by blists - more mailing lists