linux-kernel - Re: [PATCH] mm: don't invoke __alloc_pages_direct

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 10 Jul 2012 11:47:22 +0100
From:	Mel Gorman <mgorman@...e.de>
To:	JoonSoo Kim <js1304@...il.com>
Cc:	David Rientjes <rientjes@...gle.com>, akpm@...ux-foundation.org,
	Pekka Enberg <penberg@...nel.org>,
	Christoph Lameter <cl@...ux-foundation.org>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH] mm: don't invoke __alloc_pages_direct_compact when order
 0

On Sun, Jul 08, 2012 at 11:33:14AM +0900, JoonSoo Kim wrote:
> 2012/7/7 David Rientjes <rientjes@...gle.com>:
> > On Sat, 7 Jul 2012, Joonsoo Kim wrote:
> >
> >> __alloc_pages_direct_compact has many arguments so invoking it is very costly.
> >> And in almost invoking case, order is 0, so return immediately.
> >>
> >
> > If "zero cost" is "very costly", then this might make sense.
> >
> > __alloc_pages_direct_compact() is inlined by gcc.
> 
> In my kernel image, __alloc_pages_direct_compact() is not inlined by gcc.

Indeed it is due to their being two callsites. In most cases, the page
allocator takes care that functions have only one callsite so they get
inlined.

You say that invoking the function is very costly. I agree that a function
call with that many parameters is hefty but it is also in the slow path of
the allocator. For order-0 allocations we are about to enter direct reclaim
where I would expect the cost far exceeds the cost of a function call.

If the cost is indeed high and you have seen this in profiles then I
suggest you create a forced inline function alloc_pages_direct_compact
that does this;

if (order != 0)
	__alloc_pages_direct_compact(...)

and then call alloc_pages_direct_compact instead of
__alloc_pages_direct_compact. After that, recheck the profiles (although I
expect the difference to be marginal) and the size of vmlinux (if it gets
bigger, it's probably not worth it).

That would be functionally similar to your patch but it will preserve git
blame, churn less code and be harder to make mistakes with in the unlikely
event a third call to alloc_pages_direct_compact is ever added.

Thanks.

-- 
Mel Gorman
SUSE Labs
--
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/