[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <23f6e4e5-6e32-faf6-433d-67e50d2895a2@oracle.com>
Date: Fri, 13 Jul 2018 09:24:44 -0400
From: Pavel Tatashin <pasha.tatashin@...cle.com>
To: Oscar Salvador <osalvador@...hadventures.net>
Cc: steven.sistare@...cle.com, daniel.m.jordan@...cle.com,
linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
kirill.shutemov@...ux.intel.com, mhocko@...e.com,
linux-mm@...ck.org, dan.j.williams@...el.com, jack@...e.cz,
jglisse@...hat.com, jrdr.linux@...il.com, bhe@...hat.com,
gregkh@...uxfoundation.org, vbabka@...e.cz,
richard.weiyang@...il.com, dave.hansen@...el.com,
rientjes@...gle.com, mingo@...nel.org, abdhalee@...ux.vnet.ibm.com,
mpe@...erman.id.au
Subject: Re: [PATCH v5 1/5] mm/sparse: abstract sparse buffer allocations
On 07/13/2018 09:17 AM, Oscar Salvador wrote:
> On Thu, Jul 12, 2018 at 04:37:26PM -0400, Pavel Tatashin wrote:
>> +static void *sparsemap_buf __meminitdata;
>> +static void *sparsemap_buf_end __meminitdata;
>> +
>> +void __init sparse_buffer_init(unsigned long size, int nid)
>> +{
>> + BUG_ON(sparsemap_buf);
>
> Why do we need a BUG_ON() here?
> Looking at the code I cannot really see how we can end up with sparsemap_buf being NULL.
> Is it just for over-protection?
This checks that we do not accidentally leak memory by calling sparse_buffer_init() consequently without sparse_buffer_fini() in-between.
>
>> + sparsemap_buf =
>> + memblock_virt_alloc_try_nid_raw(size, PAGE_SIZE,
>> + __pa(MAX_DMA_ADDRESS),
>> + BOOTMEM_ALLOC_ACCESSIBLE, nid);
>
> In your previous version, you didn't pass a required alignment when setting up sparsemap_buf.
> size is already PMD_SIZE aligned, do we need to align it also to PAGE_SIZE?
>
I decided to add PAGE_SIZE alignment, because the implicit memblock alignment is SMP_CACHE_BYTES which is smaller than page size. While, in practice we will most likely get a page size aligned allocation, it is still possible that some ranges in memblock are not page size aligned if that the way they were passed from BIOS.
Thank you,
Pavel
Powered by blists - more mailing lists