linux-kernel - Re: [RFC PATCH V2 0/4] Reducing parameters of alloc

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: <54856213.9070909@suse.cz>
Date:	Mon, 08 Dec 2014 09:32:19 +0100
From:	Vlastimil Babka <vbabka@...e.cz>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
CC:	linux-mm <linux-mm@...ck.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Minchan Kim <minchan@...nel.org>, Mel Gorman <mgorman@...e.de>,
	Rik van Riel <riel@...hat.com>,
	David Rientjes <rientjes@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>
Subject: Re: [RFC PATCH V2 0/4] Reducing parameters of alloc_pages* family
 of functions

On 12/06/2014 02:07 AM, Linus Torvalds wrote:
> On Fri, Dec 5, 2014 at 11:59 AM, Vlastimil Babka <vbabka@...e.cz> wrote:
>> Hey all,
>>
>> this is a V2 of attempting something that has been discussed when Minchan
>> proposed to expand the x86 kernel stack [1], namely the reduction of huge
>> number of parameters that the alloc_pages* family and get_page_from_freelist()
>> functions have.
>
> So I generally like this, but looking at that "struct alloc_context",
> one member kind of stands out: the "order" parameter doesn't fit in
> with all the other members.
>
> Most everything else is describing where or what kind of pages to work
> with. The "order" in contrast, really is separate.
>
> So conceptually, my reaction is that it looks like a good cleanup even
> aside from the code/stack size reduction, but that the alloc_context
> definition is a bit odd.
>
> Quite frankly, I think the :"order" really fits much more closely with
> "alloc_flags", not with the alloc_context. Because like alloc_flags,.
> it really describes how we need to allocate things within the context,
> I'd argue.
>
> In fact, I think that the order could actually be packed with the
> alloc_flags in a single register, even on 32-bit (using a single-word
> structure, perhaps). If we really care about number of parameters.
>
> I'd rather go for "makes conceptual sense" over "packs order in
> because it kind of works" and we don't modify it".
>
> Hmm?

Thanks for the suggestions, order indeed stands out. I'll check if it 
makes more sense to have it separately, or pack as you suggest. Packing
could perhaps bring more complexity than the benefit of less parameters. 
But the suggestion made me realize that migratetype could be also packed 
into alloc_flags and it would be more straightforward for that than order.

With order and migratetype out, everything left in alloc_context would 
be about nodes and zones, which is also good I guess. Maybe a different 
name for the structure then?

Vlastimil

>
>                         Linus
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/