* [PATCH] pagemap: fix wrong KPF_THP on slab pages @ 2012-09-25 13:56 Naoya Horiguchi 2012-09-25 15:59 ` KOSAKI Motohiro 0 siblings, 1 reply; 12+ messages in thread From: Naoya Horiguchi @ 2012-09-25 13:56 UTC (permalink / raw) To: Wu Fengguang, Andrew Morton Cc: KOSAKI Motohiro, Andi Kleen, linux-mm, linux-kernel KPF_THP can be set on non-huge compound pages like slab pages, because PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug and breaks user space applications which look for thp via /proc/kpageflags. Currently thp is constructed only on anonymous pages, so this patch makes KPF_THP be set when both of PageAnon and PageTransCompound are true. Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> --- fs/proc/page.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git v3.6-rc6.orig/fs/proc/page.c v3.6-rc6/fs/proc/page.c index 7fcd0d6..613102d 100644 --- v3.6-rc6.orig/fs/proc/page.c +++ v3.6-rc6/fs/proc/page.c @@ -115,7 +115,7 @@ u64 stable_page_flags(struct page *page) u |= 1 << KPF_COMPOUND_TAIL; if (PageHuge(page)) u |= 1 << KPF_HUGE; - else if (PageTransCompound(page)) + else if (PageTransCompound(page) && PageAnon(page)) u |= 1 << KPF_THP; /* -- 1.7.11.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-25 13:56 [PATCH] pagemap: fix wrong KPF_THP on slab pages Naoya Horiguchi @ 2012-09-25 15:59 ` KOSAKI Motohiro 2012-09-25 17:05 ` Naoya Horiguchi 0 siblings, 1 reply; 12+ messages in thread From: KOSAKI Motohiro @ 2012-09-25 15:59 UTC (permalink / raw) To: Naoya Horiguchi Cc: Wu Fengguang, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, Sep 25, 2012 at 9:56 AM, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote: > KPF_THP can be set on non-huge compound pages like slab pages, because > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > and breaks user space applications which look for thp via /proc/kpageflags. > Currently thp is constructed only on anonymous pages, so this patch makes > KPF_THP be set when both of PageAnon and PageTransCompound are true. Indeed. Please add some comment too. -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-25 15:59 ` KOSAKI Motohiro @ 2012-09-25 17:05 ` Naoya Horiguchi 2012-09-25 19:03 ` KOSAKI Motohiro 2012-09-26 0:20 ` David Rientjes 0 siblings, 2 replies; 12+ messages in thread From: Naoya Horiguchi @ 2012-09-25 17:05 UTC (permalink / raw) To: KOSAKI Motohiro Cc: Naoya Horiguchi, Wu Fengguang, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, Sep 25, 2012 at 11:59:51AM -0400, KOSAKI Motohiro wrote: > On Tue, Sep 25, 2012 at 9:56 AM, Naoya Horiguchi > <n-horiguchi@ah.jp.nec.com> wrote: > > KPF_THP can be set on non-huge compound pages like slab pages, because > > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > > and breaks user space applications which look for thp via /proc/kpageflags. > > Currently thp is constructed only on anonymous pages, so this patch makes > > KPF_THP be set when both of PageAnon and PageTransCompound are true. > > Indeed. Please add some comment too. Sure. I send revised one. Thanks, Naoya --- From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Date: Mon, 24 Sep 2012 16:28:30 -0400 Subject: [PATCH v2] pagemap: fix wrong KPF_THP on slab pages KPF_THP can be set on non-huge compound pages like slab pages, because PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug and breaks user space applications which look for thp via /proc/kpageflags. Currently thp is constructed only on anonymous pages, so this patch makes KPF_THP be set when both of PageAnon and PageTransCompound are true. Changelog in v2: - add a comment in code Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> --- fs/proc/page.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/proc/page.c b/fs/proc/page.c index 7fcd0d6..f7cd2f6c 100644 --- a/fs/proc/page.c +++ b/fs/proc/page.c @@ -115,7 +115,12 @@ u64 stable_page_flags(struct page *page) u |= 1 << KPF_COMPOUND_TAIL; if (PageHuge(page)) u |= 1 << KPF_HUGE; - else if (PageTransCompound(page)) + /* + * Since THP is relevant only for anonymous pages so far, we check it + * explicitly with PageAnon. Otherwise thp is confounded with non-huge + * compound pages like slab pages. + */ + else if (PageTransCompound(page) && PageAnon(page)) u |= 1 << KPF_THP; /* -- 1.7.11.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-25 17:05 ` Naoya Horiguchi @ 2012-09-25 19:03 ` KOSAKI Motohiro 2012-09-26 0:20 ` David Rientjes 1 sibling, 0 replies; 12+ messages in thread From: KOSAKI Motohiro @ 2012-09-25 19:03 UTC (permalink / raw) To: Naoya Horiguchi Cc: Wu Fengguang, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, Sep 25, 2012 at 1:05 PM, Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> wrote: > On Tue, Sep 25, 2012 at 11:59:51AM -0400, KOSAKI Motohiro wrote: >> On Tue, Sep 25, 2012 at 9:56 AM, Naoya Horiguchi >> <n-horiguchi@ah.jp.nec.com> wrote: >> > KPF_THP can be set on non-huge compound pages like slab pages, because >> > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug >> > and breaks user space applications which look for thp via /proc/kpageflags. >> > Currently thp is constructed only on anonymous pages, so this patch makes >> > KPF_THP be set when both of PageAnon and PageTransCompound are true. >> >> Indeed. Please add some comment too. > > Sure. I send revised one. > > Thanks, > Naoya > --- > From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > Date: Mon, 24 Sep 2012 16:28:30 -0400 > Subject: [PATCH v2] pagemap: fix wrong KPF_THP on slab pages > > KPF_THP can be set on non-huge compound pages like slab pages, because > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > and breaks user space applications which look for thp via /proc/kpageflags. > Currently thp is constructed only on anonymous pages, so this patch makes > KPF_THP be set when both of PageAnon and PageTransCompound are true. > > Changelog in v2: > - add a comment in code > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > --- > fs/proc/page.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/proc/page.c b/fs/proc/page.c > index 7fcd0d6..f7cd2f6c 100644 > --- a/fs/proc/page.c > +++ b/fs/proc/page.c > @@ -115,7 +115,12 @@ u64 stable_page_flags(struct page *page) > u |= 1 << KPF_COMPOUND_TAIL; > if (PageHuge(page)) > u |= 1 << KPF_HUGE; > - else if (PageTransCompound(page)) > + /* > + * Since THP is relevant only for anonymous pages so far, we check it > + * explicitly with PageAnon. Otherwise thp is confounded with non-huge > + * compound pages like slab pages. > + */ > + else if (PageTransCompound(page) && PageAnon(page)) > u |= 1 << KPF_THP; Looks good to me. Acked-by: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-25 17:05 ` Naoya Horiguchi 2012-09-25 19:03 ` KOSAKI Motohiro @ 2012-09-26 0:20 ` David Rientjes 2012-09-26 2:06 ` Naoya Horiguchi 1 sibling, 1 reply; 12+ messages in thread From: David Rientjes @ 2012-09-26 0:20 UTC (permalink / raw) To: Naoya Horiguchi Cc: KOSAKI Motohiro, Wu Fengguang, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, 25 Sep 2012, Naoya Horiguchi wrote: > KPF_THP can be set on non-huge compound pages like slab pages, because > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > and breaks user space applications which look for thp via /proc/kpageflags. > Currently thp is constructed only on anonymous pages, so this patch makes > KPF_THP be set when both of PageAnon and PageTransCompound are true. > > Changelog in v2: > - add a comment in code > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Wouldn't PageTransCompound(page) && !PageHuge(page) && !PageSlab(page) be better for a future extension of thp support? -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 0:20 ` David Rientjes @ 2012-09-26 2:06 ` Naoya Horiguchi 2012-09-26 2:26 ` David Rientjes 2012-09-26 2:47 ` Fengguang Wu 0 siblings, 2 replies; 12+ messages in thread From: Naoya Horiguchi @ 2012-09-26 2:06 UTC (permalink / raw) To: David Rientjes Cc: Naoya Horiguchi, KOSAKI Motohiro, Wu Fengguang, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, Sep 25, 2012 at 05:20:48PM -0700, David Rientjes wrote: > On Tue, 25 Sep 2012, Naoya Horiguchi wrote: > > > KPF_THP can be set on non-huge compound pages like slab pages, because > > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > > and breaks user space applications which look for thp via /proc/kpageflags. > > Currently thp is constructed only on anonymous pages, so this patch makes > > KPF_THP be set when both of PageAnon and PageTransCompound are true. > > > > Changelog in v2: > > - add a comment in code > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > Wouldn't PageTransCompound(page) && !PageHuge(page) && !PageSlab(page) be > better for a future extension of thp support? Yes, this saves us an additional change when thp starts handling pagecaches. Andrew, can you replace the previous version in -mm tree with new one below? Thanks, Naoya --- From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Date: Tue, 25 Sep 2012 21:30:25 -0400 Subject: [PATCH v3] kpageflags: fix wrong KPF_THP on slab pages KPF_THP can be set on non-huge compound pages like slab pages, because PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug and breaks user space applications which look for thp via /proc/kpageflags. This patch rules out setting KPF_THP wrongly by additional PageSlab check. Changelog in v3: - check PageSlab instead of PageAnon - fix patch subject Changelog in v2: - add a comment in code Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> --- fs/proc/page.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/proc/page.c b/fs/proc/page.c index 7fcd0d6..e36d1f3 100644 --- a/fs/proc/page.c +++ b/fs/proc/page.c @@ -115,7 +115,12 @@ u64 stable_page_flags(struct page *page) u |= 1 << KPF_COMPOUND_TAIL; if (PageHuge(page)) u |= 1 << KPF_HUGE; - else if (PageTransCompound(page)) + /* + * PageTransCompound can be true for slab pages because it just sees + * PG_head/PG_head, so we need to check PageSlab to make sure the given + * page is a thp, not a non-huge compound page. + */ + else if (PageTransCompound(page) && !PageSlab(page)) u |= 1 << KPF_THP; /* -- 1.7.11.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 2:06 ` Naoya Horiguchi @ 2012-09-26 2:26 ` David Rientjes 2012-09-26 2:47 ` Fengguang Wu 1 sibling, 0 replies; 12+ messages in thread From: David Rientjes @ 2012-09-26 2:26 UTC (permalink / raw) To: Naoya Horiguchi Cc: KOSAKI Motohiro, Wu Fengguang, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, 25 Sep 2012, Naoya Horiguchi wrote: > KPF_THP can be set on non-huge compound pages like slab pages, because > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > and breaks user space applications which look for thp via /proc/kpageflags. > This patch rules out setting KPF_THP wrongly by additional PageSlab check. > > Changelog in v3: > - check PageSlab instead of PageAnon > - fix patch subject > > Changelog in v2: > - add a comment in code > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> Acked-by: David Rientjes <rientjes@google.com> -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 2:06 ` Naoya Horiguchi 2012-09-26 2:26 ` David Rientjes @ 2012-09-26 2:47 ` Fengguang Wu 2012-09-26 4:02 ` Naoya Horiguchi 1 sibling, 1 reply; 12+ messages in thread From: Fengguang Wu @ 2012-09-26 2:47 UTC (permalink / raw) To: Naoya Horiguchi Cc: David Rientjes, KOSAKI Motohiro, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Tue, Sep 25, 2012 at 10:06:08PM -0400, Naoya Horiguchi wrote: > On Tue, Sep 25, 2012 at 05:20:48PM -0700, David Rientjes wrote: > > On Tue, 25 Sep 2012, Naoya Horiguchi wrote: > > > > > KPF_THP can be set on non-huge compound pages like slab pages, because > > > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > > > and breaks user space applications which look for thp via /proc/kpageflags. > > > Currently thp is constructed only on anonymous pages, so this patch makes > > > KPF_THP be set when both of PageAnon and PageTransCompound are true. > > > > > > Changelog in v2: > > > - add a comment in code > > > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > > > Wouldn't PageTransCompound(page) && !PageHuge(page) && !PageSlab(page) be > > better for a future extension of thp support? > > Yes, this saves us an additional change when thp starts handling pagecaches. > Andrew, can you replace the previous version in -mm tree with new one below? > > Thanks, > Naoya > --- > From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > Date: Tue, 25 Sep 2012 21:30:25 -0400 > Subject: [PATCH v3] kpageflags: fix wrong KPF_THP on slab pages > > KPF_THP can be set on non-huge compound pages like slab pages, because > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug s/sees/checks/ > and breaks user space applications which look for thp via /proc/kpageflags. > This patch rules out setting KPF_THP wrongly by additional PageSlab check. > > Changelog in v3: > - check PageSlab instead of PageAnon > - fix patch subject > > Changelog in v2: > - add a comment in code > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > --- > fs/proc/page.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/proc/page.c b/fs/proc/page.c > index 7fcd0d6..e36d1f3 100644 > --- a/fs/proc/page.c > +++ b/fs/proc/page.c > @@ -115,7 +115,12 @@ u64 stable_page_flags(struct page *page) > u |= 1 << KPF_COMPOUND_TAIL; > if (PageHuge(page)) > u |= 1 << KPF_HUGE; > - else if (PageTransCompound(page)) > + /* > + * PageTransCompound can be true for slab pages because it just sees s/sees/checks/ > + * PG_head/PG_head, so we need to check PageSlab to make sure the given PG_head/PG_head should be PG_head/PG_tail. > + * page is a thp, not a non-huge compound page. > + */ > + else if (PageTransCompound(page) && !PageSlab(page)) > u |= 1 << KPF_THP; Good catch! Will this report THP for the various drivers that do __GFP_COMP page allocations? Thanks, Fengguang -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 2:47 ` Fengguang Wu @ 2012-09-26 4:02 ` Naoya Horiguchi 2012-09-26 6:06 ` Naoya Horiguchi 0 siblings, 1 reply; 12+ messages in thread From: Naoya Horiguchi @ 2012-09-26 4:02 UTC (permalink / raw) To: Wu Fengguang Cc: Naoya Horiguchi, David Rientjes, KOSAKI Motohiro, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Wed, Sep 26, 2012 at 10:47:54AM +0800, Fengguang Wu wrote: > On Tue, Sep 25, 2012 at 10:06:08PM -0400, Naoya Horiguchi wrote: > > On Tue, Sep 25, 2012 at 05:20:48PM -0700, David Rientjes wrote: > > > On Tue, 25 Sep 2012, Naoya Horiguchi wrote: > > > > > > > KPF_THP can be set on non-huge compound pages like slab pages, because > > > > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > > > > and breaks user space applications which look for thp via /proc/kpageflags. > > > > Currently thp is constructed only on anonymous pages, so this patch makes > > > > KPF_THP be set when both of PageAnon and PageTransCompound are true. > > > > > > > > Changelog in v2: > > > > - add a comment in code > > > > > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > > > > > Wouldn't PageTransCompound(page) && !PageHuge(page) && !PageSlab(page) be > > > better for a future extension of thp support? > > > > Yes, this saves us an additional change when thp starts handling pagecaches. > > Andrew, can you replace the previous version in -mm tree with new one below? > > > > Thanks, > > Naoya > > --- > > From: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > Date: Tue, 25 Sep 2012 21:30:25 -0400 > > Subject: [PATCH v3] kpageflags: fix wrong KPF_THP on slab pages > > > > KPF_THP can be set on non-huge compound pages like slab pages, because > > PageTransCompound only sees PG_head and PG_tail. Obviously this is a bug > > s/sees/checks/ > > > and breaks user space applications which look for thp via /proc/kpageflags. > > This patch rules out setting KPF_THP wrongly by additional PageSlab check. > > > > Changelog in v3: > > - check PageSlab instead of PageAnon > > - fix patch subject > > > > Changelog in v2: > > - add a comment in code > > > > Signed-off-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com> > > --- > > fs/proc/page.c | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/fs/proc/page.c b/fs/proc/page.c > > index 7fcd0d6..e36d1f3 100644 > > --- a/fs/proc/page.c > > +++ b/fs/proc/page.c > > @@ -115,7 +115,12 @@ u64 stable_page_flags(struct page *page) > > u |= 1 << KPF_COMPOUND_TAIL; > > if (PageHuge(page)) > > u |= 1 << KPF_HUGE; > > - else if (PageTransCompound(page)) > > + /* > > + * PageTransCompound can be true for slab pages because it just sees > > s/sees/checks/ > > > + * PG_head/PG_head, so we need to check PageSlab to make sure the given > > PG_head/PG_head should be PG_head/PG_tail. Ah, sorry for my carelessness. > > + * page is a thp, not a non-huge compound page. > > + */ > > + else if (PageTransCompound(page) && !PageSlab(page)) > > u |= 1 << KPF_THP; > > Good catch! > > Will this report THP for the various drivers that do __GFP_COMP > page allocations? I'm afraid it will. I think of checking PageLRU as an alternative, but it needs compound_head() to report tail pages correctly. In this context, pages are not pinned or locked, so it's unsafe to use compound_head() because it can return a dangling pointer. Maybe it's a thp's/hugetlbfs's (not kpageflags specific) problem, so going forward with compound_head() expecting that it will be fixed in the future work can be an option. Thanks, Naoya -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 4:02 ` Naoya Horiguchi @ 2012-09-26 6:06 ` Naoya Horiguchi 2012-09-26 7:38 ` Fengguang Wu 0 siblings, 1 reply; 12+ messages in thread From: Naoya Horiguchi @ 2012-09-26 6:06 UTC (permalink / raw) To: Naoya Horiguchi Cc: Wu Fengguang, David Rientjes, KOSAKI Motohiro, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Wed, Sep 26, 2012 at 12:02:34AM -0400, Naoya Horiguchi wrote: ... > > > + * page is a thp, not a non-huge compound page. > > > + */ > > > + else if (PageTransCompound(page) && !PageSlab(page)) > > > u |= 1 << KPF_THP; > > > > Good catch! > > > > Will this report THP for the various drivers that do __GFP_COMP > > page allocations? > > I'm afraid it will. I think of checking PageLRU as an alternative, > but it needs compound_head() to report tail pages correctly. > In this context, pages are not pinned or locked, so it's unsafe to > use compound_head() because it can return a dangling pointer. > Maybe it's a thp's/hugetlbfs's (not kpageflags specific) problem, > so going forward with compound_head() expecting that it will be > fixed in the future work can be an option. It seems that compound_trans_head() solves this problem, so I'll simply use it. Naoya -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 6:06 ` Naoya Horiguchi @ 2012-09-26 7:38 ` Fengguang Wu 2012-09-26 14:42 ` Naoya Horiguchi 0 siblings, 1 reply; 12+ messages in thread From: Fengguang Wu @ 2012-09-26 7:38 UTC (permalink / raw) To: Naoya Horiguchi Cc: David Rientjes, KOSAKI Motohiro, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Wed, Sep 26, 2012 at 02:06:08AM -0400, Naoya Horiguchi wrote: > On Wed, Sep 26, 2012 at 12:02:34AM -0400, Naoya Horiguchi wrote: > ... > > > > + * page is a thp, not a non-huge compound page. > > > > + */ > > > > + else if (PageTransCompound(page) && !PageSlab(page)) > > > > u |= 1 << KPF_THP; > > > > > > Good catch! > > > > > > Will this report THP for the various drivers that do __GFP_COMP > > > page allocations? > > > > I'm afraid it will. I think of checking PageLRU as an alternative, > > but it needs compound_head() to report tail pages correctly. > > In this context, pages are not pinned or locked, so it's unsafe to > > use compound_head() because it can return a dangling pointer. > > Maybe it's a thp's/hugetlbfs's (not kpageflags specific) problem, > > so going forward with compound_head() expecting that it will be > > fixed in the future work can be an option. > > It seems that compound_trans_head() solves this problem, so I'll > simply use it. Naoya, in fact I didn't quite catch your concerns. Why not just test PageTransCompound(page) && PageLRU(page) The whole page flag report thing is inherently racy and it's fine to report wrong values due to races. The "__GFP_COMP reported as THP", however, should be avoided because it will make consistent wrong reporting of page flags. Thanks, Fengguang -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] pagemap: fix wrong KPF_THP on slab pages 2012-09-26 7:38 ` Fengguang Wu @ 2012-09-26 14:42 ` Naoya Horiguchi 0 siblings, 0 replies; 12+ messages in thread From: Naoya Horiguchi @ 2012-09-26 14:42 UTC (permalink / raw) To: Wu Fengguang Cc: Naoya Horiguchi, David Rientjes, KOSAKI Motohiro, Andrew Morton, Andi Kleen, linux-mm, linux-kernel On Wed, Sep 26, 2012 at 03:38:41PM +0800, Fengguang Wu wrote: > On Wed, Sep 26, 2012 at 02:06:08AM -0400, Naoya Horiguchi wrote: > > On Wed, Sep 26, 2012 at 12:02:34AM -0400, Naoya Horiguchi wrote: > > ... > > > > > + * page is a thp, not a non-huge compound page. > > > > > + */ > > > > > + else if (PageTransCompound(page) && !PageSlab(page)) > > > > > u |= 1 << KPF_THP; > > > > > > > > Good catch! > > > > > > > > Will this report THP for the various drivers that do __GFP_COMP > > > > page allocations? > > > > > > I'm afraid it will. I think of checking PageLRU as an alternative, > > > but it needs compound_head() to report tail pages correctly. > > > In this context, pages are not pinned or locked, so it's unsafe to > > > use compound_head() because it can return a dangling pointer. > > > Maybe it's a thp's/hugetlbfs's (not kpageflags specific) problem, > > > so going forward with compound_head() expecting that it will be > > > fixed in the future work can be an option. > > > > It seems that compound_trans_head() solves this problem, so I'll > > simply use it. > > Naoya, in fact I didn't quite catch your concerns. Why not just test > > PageTransCompound(page) && PageLRU(page) If we simply check PageLRU, tail pages in thp only show KPF_COMPOUND_TAIL and we can't distinguish them from tail pages in non-huge compound pages. Moreover this behavior is not consistent with that of hugetlbfs tail pages where tail pages also have KPF_HUGE and are distinct from non-huge compound pages. I show the output of page-types: offset len flags ... 2d400 1 ___U_lA____Ma_bH______t____________ (thp head) 2d401 1ff ________________T__________________ (thp tail) # no KPF_THP ... 77000 1 ___U_______Ma__H_G_________________ (hugetlbfs head) 77001 1ff ________________TG_________________ (hugetlbfs tail) ... 11fb50 1 _______________H___________________ (compound head) 11fb51 3 ________________T__________________ (compound tail) ... 11fb58 1 _______S_______H___________________ (slab head) 11fb59 7 ________________T__________________ (slab tail) H: KPF_COMPOUND_HEAD T: KPF_COMPOUND_TAIL G: KPF_HUGE t: KPF_THP So I think it's better to set KPF_THP on thp tail pages. > The whole page flag report thing is inherently racy and it's fine to > report wrong values due to races. The "__GFP_COMP reported as THP", > however, should be avoided because it will make consistent wrong > reporting of page flags. Yes, I agree with this point. Thanks, Naoya -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2012-09-26 14:43 UTC | newest] Thread overview: 12+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-25 13:56 [PATCH] pagemap: fix wrong KPF_THP on slab pages Naoya Horiguchi 2012-09-25 15:59 ` KOSAKI Motohiro 2012-09-25 17:05 ` Naoya Horiguchi 2012-09-25 19:03 ` KOSAKI Motohiro 2012-09-26 0:20 ` David Rientjes 2012-09-26 2:06 ` Naoya Horiguchi 2012-09-26 2:26 ` David Rientjes 2012-09-26 2:47 ` Fengguang Wu 2012-09-26 4:02 ` Naoya Horiguchi 2012-09-26 6:06 ` Naoya Horiguchi 2012-09-26 7:38 ` Fengguang Wu 2012-09-26 14:42 ` Naoya Horiguchi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).