2013-09-30 07:58:59

by Joonsoo Kim

Subject: [PATCH] mm, hugetlb: correct missing private flag clearing

We should clear the page's private flag when returing the page to
the page allocator or the hugepage pool. This patch fixes it.

Signed-off-by: Joonsoo Kim <[email protected]>
---
Hello, Andrew.

I sent the new version of commit ('07443a8') before you did pull request,
but it isn't included. It may be losted :)
So I send this fix. IMO, this is good for v3.12.

Thanks.

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b49579c..691f226 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -653,6 +653,7 @@ static void free_huge_page(struct page *page)
BUG_ON(page_count(page));
BUG_ON(page_mapcount(page));
restore_reserve = PagePrivate(page);
+ ClearPagePrivate(page);

spin_lock(&hugetlb_lock);
hugetlb_cgroup_uncharge_page(hstate_index(h),
--
1.7.9.5

2013-09-30 21:35:18

by Andrew Morton

Subject: Re: [PATCH] mm, hugetlb: correct missing private flag clearing

On Mon, 30 Sep 2013 16:59:44 +0900 Joonsoo Kim <[email protected]> wrote:

> We should clear the page's private flag when returing the page to
> the page allocator or the hugepage pool. This patch fixes it.
>
> Signed-off-by: Joonsoo Kim <[email protected]>
> ---
> Hello, Andrew.
>
> I sent the new version of commit ('07443a8') before you did pull request,
> but it isn't included. It may be losted :)
> So I send this fix. IMO, this is good for v3.12.
>
> Thanks.
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index b49579c..691f226 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -653,6 +653,7 @@ static void free_huge_page(struct page *page)
> BUG_ON(page_count(page));
> BUG_ON(page_mapcount(page));
> restore_reserve = PagePrivate(page);
> + ClearPagePrivate(page);
>

You describe it as a fix, but what does it fix? IOW, what are the
user-visible effects of the change?

update_and_free_page() already clears PG_private, but afaict the bit
remains unaltered if free_huge_page() takes the enqueue_huge_page()
route.

2013-10-01 01:15:37

by Joonsoo Kim

Subject: Re: [PATCH] mm, hugetlb: correct missing private flag clearing

On Mon, Sep 30, 2013 at 02:35:14PM -0700, Andrew Morton wrote:
> On Mon, 30 Sep 2013 16:59:44 +0900 Joonsoo Kim <[email protected]> wrote:
>
> > We should clear the page's private flag when returing the page to
> > the page allocator or the hugepage pool. This patch fixes it.
> >
> > Signed-off-by: Joonsoo Kim <[email protected]>
> > ---
> > Hello, Andrew.
> >
> > I sent the new version of commit ('07443a8') before you did pull request,
> > but it isn't included. It may be losted :)
> > So I send this fix. IMO, this is good for v3.12.
> >
> > Thanks.
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index b49579c..691f226 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -653,6 +653,7 @@ static void free_huge_page(struct page *page)
> > BUG_ON(page_count(page));
> > BUG_ON(page_mapcount(page));
> > restore_reserve = PagePrivate(page);
> > + ClearPagePrivate(page);
> >
>
> You describe it as a fix, but what does it fix? IOW, what are the
> user-visible effects of the change?
>
> update_and_free_page() already clears PG_private, but afaict the bit
> remains unaltered if free_huge_page() takes the enqueue_huge_page()
> route.

Yes, you are right.
I attach another version having more explanation.
Please refer this and merge it.

Thanks.
------------------------->8---------------------------------
>From 9d2ead1800de6e1192824e11dafe68bed7b78cce Mon Sep 17 00:00:00 2001
From: Joonsoo Kim <[email protected]>
Date: Mon, 30 Sep 2013 11:51:54 +0900
Subject: [PATCH] mm, hugetlb: correct missing private flag clearing

We should clear the page's private flag when returing the page to
the hugepage pool. Otherwise, marked hugepage can be allocated to
the user who tries to allocate the non-reserved hugepage. If this user
fail to map this hugepage, he would try to return the page to the
hugepage pool. Since this page has a private flag, resv_huge_pages would
mistakenly increase. This patch fixes this situation.

Signed-off-by: Joonsoo Kim <[email protected]>

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index b49579c..691f226 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -653,6 +653,7 @@ static void free_huge_page(struct page *page)
BUG_ON(page_count(page));
BUG_ON(page_mapcount(page));
restore_reserve = PagePrivate(page);
+ ClearPagePrivate(page);

spin_lock(&hugetlb_lock);
hugetlb_cgroup_uncharge_page(hstate_index(h),
--
1.7.9.5