Date:	Tue, 2 Jun 2015 12:00:37 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	Vlastimil Babka <vbabka@...e.cz>
Cc:	Mel Gorman <mgorman@...e.de>, Johannes Weiner <hannes@...xchg.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Naoya Horiguchi <n-horiguchi@...jp.nec.com>,
	linux-mm@...ck.org, LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hugetlb: Do not account hugetlb pages as NR_FILE_PAGES

On Tue 02-06-15 11:38:05, Michal Hocko wrote:
> On Tue 02-06-15 11:33:05, Vlastimil Babka wrote:
> > On 06/02/2015 11:25 AM, Michal Hocko wrote:
[...]
> > >diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > >index 91b7f9b2b774..bb8a70e8fc77 100644
> > >--- a/include/linux/page-flags.h
> > >+++ b/include/linux/page-flags.h
> > >@@ -547,7 +547,13 @@ static inline void ClearPageCompound(struct page *page)
> > >  #endif /* !PAGEFLAGS_EXTENDED */
> > >
> > >  #ifdef CONFIG_HUGETLB_PAGE
> > >-int PageHuge(struct page *page);
> > >+int __PageHuge(struct page *page);
> > >+static inline int PageHuge(struct page *page)
> > >+{
> > >+	if (!PageCompound(page))
> > 
> > Perhaps the above as likely()?
> 
> I have added it already when writing the changelog.
> 
> > [...]
> > 
> > >-EXPORT_SYMBOL_GPL(PageHuge);
> > >+EXPORT_SYMBOL_GPL(__PageHuge);
> > >
> > >  /*
> > >   * PageHeadHuge() only returns true for hugetlbfs head page, but not for
> > >
> > 
> > Do the same thing here by inlining the PageHead() test?
> > I guess the page_to_pgoff and __compound_tail_refcounted callers are rather
> > hot?
> 
> Yes, that sounds like a good idea.

So the overal codesize (with defconfig) has still grown with the patch:
   text    data     bss     dec     hex filename
 443075   59217   25604  527896   80e18 mm/built-in.o.before
 443477   59217   25604  528298   80faa mm/built-in.o.PageHuge
 443653   59217   25604  528474   8105a mm/built-in.o.both

It is still not ~1K with the full inline but quite large on its own.
So I am not sure it makes sense to fiddle with this without actually
seeing some penalty in profiles.

Here is what I have if somebody wants to play with it:
---
>From a1d33be3ac209da1721864c2a5a1d43635c17444 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@...e.cz>
Date: Tue, 2 Jun 2015 11:25:48 +0200
Subject: [PATCH] hugetlb: split PageHuge into fast and slow path

Johannes Weiner has noted that PageHuge proliferates into relatively hot
paths like __add_to_page_cache_locked or __delete_from_page_cache and
that means that we unconditionally do a function call even though that
hugetlb pages in those path are rare wrt. to normal pages. Let's split
out PageHuge into two parts. Fast path will check for PageCompound()
which will get inlined and it would return false for the normal case
and __PageHuge which does the function call and peform the remaining
hugetlb specific checks.

PageHeadHuge is a similar case as pointed out by Vlastimil Babka. It is
called from page_to_pgoff which is then used in rmap walk. Do the
similar thing and check for PageHead inlined and do the rest via a
function call (__PageHeadHuge).

We could get rid of the function call completely because both __PageHuge
and __PageHeadHuge are trivial but we would have to export other hugetlb
internals and that would increase the generated code size by quite much.
This patch has a smaller codesize footprint:
 443075   59217   25604  527896   80e18 mm/built-in.o.before
 443653   59217   25604  528474   8105a mm/built-in.o.after

Signed-off-by: Michal Hocko <mhocko@...e.cz>
---
 include/linux/page-flags.h | 17 +++++++++++++++--
 mm/hugetlb.c               | 18 +++++++-----------
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 91b7f9b2b774..151fb6b4e95b 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -547,8 +547,21 @@ static inline void ClearPageCompound(struct page *page)
 #endif /* !PAGEFLAGS_EXTENDED */
 
 #ifdef CONFIG_HUGETLB_PAGE
-int PageHuge(struct page *page);
-int PageHeadHuge(struct page *page);
+int __PageHuge(struct page *page);
+static inline int PageHuge(struct page *page)
+{
+	if (likely(!PageCompound(page)))
+		return 0;
+	return __PageHuge(page);
+}
+
+int __PageHeadHuge(struct page *page);
+static inline int PageHeadHuge(struct page *page_head)
+{
+	if (likely(!PageHead(page_head)))
+		return 0;
+	return __PageHeadHuge(page_head);
+}
 bool page_huge_active(struct page *page);
 #else
 TESTPAGEFLAG_FALSE(Huge)
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 33defbe1897f..d28024c7ca73 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1107,29 +1107,25 @@ static void prep_compound_gigantic_page(struct page *page, unsigned long order)
 }
 
 /*
- * PageHuge() only returns true for hugetlbfs pages, but not for normal or
+ * __PageHuge() only returns true for hugetlbfs pages, but not for normal or
  * transparent huge pages.  See the PageTransHuge() documentation for more
  * details.
  */
-int PageHuge(struct page *page)
+int __PageHuge(struct page *page)
 {
-	if (!PageCompound(page))
-		return 0;
-
+	VM_BUG_ON(!PageCompound(page));
 	page = compound_head(page);
 	return get_compound_page_dtor(page) == free_huge_page;
 }
-EXPORT_SYMBOL_GPL(PageHuge);
+EXPORT_SYMBOL_GPL(__PageHuge);
 
 /*
- * PageHeadHuge() only returns true for hugetlbfs head page, but not for
+ * __PageHeadHuge() only returns true for hugetlbfs head page, but not for
  * normal or transparent huge pages.
  */
-int PageHeadHuge(struct page *page_head)
+int __PageHeadHuge(struct page *page_head)
 {
-	if (!PageHead(page_head))
-		return 0;
-
+	VM_BUG_ON(!PageHead(page_head));
 	return get_compound_page_dtor(page_head) == free_huge_page;
 }
 
-- 
2.1.4

-- 
Michal Hocko
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/

Powered by blists - more mailing lists