linux-kernel - Re: [PATCH] mm: Exclude HugeTLB pages from THP page

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20160330092448.GA19367@e103986-lin>
Date:	Wed, 30 Mar 2016 10:24:48 +0100
From:	Steve Capper <steve.capper@....com>
To:	"Kirill A. Shutemov" <kirill@...temov.name>
Cc:	Steve Capper <steve.capper@....com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, will.deacon@....com,
	dwoods@...lanox.com, mhocko@...e.com, mingo@...nel.org,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Subject: Re: [PATCH] mm: Exclude HugeTLB pages from THP page_mapped logic

On Tue, Mar 29, 2016 at 07:51:49PM +0300, Kirill A. Shutemov wrote:
> On Tue, Mar 29, 2016 at 05:39:41PM +0100, Steve Capper wrote:
> > HugeTLB pages cannot be split, thus use the compound_mapcount to
> > track rmaps.
> > 
> > Currently the page_mapped function will check the compound_mapcount, but
> > will also go through the constituent pages of a THP compound page and
> > query the individual _mapcount's too.
> > 
> > Unfortunately, the page_mapped function does not distinguish between
> > HugeTLB and THP compound pages and assumes that a compound page always
> > needs to have HPAGE_PMD_NR pages querying.
> > 
> > For most cases when dealing with HugeTLB this is just inefficient, but
> > for scenarios where the HugeTLB page size is less than the pmd block
> > size (e.g. when using contiguous bit on ARM) this can lead to crashes.
> > 
> > This patch adjusts the page_mapped function such that we skip the
> > unnecessary THP reference checks for HugeTLB pages.
> > 
> > Fixes: e1534ae95004 ("mm: differentiate page_mapped() from page_mapcount() for compound pages")
> > Cc: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> > Signed-off-by: Steve Capper <steve.capper@....com>
> 
> Acked-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>

Thanks!

> 
> > ---
> > 
> > Hi,
> > 
> > This patch is my approach to fixing a problem that unearthed with
> > HugeTLB pages on arm64. We ran with PAGE_SIZE=64KB and placed down 32
> > contiguous ptes to create 2MB HugeTLB pages. (We can provide hints to
> > the MMU that page table entries are contiguous thus larger TLB entries
> > can be used to represent them).
> > 
> > The PMD_SIZE was 512MB thus the old version of page_mapped would read
> > through too many struct pages and lead to BUGs.
> > 
> > Original problem reported here:
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-March/414657.html
> > 
> > Having examined the HugeTLB code, I understand that only the
> > compound_mapcount_ptr is used to track rmap presence so going through
> > the individual _mapcounts for HugeTLB pages is superfluous? Or should I
> > instead post a patch that changes hpage_nr_pages to use the compound
> > order?
> 
> I would not touch hpage_nr_page().
> 
> We probably need to introduce compound_nr_pages() or something to replace
> (1 << compound_order(page)) to be used independetely from thp/hugetlb
> pages.

Okay, I will stick with the approach in this patch. With HugeTLB we also
have hstate information to use.

> 
> > Also, for the sake of readability, would it be worth changing the
> > definition of PageTransHuge to refer to only THPs (not both HugeTLB
> > and THP)?
> 
> I don't think so.
> 
> That would have overhead, since we wound need to do function call inside
> PageTransHuge(). HugeTLB() is not inlinable.

Ahh, I hadn't considered that...

> 
> hugetlb deverges from rest of mm pretty early, so thp vs. hugetlb
> confusion is not that ofter. We just don't share enough codepath.

Thanks Kirill, agreed.

Cheers,
-- 
Steve