* [BUG/REGRESSION] THP: broken page count after commit aa88b68c
@ 2016-06-02 15:21 Gerald Schaefer
2016-06-02 15:51 ` Kirill A. Shutemov
0 siblings, 1 reply; 10+ messages in thread
From: Gerald Schaefer @ 2016-06-02 15:21 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Andrea Arcangeli, Aneesh Kumar K.V, Mel Gorman, Hugh Dickins,
Johannes Weiner, Dave Hansen, Vlastimil Babka, Andrew Morton,
Linus Torvalds, linux-mm, linux-kernel, Christian Borntraeger,
Martin Schwidefsky, Heiko Carstens
Christian Borntraeger reported a kernel panic after corrupt page counts,
and it turned out to be a regression introduced with commit aa88b68c
"thp: keep huge zero page pinned until tlb flush", at least on s390.
put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
and it was replaced by tlb_remove_page(). However, release_pages() might
not always be triggered by (the arch-specific) tlb_remove_page().
On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
because we don't use the MMU-gather logic. Although both functions have very
similar names, they are doing very unsimilar things, in particular
free_page_xxx is just doing a put_page(), while free_pages_xxx calls
release_pages().
This of course results in very harmful put_page()s on the huge zero page,
on architectures where tlb_remove_page() is implemented in this way. It
seems to affect only s390 and sh, but sh doesn't have THP support, so
the problem (currently) probably only exists on s390.
The following quick hack fixed the issue:
diff --git a/mm/swap_state.c b/mm/swap_state.c
index 0d457e7..c99463a 100644
--- a/mm/swap_state.c
+++ b/mm/swap_state.c
@@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
void free_page_and_swap_cache(struct page *page)
{
free_swap_cache(page);
- put_page(page);
+ if (is_huge_zero_page(page))
+ put_huge_zero_page();
+ else
+ put_page(page);
}
/*
But of course there might be a better solution, and there still are some
questions left:
- Why does free_page_xxx() behave so differently from free_pages_xxx()?
- Would it be OK to implement free_page_xxx() by calling free_pages_xxx()
with nr = 1, similar to free_page() vs. free_pages()?
- Would it be OK to replace the put_page() in free_page_xxx() with a call
to release_pages() with nr = 1?
- Would it be better to fix this in the arch-specific tlb_remove_page(),
by calling free_pages_xxx() with nr = 1 instead of free_page_xxx()?
Regards,
Gerald
--
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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 15:21 [BUG/REGRESSION] THP: broken page count after commit aa88b68c Gerald Schaefer
@ 2016-06-02 15:51 ` Kirill A. Shutemov
2016-06-02 18:40 ` Andrew Morton
2016-06-02 19:47 ` Hugh Dickins
0 siblings, 2 replies; 10+ messages in thread
From: Kirill A. Shutemov @ 2016-06-02 15:51 UTC (permalink / raw)
To: Gerald Schaefer
Cc: Kirill A. Shutemov, Andrea Arcangeli, Aneesh Kumar K.V,
Mel Gorman, Hugh Dickins, Johannes Weiner, Dave Hansen,
Vlastimil Babka, Andrew Morton, Linus Torvalds, linux-mm,
linux-kernel, Christian Borntraeger, Martin Schwidefsky,
Heiko Carstens
On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
> Christian Borntraeger reported a kernel panic after corrupt page counts,
> and it turned out to be a regression introduced with commit aa88b68c
> "thp: keep huge zero page pinned until tlb flush", at least on s390.
>
> put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
> and it was replaced by tlb_remove_page(). However, release_pages() might
> not always be triggered by (the arch-specific) tlb_remove_page().
>
> On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
> tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
> because we don't use the MMU-gather logic. Although both functions have very
> similar names, they are doing very unsimilar things, in particular
> free_page_xxx is just doing a put_page(), while free_pages_xxx calls
> release_pages().
>
> This of course results in very harmful put_page()s on the huge zero page,
> on architectures where tlb_remove_page() is implemented in this way. It
> seems to affect only s390 and sh, but sh doesn't have THP support, so
> the problem (currently) probably only exists on s390.
>
> The following quick hack fixed the issue:
>
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 0d457e7..c99463a 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
> void free_page_and_swap_cache(struct page *page)
> {
> free_swap_cache(page);
> - put_page(page);
> + if (is_huge_zero_page(page))
> + put_huge_zero_page();
> + else
> + put_page(page);
> }
>
> /*
The fix looks good to me.
> But of course there might be a better solution, and there still are some
> questions left:
> - Why does free_page_xxx() behave so differently from free_pages_xxx()?
I don't see it behave too deiferently. It just try to batch freeing to
lower locking overhead.
> - Would it be OK to implement free_page_xxx() by calling free_pages_xxx()
> with nr = 1, similar to free_page() vs. free_pages()?
> - Would it be OK to replace the put_page() in free_page_xxx() with a call
> to release_pages() with nr = 1?
release_pages() somewhat suboptimal for nr=1. I guess we can fix this with
shortcut to put_page() at start of release_page() if nr == 1.
> - Would it be better to fix this in the arch-specific tlb_remove_page(),
> by calling free_pages_xxx() with nr = 1 instead of free_page_xxx()?
>
> Regards,
> Gerald
>
> --
> 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>
--
Kirill A. Shutemov
--
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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 15:51 ` Kirill A. Shutemov
@ 2016-06-02 18:40 ` Andrew Morton
2016-06-02 18:56 ` Christian Borntraeger
` (2 more replies)
2016-06-02 19:47 ` Hugh Dickins
1 sibling, 3 replies; 10+ messages in thread
From: Andrew Morton @ 2016-06-02 18:40 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Gerald Schaefer, Kirill A. Shutemov, Andrea Arcangeli,
Aneesh Kumar K.V, Mel Gorman, Hugh Dickins, Johannes Weiner,
Dave Hansen, Vlastimil Babka, Linus Torvalds, linux-mm,
linux-kernel, Christian Borntraeger, Martin Schwidefsky,
Heiko Carstens
On Thu, 2 Jun 2016 18:51:50 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
> On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
> > Christian Borntraeger reported a kernel panic after corrupt page counts,
> > and it turned out to be a regression introduced with commit aa88b68c
> > "thp: keep huge zero page pinned until tlb flush", at least on s390.
> >
> > put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
> > and it was replaced by tlb_remove_page(). However, release_pages() might
> > not always be triggered by (the arch-specific) tlb_remove_page().
> >
> > On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
> > tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
> > because we don't use the MMU-gather logic. Although both functions have very
> > similar names, they are doing very unsimilar things, in particular
> > free_page_xxx is just doing a put_page(), while free_pages_xxx calls
> > release_pages().
> >
> > This of course results in very harmful put_page()s on the huge zero page,
> > on architectures where tlb_remove_page() is implemented in this way. It
> > seems to affect only s390 and sh, but sh doesn't have THP support, so
> > the problem (currently) probably only exists on s390.
> >
> > The following quick hack fixed the issue:
> >
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 0d457e7..c99463a 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
> > void free_page_and_swap_cache(struct page *page)
> > {
> > free_swap_cache(page);
> > - put_page(page);
> > + if (is_huge_zero_page(page))
> > + put_huge_zero_page();
> > + else
> > + put_page(page);
> > }
> >
> > /*
>
> The fix looks good to me.
Yes. A bit regrettable, but that's what release_pages() does.
Can we have a signed-off-by please?
--
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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 18:40 ` Andrew Morton
2016-06-02 18:56 ` Christian Borntraeger
@ 2016-06-02 18:56 ` Christian Borntraeger
[not found] ` <201606021856.u52ImC6o037023@mx0a-001b2d01.pphosted.com>
2 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2016-06-02 18:56 UTC (permalink / raw)
To: Andrew Morton, Kirill A. Shutemov
Cc: Gerald Schaefer, Kirill A. Shutemov, Andrea Arcangeli,
Aneesh Kumar K.V, Mel Gorman, Hugh Dickins, Johannes Weiner,
Dave Hansen, Vlastimil Babka, Linus Torvalds, linux-mm,
linux-kernel, Martin Schwidefsky, Heiko Carstens
On 06/02/2016 08:40 PM, Andrew Morton wrote:
> On Thu, 2 Jun 2016 18:51:50 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>
>> On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
>>> Christian Borntraeger reported a kernel panic after corrupt page counts,
>>> and it turned out to be a regression introduced with commit aa88b68c
>>> "thp: keep huge zero page pinned until tlb flush", at least on s390.
>>>
>>> put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
>>> and it was replaced by tlb_remove_page(). However, release_pages() might
>>> not always be triggered by (the arch-specific) tlb_remove_page().
>>>
>>> On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
>>> tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
>>> because we don't use the MMU-gather logic. Although both functions have very
>>> similar names, they are doing very unsimilar things, in particular
>>> free_page_xxx is just doing a put_page(), while free_pages_xxx calls
>>> release_pages().
>>>
>>> This of course results in very harmful put_page()s on the huge zero page,
>>> on architectures where tlb_remove_page() is implemented in this way. It
>>> seems to affect only s390 and sh, but sh doesn't have THP support, so
>>> the problem (currently) probably only exists on s390.
>>>
>>> The following quick hack fixed the issue:
>>>
>>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>>> index 0d457e7..c99463a 100644
>>> --- a/mm/swap_state.c
>>> +++ b/mm/swap_state.c
>>> @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
>>> void free_page_and_swap_cache(struct page *page)
>>> {
>>> free_swap_cache(page);
>>> - put_page(page);
>>> + if (is_huge_zero_page(page))
>>> + put_huge_zero_page();
>>> + else
>>> + put_page(page);
>>> }
>>>
>>> /*
>>
>> The fix looks good to me.
>
> Yes. A bit regrettable, but that's what release_pages() does.
>
> Can we have a signed-off-by please?
Please also add CC: stable for 4.6
--
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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 18:40 ` Andrew Morton
@ 2016-06-02 18:56 ` Christian Borntraeger
2016-06-02 18:56 ` Christian Borntraeger
[not found] ` <201606021856.u52ImC6o037023@mx0a-001b2d01.pphosted.com>
2 siblings, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2016-06-02 18:56 UTC (permalink / raw)
To: Andrew Morton, Kirill A. Shutemov
Cc: Gerald Schaefer, Kirill A. Shutemov, Andrea Arcangeli,
Aneesh Kumar K.V, Mel Gorman, Hugh Dickins, Johannes Weiner,
Dave Hansen, Vlastimil Babka, Linus Torvalds, linux-mm,
linux-kernel, Martin Schwidefsky, Heiko Carstens
On 06/02/2016 08:40 PM, Andrew Morton wrote:
> On Thu, 2 Jun 2016 18:51:50 +0300 "Kirill A. Shutemov" <kirill@shutemov.name> wrote:
>
>> On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
>>> Christian Borntraeger reported a kernel panic after corrupt page counts,
>>> and it turned out to be a regression introduced with commit aa88b68c
>>> "thp: keep huge zero page pinned until tlb flush", at least on s390.
>>>
>>> put_huge_zero_page() was moved over from zap_huge_pmd() to release_pages(),
>>> and it was replaced by tlb_remove_page(). However, release_pages() might
>>> not always be triggered by (the arch-specific) tlb_remove_page().
>>>
>>> On s390 we call free_page_and_swap_cache() from tlb_remove_page(), and not
>>> tlb_flush_mmu() -> free_pages_and_swap_cache() like the generic version,
>>> because we don't use the MMU-gather logic. Although both functions have very
>>> similar names, they are doing very unsimilar things, in particular
>>> free_page_xxx is just doing a put_page(), while free_pages_xxx calls
>>> release_pages().
>>>
>>> This of course results in very harmful put_page()s on the huge zero page,
>>> on architectures where tlb_remove_page() is implemented in this way. It
>>> seems to affect only s390 and sh, but sh doesn't have THP support, so
>>> the problem (currently) probably only exists on s390.
>>>
>>> The following quick hack fixed the issue:
>>>
>>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>>> index 0d457e7..c99463a 100644
>>> --- a/mm/swap_state.c
>>> +++ b/mm/swap_state.c
>>> @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
>>> void free_page_and_swap_cache(struct page *page)
>>> {
>>> free_swap_cache(page);
>>> - put_page(page);
>>> + if (is_huge_zero_page(page))
>>> + put_huge_zero_page();
>>> + else
>>> + put_page(page);
>>> }
>>>
>>> /*
>>
>> The fix looks good to me.
>
> Yes. A bit regrettable, but that's what release_pages() does.
>
> Can we have a signed-off-by please?
Please also add CC: stable for 4.6
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
[not found] ` <201606021856.u52ImC6o037023@mx0a-001b2d01.pphosted.com>
@ 2016-06-02 19:03 ` Andrew Morton
2016-06-02 19:10 ` Christian Borntraeger
2016-06-02 19:10 ` Christian Borntraeger
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Morton @ 2016-06-02 19:03 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Kirill A. Shutemov, Gerald Schaefer, Kirill A. Shutemov,
Andrea Arcangeli, Aneesh Kumar K.V, Mel Gorman, Hugh Dickins,
Johannes Weiner, Dave Hansen, Vlastimil Babka, Linus Torvalds,
linux-mm, linux-kernel, Martin Schwidefsky, Heiko Carstens
On Thu, 2 Jun 2016 20:56:27 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >> The fix looks good to me.
> >
> > Yes. A bit regrettable, but that's what release_pages() does.
> >
> > Can we have a signed-off-by please?
>
> Please also add CC: stable for 4.6
I shall take that as a "yes" and I'll add
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
to the changelog.
--
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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 19:03 ` Andrew Morton
2016-06-02 19:10 ` Christian Borntraeger
@ 2016-06-02 19:10 ` Christian Borntraeger
1 sibling, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2016-06-02 19:10 UTC (permalink / raw)
To: Andrew Morton
Cc: Kirill A. Shutemov, Gerald Schaefer, Kirill A. Shutemov,
Andrea Arcangeli, Aneesh Kumar K.V, Mel Gorman, Hugh Dickins,
Johannes Weiner, Dave Hansen, Vlastimil Babka, Linus Torvalds,
linux-mm, linux-kernel, Martin Schwidefsky, Heiko Carstens
On 06/02/2016 09:03 PM, Andrew Morton wrote:
> On Thu, 2 Jun 2016 20:56:27 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>>>> The fix looks good to me.
>>>
>>> Yes. A bit regrettable, but that's what release_pages() does.
>>>
>>> Can we have a signed-off-by please?
>>
>> Please also add CC: stable for 4.6
>
> I shall take that as a "yes" and I'll add
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> to the changelog.
Gerald has created the patch,
but you could add
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 19:03 ` Andrew Morton
@ 2016-06-02 19:10 ` Christian Borntraeger
2016-06-02 19:10 ` Christian Borntraeger
1 sibling, 0 replies; 10+ messages in thread
From: Christian Borntraeger @ 2016-06-02 19:10 UTC (permalink / raw)
To: Andrew Morton
Cc: Kirill A. Shutemov, Gerald Schaefer, Kirill A. Shutemov,
Andrea Arcangeli, Aneesh Kumar K.V, Mel Gorman, Hugh Dickins,
Johannes Weiner, Dave Hansen, Vlastimil Babka, Linus Torvalds,
linux-mm, linux-kernel, Martin Schwidefsky, Heiko Carstens
On 06/02/2016 09:03 PM, Andrew Morton wrote:
> On Thu, 2 Jun 2016 20:56:27 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>>>> The fix looks good to me.
>>>
>>> Yes. A bit regrettable, but that's what release_pages() does.
>>>
>>> Can we have a signed-off-by please?
>>
>> Please also add CC: stable for 4.6
>
> I shall take that as a "yes" and I'll add
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>
> to the changelog.
Gerald has created the patch,
but you could add
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 15:51 ` Kirill A. Shutemov
2016-06-02 18:40 ` Andrew Morton
@ 2016-06-02 19:47 ` Hugh Dickins
2016-06-03 10:36 ` Kirill A. Shutemov
1 sibling, 1 reply; 10+ messages in thread
From: Hugh Dickins @ 2016-06-02 19:47 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Gerald Schaefer, Kirill A. Shutemov, Andrea Arcangeli,
Aneesh Kumar K.V, Mel Gorman, Hugh Dickins, Johannes Weiner,
Dave Hansen, Vlastimil Babka, Andrew Morton, Linus Torvalds,
linux-mm, linux-kernel, Christian Borntraeger,
Martin Schwidefsky, Heiko Carstens
On Thu, 2 Jun 2016, Kirill A. Shutemov wrote:
> On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
> >
> > The following quick hack fixed the issue:
> >
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 0d457e7..c99463a 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
> > void free_page_and_swap_cache(struct page *page)
> > {
> > free_swap_cache(page);
> > - put_page(page);
> > + if (is_huge_zero_page(page))
> > + put_huge_zero_page();
> > + else
> > + put_page(page);
> > }
> >
> > /*
>
> The fix looks good to me.
Is there a good reason why the refcount of the huge_zero_page is
huge_zero_refcount, instead of the refcount of the huge_zero_page?
Wouldn't the latter avoid such is_huge_zero_page() special-casing?
Hugh
--
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] 10+ messages in thread
* Re: [BUG/REGRESSION] THP: broken page count after commit aa88b68c
2016-06-02 19:47 ` Hugh Dickins
@ 2016-06-03 10:36 ` Kirill A. Shutemov
0 siblings, 0 replies; 10+ messages in thread
From: Kirill A. Shutemov @ 2016-06-03 10:36 UTC (permalink / raw)
To: Hugh Dickins
Cc: Gerald Schaefer, Kirill A. Shutemov, Andrea Arcangeli,
Aneesh Kumar K.V, Mel Gorman, Johannes Weiner, Dave Hansen,
Vlastimil Babka, Andrew Morton, Linus Torvalds, linux-mm,
linux-kernel, Christian Borntraeger, Martin Schwidefsky,
Heiko Carstens
On Thu, Jun 02, 2016 at 12:47:57PM -0700, Hugh Dickins wrote:
> On Thu, 2 Jun 2016, Kirill A. Shutemov wrote:
> > On Thu, Jun 02, 2016 at 05:21:41PM +0200, Gerald Schaefer wrote:
> > >
> > > The following quick hack fixed the issue:
> > >
> > > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > > index 0d457e7..c99463a 100644
> > > --- a/mm/swap_state.c
> > > +++ b/mm/swap_state.c
> > > @@ -252,7 +252,10 @@ static inline void free_swap_cache(struct page *page)
> > > void free_page_and_swap_cache(struct page *page)
> > > {
> > > free_swap_cache(page);
> > > - put_page(page);
> > > + if (is_huge_zero_page(page))
> > > + put_huge_zero_page();
> > > + else
> > > + put_page(page);
> > > }
> > >
> > > /*
> >
> > The fix looks good to me.
>
> Is there a good reason why the refcount of the huge_zero_page is
> huge_zero_refcount, instead of the refcount of the huge_zero_page?
> Wouldn't the latter avoid such is_huge_zero_page() special-casing?
Hm. I thought I had a reason for not using page's refcount, but I can't
find any now. We would loose sanity check in put_huge_zero_page(), but I
guess it's fine since we never triggered it.
I'll put it to my todo list.
--
Kirill A. Shutemov
--
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] 10+ messages in thread
end of thread, other threads:[~2016-06-03 10:37 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-02 15:21 [BUG/REGRESSION] THP: broken page count after commit aa88b68c Gerald Schaefer
2016-06-02 15:51 ` Kirill A. Shutemov
2016-06-02 18:40 ` Andrew Morton
2016-06-02 18:56 ` Christian Borntraeger
2016-06-02 18:56 ` Christian Borntraeger
[not found] ` <201606021856.u52ImC6o037023@mx0a-001b2d01.pphosted.com>
2016-06-02 19:03 ` Andrew Morton
2016-06-02 19:10 ` Christian Borntraeger
2016-06-02 19:10 ` Christian Borntraeger
2016-06-02 19:47 ` Hugh Dickins
2016-06-03 10:36 ` Kirill A. Shutemov
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).