linux-kernel - Re: [PATCH V2] mm/page_alloc: Add alloc_contig

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <31505b6d-1c09-4fcc-a079-3fbb3c96da48@redhat.com>
Date:   Thu, 17 Oct 2019 09:38:48 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Anshuman Khandual <anshuman.khandual@....com>, linux-mm@...ck.org,
        Mike Kravetz <mike.kravetz@...cle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        David Rientjes <rientjes@...gle.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Oscar Salvador <osalvador@...e.de>,
        Mel Gorman <mgorman@...hsingularity.net>,
        Mike Rapoport <rppt@...ux.ibm.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Pavel Tatashin <pavel.tatashin@...rosoft.com>,
        Matthew Wilcox <willy@...radead.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] mm/page_alloc: Add alloc_contig_pages()

On 17.10.19 09:34, Michal Hocko wrote:
> On Thu 17-10-19 09:21:24, David Hildenbrand wrote:
>> On 17.10.19 09:11, Michal Hocko wrote:
>>> On Thu 17-10-19 10:44:41, Anshuman Khandual wrote:
>>> [...]
>>>> Does this add-on documentation look okay ? Should we also mention about the
>>>> possible reduction in chances of success during pfn block search for the
>>>> non-power-of-two cases as the implicit alignment will probably turn out to
>>>> be bigger than nr_pages itself ?
>>>>
>>>>    * Requested nr_pages may or may not be power of two. The search for suitable
>>>>    * memory range in a zone happens in nr_pages aligned pfn blocks. But in case
>>>>    * when nr_pages is not power of two, an implicitly aligned pfn block search
>>>>    * will happen which in turn will impact allocated memory block's alignment.
>>>>    * In these cases, the size (i.e nr_pages) and the alignment of the allocated
>>>>    * memory will be different. This problem does not exist when nr_pages is power
>>>>    * of two where the size and the alignment of the allocated memory will always
>>>>    * be nr_pages.
>>>
>>> I dunno, it sounds more complicated than really necessary IMHO. Callers
>>> shouldn't really be bothered by memory blocks and other really deep
>>> implementation details.. Wouldn't be the below sufficient?
>>>
>>> The allocated memory is always aligned to a page boundary. If nr_pages
>>> is a power of two then the alignement is guaranteed to be to the given
>>
>> s/alignement/alignment/
>>
>> and "the PFN is guaranteed to be aligned to nr_pages" (the address is
>> aligned to nr_pages*PAGE_SIZE)
> 
> thx for the correction.
> 
>>> nr_pages (e.g. 1GB request would be aligned to 1GB).
>>>
>>
>> I'd probably add "This function will miss allocation opportunities if
>> nr_pages is not a power of two (and the implicit alignment is bogus)."
> 
> This is again an implementation detail and quite a confusing one to
> whoever not familiar with the MM internals. And to be fair even a proper
> alignment doesn't give you any stronger guarantee as long as the
> allocation operates on non movable zones anyway.
> 

To be honest, I'd not suggest to anyone to use this function with 
nr_pages not being a power of two, and I already explained why. I prefer 
to spill that out than having people complain afterwards. Yes it's an 
implementation detail users should be aware of until reworked.

But I think we talked about this here for way too long, so I am fine 
with either.

-- 

Thanks,

David / dhildenb