linux-kernel - Re: [PATCH 01/28] mm, page_alloc: Only check PageCompound for high-order pages

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]

Message-ID: <20160426103334.GB2858@techsingularity.net>
Date:	Tue, 26 Apr 2016 11:33:34 +0100
From:	Mel Gorman <mgorman@...hsingularity.net>
To:	Vlastimil Babka <vbabka@...e.cz>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Jesper Dangaard Brouer <brouer@...hat.com>,
	Linux-MM <linux-mm@...ck.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 01/28] mm, page_alloc: Only check PageCompound for
 high-order pages

On Mon, Apr 25, 2016 at 11:33:15AM +0200, Vlastimil Babka wrote:
> On 04/15/2016 10:58 AM, Mel Gorman wrote:
> >order-0 pages by definition cannot be compound so avoid the check in the
> >fast path for those pages.
> >
> >Signed-off-by: Mel Gorman <mgorman@...hsingularity.net>
> 
> Acked-by: Vlastimil Babka <vbabka@...e.cz>
> 
> Suggestion to improve below:
> 
> >---
> >  mm/page_alloc.c | 25 +++++++++++++++++--------
> >  1 file changed, 17 insertions(+), 8 deletions(-)
> >
> >diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> >index 59de90d5d3a3..5d205bcfe10d 100644
> >--- a/mm/page_alloc.c
> >+++ b/mm/page_alloc.c
> >@@ -1024,24 +1024,33 @@ void __meminit reserve_bootmem_region(unsigned long start, unsigned long end)
> >
> >  static bool free_pages_prepare(struct page *page, unsigned int order)
> >  {
> >-	bool compound = PageCompound(page);
> >-	int i, bad = 0;
> >+	int bad = 0;
> >
> >  	VM_BUG_ON_PAGE(PageTail(page), page);
> >-	VM_BUG_ON_PAGE(compound && compound_order(page) != order, page);
> >
> >  	trace_mm_page_free(page, order);
> >  	kmemcheck_free_shadow(page, order);
> >  	kasan_free_pages(page, order);
> >
> >+	/*
> >+	 * Check tail pages before head page information is cleared to
> >+	 * avoid checking PageCompound for order-0 pages.
> >+	 */
> >+	if (order) {
> 
> Sticking unlikely() here results in:
> 
> add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-30 (-30)
> function                                     old     new   delta
> free_pages_prepare                           771     741     -30
> 
> And from brief comparison of disassembly it really seems it's moved the
> compound handling towards the end of the function, which should be nicer for
> the instruction cache, branch prediction etc. And since this series is about
> microoptimization, I think the extra step is worth it.
> 

I dithered on this a bit and could not convince myself that the order
case really is unlikely. It depends on the situation as we could be
tearing down a large THP-backed mapping. SLUB is also using compound
pages so it's both workload and configuration dependent whether this
path is really likely or not.

-- 
Mel Gorman
SUSE Labs