2018-11-29 11:47:57

by Yongkai Wu

[permalink] [raw]
Subject: [PATCH] hugetlbfs: Call VM_BUG_ON_PAGE earlier in free_huge_page

A stack trace was triggered by VM_BUG_ON_PAGE(page_mapcount(page),
page) in free_huge_page(). Unfortunately, the page->mapping field
was set to NULL before this test. This made it more difficult to
determine the root cause of the problem.

Move the VM_BUG_ON_PAGE tests earlier in the function so that if
they do trigger more information is present in the page struct.

Signed-off-by: Yongkai Wu <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Acked-by: Mike Kravetz <[email protected]>
---
mm/hugetlb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 7f2a28a..14ef274 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1248,10 +1248,11 @@ void free_huge_page(struct page *page)
(struct hugepage_subpool *)page_private(page);
bool restore_reserve;

- set_page_private(page, 0);
- page->mapping = NULL;
VM_BUG_ON_PAGE(page_count(page), page);
VM_BUG_ON_PAGE(page_mapcount(page), page);
+
+ set_page_private(page, 0);
+ page->mapping = NULL;
restore_reserve = PagePrivate(page);
ClearPagePrivate(page);

--
1.8.3.1



2018-11-29 13:54:17

by William Kucharski

[permalink] [raw]
Subject: Re: [PATCH] hugetlbfs: Call VM_BUG_ON_PAGE earlier in free_huge_page

Reviewed-by: William Kucharski <[email protected]>

> On Nov 29, 2018, at 4:44 AM, Yongkai Wu <[email protected]> wrote:
>
> A stack trace was triggered by VM_BUG_ON_PAGE(page_mapcount(page),
> page) in free_huge_page(). Unfortunately, the page->mapping field
> was set to NULL before this test. This made it more difficult to
> determine the root cause of the problem.
>
> Move the VM_BUG_ON_PAGE tests earlier in the function so that if
> they do trigger more information is present in the page struct.
>
> Signed-off-by: Yongkai Wu <[email protected]>
> Acked-by: Michal Hocko <[email protected]>
> Acked-by: Mike Kravetz <[email protected]>
> ---
> mm/hugetlb.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 7f2a28a..14ef274 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1248,10 +1248,11 @@ void free_huge_page(struct page *page)
> (struct hugepage_subpool *)page_private(page);
> bool restore_reserve;
>
> - set_page_private(page, 0);
> - page->mapping = NULL;
> VM_BUG_ON_PAGE(page_count(page), page);
> VM_BUG_ON_PAGE(page_mapcount(page), page);
> +
> + set_page_private(page, 0);
> + page->mapping = NULL;
> restore_reserve = PagePrivate(page);
> ClearPagePrivate(page);
>
> --
> 1.8.3.1
>


2018-11-29 20:54:05

by Mike Kravetz

[permalink] [raw]
Subject: Re: [PATCH] hugetlbfs: Call VM_BUG_ON_PAGE earlier in free_huge_page

On 11/29/18 5:51 AM, William Kucharski wrote:
> Reviewed-by: William Kucharski <[email protected]>
>
>> On Nov 29, 2018, at 4:44 AM, Yongkai Wu <[email protected]> wrote:
>>
>> A stack trace was triggered by VM_BUG_ON_PAGE(page_mapcount(page),
>> page) in free_huge_page(). Unfortunately, the page->mapping field
>> was set to NULL before this test. This made it more difficult to
>> determine the root cause of the problem.
>>
>> Move the VM_BUG_ON_PAGE tests earlier in the function so that if
>> they do trigger more information is present in the page struct.
>>
>> Signed-off-by: Yongkai Wu <[email protected]>
>> Acked-by: Michal Hocko <[email protected]>
>> Acked-by: Mike Kravetz <[email protected]>

Thank you for fixing the formatting and commit message.

Adding Andrew on so he can add to his tree as appropriatre. Also Cc'ing Michal.

>> ---
>> mm/hugetlb.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 7f2a28a..14ef274 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -1248,10 +1248,11 @@ void free_huge_page(struct page *page)
>> (struct hugepage_subpool *)page_private(page);
>> bool restore_reserve;
>>
>> - set_page_private(page, 0);
>> - page->mapping = NULL;
>> VM_BUG_ON_PAGE(page_count(page), page);
>> VM_BUG_ON_PAGE(page_mapcount(page), page);
>> +
>> + set_page_private(page, 0);
>> + page->mapping = NULL;
>> restore_reserve = PagePrivate(page);
>> ClearPagePrivate(page);
>>
>> --
>> 1.8.3.1
>>
>


--
Mike Kravetz