2015-04-09 16:12:34

by Gerald Schaefer

[permalink] [raw]
Subject: [PATCH] mm/hugetlb: use pmd_page() in follow_huge_pmd()

commit 61f77eda "mm/hugetlb: reduce arch dependent code around follow_huge_*"
broke follow_huge_pmd() on s390, where pmd and pte layout differ and using
pte_page() on a huge pmd will return wrong results. Using pmd_page() instead
fixes this.

All architectures that were touched by commit 61f77eda have pmd_page()
defined, so this should not break anything on other architectures.

Signed-off-by: Gerald Schaefer <[email protected]>
Cc: [email protected] # v3.12
---
mm/hugetlb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index e8c92ae..271e443 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -3865,8 +3865,7 @@ retry:
if (!pmd_huge(*pmd))
goto out;
if (pmd_present(*pmd)) {
- page = pte_page(*(pte_t *)pmd) +
- ((address & ~PMD_MASK) >> PAGE_SHIFT);
+ page = pmd_page(*pmd) + ((address & ~PMD_MASK) >> PAGE_SHIFT);
if (flags & FOLL_GET)
get_page(page);
} else {
--
2.1.4


2015-04-09 19:41:52

by David Rientjes

[permalink] [raw]
Subject: Re: [PATCH] mm/hugetlb: use pmd_page() in follow_huge_pmd()

On Thu, 9 Apr 2015, Gerald Schaefer wrote:

> commit 61f77eda "mm/hugetlb: reduce arch dependent code around follow_huge_*"
> broke follow_huge_pmd() on s390, where pmd and pte layout differ and using
> pte_page() on a huge pmd will return wrong results. Using pmd_page() instead
> fixes this.
>
> All architectures that were touched by commit 61f77eda have pmd_page()
> defined, so this should not break anything on other architectures.
>
> Signed-off-by: Gerald Schaefer <[email protected]>
> Cc: [email protected] # v3.12

Acked-by: David Rientjes <[email protected]>

I'm not sure where the stable cc came from, though: commit 61f77eda makes
s390 use a generic version of follow_huge_pmd() and that generic version
is buggy for s930 because of commit e66f17ff7177 ("mm/hugetlb: take page
table lock in follow_huge_pmd()"). Both of those are 4.0 material,
though, so why is this needed for stable 3.12?

2015-04-10 00:10:23

by Naoya Horiguchi

[permalink] [raw]
Subject: Re: [PATCH] mm/hugetlb: use pmd_page() in follow_huge_pmd()

On Thu, Apr 09, 2015 at 06:11:35PM +0200, Gerald Schaefer wrote:
> commit 61f77eda "mm/hugetlb: reduce arch dependent code around follow_huge_*"
> broke follow_huge_pmd() on s390, where pmd and pte layout differ and using
> pte_page() on a huge pmd will return wrong results. Using pmd_page() instead
> fixes this.
>
> All architectures that were touched by commit 61f77eda have pmd_page()
> defined, so this should not break anything on other architectures.
>
> Signed-off-by: Gerald Schaefer <[email protected]>

Thank you for the report. This looks good to me.

Acked-by: Naoya Horiguchi <[email protected]>

> Cc: [email protected] # v3.12
> ---
> mm/hugetlb.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index e8c92ae..271e443 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -3865,8 +3865,7 @@ retry:
> if (!pmd_huge(*pmd))
> goto out;
> if (pmd_present(*pmd)) {
> - page = pte_page(*(pte_t *)pmd) +
> - ((address & ~PMD_MASK) >> PAGE_SHIFT);
> + page = pmd_page(*pmd) + ((address & ~PMD_MASK) >> PAGE_SHIFT);
> if (flags & FOLL_GET)
> get_page(page);
> } else {
> --
> 2.1.4
> -

2015-04-10 08:09:12

by Gerald Schaefer

[permalink] [raw]
Subject: Re: [PATCH] mm/hugetlb: use pmd_page() in follow_huge_pmd()

On Thu, 9 Apr 2015 12:41:47 -0700 (PDT)
David Rientjes <[email protected]> wrote:

> On Thu, 9 Apr 2015, Gerald Schaefer wrote:
>
> > commit 61f77eda "mm/hugetlb: reduce arch dependent code around
> > follow_huge_*" broke follow_huge_pmd() on s390, where pmd and pte
> > layout differ and using pte_page() on a huge pmd will return wrong
> > results. Using pmd_page() instead fixes this.
> >
> > All architectures that were touched by commit 61f77eda have
> > pmd_page() defined, so this should not break anything on other
> > architectures.
> >
> > Signed-off-by: Gerald Schaefer <[email protected]>
> > Cc: [email protected] # v3.12
>
> Acked-by: David Rientjes <[email protected]>
>
> I'm not sure where the stable cc came from, though: commit 61f77eda
> makes s390 use a generic version of follow_huge_pmd() and that
> generic version is buggy for s930 because of commit e66f17ff7177
> ("mm/hugetlb: take page table lock in follow_huge_pmd()"). Both of
> those are 4.0 material, though, so why is this needed for stable 3.12?

Both commits 61f77eda and e66f17ff already made it into the 3.12 stable
tree, probably because of SLES 12 (actually that's how I noticed them).

But I guess I screwed up the stable CC, [email protected].#.v3.12
somehow doesn't look right, not sure if the CC in the patch header
suffices. Looks like Jiri Slaby added the patches to 3.12, putting him
on CC now.

2015-04-10 20:39:04

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] mm/hugetlb: use pmd_page() in follow_huge_pmd()

On Fri, 10 Apr 2015 10:08:49 +0200 Gerald Schaefer <[email protected]> wrote:

> On Thu, 9 Apr 2015 12:41:47 -0700 (PDT)
> David Rientjes <[email protected]> wrote:
>
> > On Thu, 9 Apr 2015, Gerald Schaefer wrote:
> >
> > > commit 61f77eda "mm/hugetlb: reduce arch dependent code around
> > > follow_huge_*" broke follow_huge_pmd() on s390, where pmd and pte
> > > layout differ and using pte_page() on a huge pmd will return wrong
> > > results. Using pmd_page() instead fixes this.
> > >
> > > All architectures that were touched by commit 61f77eda have
> > > pmd_page() defined, so this should not break anything on other
> > > architectures.
> > >
> > > Signed-off-by: Gerald Schaefer <[email protected]>
> > > Cc: [email protected] # v3.12
> >
> > Acked-by: David Rientjes <[email protected]>
> >
> > I'm not sure where the stable cc came from, though: commit 61f77eda
> > makes s390 use a generic version of follow_huge_pmd() and that
> > generic version is buggy for s930 because of commit e66f17ff7177
> > ("mm/hugetlb: take page table lock in follow_huge_pmd()"). Both of
> > those are 4.0 material, though, so why is this needed for stable 3.12?
>
> Both commits 61f77eda and e66f17ff already made it into the 3.12 stable
> tree, probably because of SLES 12 (actually that's how I noticed them).
>
> But I guess I screwed up the stable CC, [email protected].#.v3.12
> somehow doesn't look right, not sure if the CC in the patch header
> suffices. Looks like Jiri Slaby added the patches to 3.12, putting him
> on CC now.

hm. I think I'll make it

Fixes: 61f77eda "mm/hugetlb: reduce arch dependent code around follow_huge_*"
...
Cc: <[email protected]>

There's enough info here for the various tree maintainers to work out
whether their kernel needs this fix.

2015-04-23 07:46:32

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] mm/hugetlb: use pmd_page() in follow_huge_pmd()

On 04/10/2015, 10:38 PM, Andrew Morton wrote:
> hm. I think I'll make it
>
> Fixes: 61f77eda "mm/hugetlb: reduce arch dependent code around follow_huge_*"

+1. This is the best tag to work with.

thanks,
--
js
suse labs