linux-kernel - Re: [PATCHv4 09/39] thp, mm: introduce mapping_can_have

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:	Wed, 22 May 2013 16:51:12 +0300 (EEST)
From:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
To:	Dave Hansen <dave@...1.net>
Cc:	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Hugh Dickins <hughd@...gle.com>,
	Wu Fengguang <fengguang.wu@...el.com>, Jan Kara <jack@...e.cz>,
	Mel Gorman <mgorman@...e.de>, linux-mm@...ck.org,
	Andi Kleen <ak@...ux.intel.com>,
	Matthew Wilcox <matthew.r.wilcox@...el.com>,
	"Kirill A. Shutemov" <kirill@...temov.name>,
	Hillf Danton <dhillf@...il.com>, linux-fsdevel@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCHv4 09/39] thp, mm: introduce mapping_can_have_hugepages()
 predicate

Dave Hansen wrote:
> On 05/11/2013 06:23 PM, Kirill A. Shutemov wrote:
> > From: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
> > 
> > Returns true if mapping can have huge pages. Just check for __GFP_COMP
> > in gfp mask of the mapping for now.
> > 
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@...ux.intel.com>
> > ---
> >  include/linux/pagemap.h |   12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> > index e3dea75..28597ec 100644
> > --- a/include/linux/pagemap.h
> > +++ b/include/linux/pagemap.h
> > @@ -84,6 +84,18 @@ static inline void mapping_set_gfp_mask(struct address_space *m, gfp_t mask)
> >  				(__force unsigned long)mask;
> >  }
> >  
> > +static inline bool mapping_can_have_hugepages(struct address_space *m)
> > +{
> > +	if (IS_ENABLED(CONFIG_TRANSPARENT_HUGEPAGE_PAGECACHE)) {
> > +		gfp_t gfp_mask = mapping_gfp_mask(m);
> > +		/* __GFP_COMP is key part of GFP_TRANSHUGE */
> > +		return !!(gfp_mask & __GFP_COMP) &&
> > +			transparent_hugepage_pagecache();
> > +	}
> > +
> > +	return false;
> > +}
> 
> transparent_hugepage_pagecache() already has the same IS_ENABLED()
> check,  Is it really necessary to do it again here?
> 
> IOW, can you do this?
> 
> > +static inline bool mapping_can_have_hugepages(struct address_space
> > +{
> > +		gfp_t gfp_mask = mapping_gfp_mask(m);
> 		if (!transparent_hugepage_pagecache())
> 			return false;
> > +		/* __GFP_COMP is key part of GFP_TRANSHUGE */
> > +		return !!(gfp_mask & __GFP_COMP);
> > +}

Yeah, it's better.

> I know we talked about this in the past, but I've forgotten already.
> Why is this checking for __GFP_COMP instead of GFP_TRANSHUGE?

It's up to filesystem what gfp mask to use. For example ramfs's pages are
not movable currently. So, we check only part which matters.

> Please flesh out the comment.

I'll make the comment in code a bit more descriptive.

> Also, what happens if "transparent_hugepage_flags &
> (1<<TRANSPARENT_HUGEPAGE_PAGECACHE)" becomes false at runtime and you
> have some already-instantiated huge page cache mappings around?  Will
> things like mapping_align_mask() break?

We will not touch existing huge pages in existing VMAs. The userspace can
use them until they will be unmapped or split. It's consistent with anon
THP pages.

If anybody mmap() the file after disabling the feature, we will not
setup huge pages anymore: transparent_hugepage_enabled() check in
handle_mm_fault will fail and the page fill be split.

mapping_align_mask() is part of mmap() call path, so there's only chance
that we will get VMA aligned more strictly then needed. Nothing to worry
about.

-- 
 Kirill A. Shutemov
--
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/