* [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS
@ 2019-11-15 3:18 Li Xinhai
2019-11-15 5:59 ` lixinhai.lxh
0 siblings, 1 reply; 6+ messages in thread
From: Li Xinhai @ 2019-11-15 3:18 UTC (permalink / raw)
To: linux-mm; +Cc: Andrew Morton, Michal Hocko
PageAnon() just checking on PAGE_MAPPING_ANON bit would cause page,
with PageKsm as true, been wrongly considered as PageAnon. Now,
checking the whole PAGE_MAPPING_FLAGS to avoid this error.
Reported from:
https://lore.kernel.org/linux-mm/20191113000651.20677-1-rcampbell@nvidia.com/
Reported-by: Ralph Campbell <rcampbell@nvidia.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
---
include/linux/page-flags.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 1bf83c8..1849fc3 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -461,7 +461,8 @@ static __always_inline int PageMappingFlags(struct page *page)
static __always_inline int PageAnon(struct page *page)
{
page = compound_head(page);
- return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0;
+ return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
+ PAGE_MAPPING_ANON;
}
static __always_inline int __PageMovable(struct page *page)
--
1.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS
2019-11-15 3:18 [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS Li Xinhai
@ 2019-11-15 5:59 ` lixinhai.lxh
2019-11-15 12:50 ` David Hildenbrand
0 siblings, 1 reply; 6+ messages in thread
From: lixinhai.lxh @ 2019-11-15 5:59 UTC (permalink / raw)
To: linux-mm; +Cc: akpm, mhocko
On 2019-11-15 at 11:18 Li Xinhai wrote:
>PageAnon() just checking on PAGE_MAPPING_ANON bit would cause page,
>with PageKsm as true, been wrongly considered as PageAnon. Now,
>checking the whole PAGE_MAPPING_FLAGS to avoid this error.
>
>Reported from:
>https://lore.kernel.org/linux-mm/20191113000651.20677-1-rcampbell@nvidia.com/
>
>Reported-by: Ralph Campbell <rcampbell@nvidia.com>
>Cc: Andrew Morton <akpm@linux-foundation.org>
>Cc: Michal Hocko <mhocko@suse.com>
>Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
>---
> include/linux/page-flags.h | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>index 1bf83c8..1849fc3 100644
>--- a/include/linux/page-flags.h
>+++ b/include/linux/page-flags.h
>@@ -461,7 +461,8 @@ static __always_inline int PageMappingFlags(struct page *page)
> static __always_inline int PageAnon(struct page *page)
> {
> page = compound_head(page);
>- return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0;
>+ return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
>+ PAGE_MAPPING_ANON;
> }
>
> static __always_inline int __PageMovable(struct page *page)
>--
>1.8.3.1
>
The current semantics of PageAnon() for both KSM and !KSM are used in many
places, so can't change it alone without change other code.
Need skip this patch.
- Xinhai
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS
2019-11-15 5:59 ` lixinhai.lxh
@ 2019-11-15 12:50 ` David Hildenbrand
2019-11-15 13:09 ` lixinhai.lxh
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2019-11-15 12:50 UTC (permalink / raw)
To: lixinhai.lxh, linux-mm; +Cc: akpm, mhocko
On 15.11.19 06:59, lixinhai.lxh@gmail.com wrote:
> On 2019-11-15 at 11:18 Li Xinhai wrote:
>> PageAnon() just checking on PAGE_MAPPING_ANON bit would cause page,
>> with PageKsm as true, been wrongly considered as PageAnon. Now,
>> checking the whole PAGE_MAPPING_FLAGS to avoid this error.
>>
>> Reported from:
>> https://lore.kernel.org/linux-mm/20191113000651.20677-1-rcampbell@nvidia.com/
>>
>> Reported-by: Ralph Campbell <rcampbell@nvidia.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
>> ---
>> include/linux/page-flags.h | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>> index 1bf83c8..1849fc3 100644
>> --- a/include/linux/page-flags.h
>> +++ b/include/linux/page-flags.h
>> @@ -461,7 +461,8 @@ static __always_inline int PageMappingFlags(struct page *page)
>> static __always_inline int PageAnon(struct page *page)
>> {
>> page = compound_head(page);
>> - return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0;
>> + return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
>> + PAGE_MAPPING_ANON;
>> }
>>
>> static __always_inline int __PageMovable(struct page *page)
>> --
>> 1.8.3.1
>>
>
> The current semantics of PageAnon() for both KSM and !KSM are used in many
> places, so can't change it alone without change other code.
> Need skip this patch.
... I assume that was intended because KSM only merges anonymous pages?
If it was not intended, it would scream for a cleanup.
>
> - Xinhai
>
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS
2019-11-15 12:50 ` David Hildenbrand
@ 2019-11-15 13:09 ` lixinhai.lxh
2019-11-15 13:25 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: lixinhai.lxh @ 2019-11-15 13:09 UTC (permalink / raw)
To: David Hildenbrand, linux-mm; +Cc: akpm, mhocko
On 2019-11-15 at 20:50 David Hildenbrand wrote:
>On 15.11.19 06:59, lixinhai.lxh@gmail.com wrote:
>> On 2019-11-15 at 11:18 Li Xinhai wrote:
>>> PageAnon() just checking on PAGE_MAPPING_ANON bit would cause page,
>>> with PageKsm as true, been wrongly considered as PageAnon. Now,
>>> checking the whole PAGE_MAPPING_FLAGS to avoid this error.
>>>
>>> Reported from:
>>> https://lore.kernel.org/linux-mm/20191113000651.20677-1-rcampbell@nvidia.com/
>>>
>>> Reported-by: Ralph Campbell <rcampbell@nvidia.com>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: Michal Hocko <mhocko@suse.com>
>>> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
>>> ---
>>> include/linux/page-flags.h | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>>> index 1bf83c8..1849fc3 100644
>>> --- a/include/linux/page-flags.h
>>> +++ b/include/linux/page-flags.h
>>> @@ -461,7 +461,8 @@ static __always_inline int PageMappingFlags(struct page *page)
>>> static __always_inline int PageAnon(struct page *page)
>>> {
>>> page = compound_head(page);
>>> - return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0;
>>> + return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
>>> + PAGE_MAPPING_ANON;
>>> }
>>>
>>> static __always_inline int __PageMovable(struct page *page)
>>> --
>>> 1.8.3.1
>>>
>>
>> The current semantics of PageAnon() for both KSM and !KSM are used in many
>> places, so can't change it alone without change other code.
>> Need skip this patch.
>
>... I assume that was intended because KSM only merges anonymous pages?
>If it was not intended, it would scream for a cleanup.
>
Yes, I think that was intended to keep the origial version of PageAnon() when add the
PageKsm(). So all places where used to have PageAnon() didn't requre change, but other
places for new code we have some thing like (PageAnon() && !PageKsm()) check, or do
checking in correct sequence.
One thing I am not quite sure is about couting of anonymous page, in case page is KSM
or !KSM, hope those old codes has been correctly handled.
>>
>> - Xinhai
>>
>
>
>--
>
>Thanks,
>
>David / dhildenb
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS
2019-11-15 13:09 ` lixinhai.lxh
@ 2019-11-15 13:25 ` Michal Hocko
2019-11-15 15:01 ` lixinhai.lxh
0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2019-11-15 13:25 UTC (permalink / raw)
To: lixinhai.lxh; +Cc: David Hildenbrand, linux-mm, akpm
On Fri 15-11-19 21:09:55, lixinhai.lxh@gmail.com wrote:
> On 2019-11-15 at 20:50 David Hildenbrand wrote:
> >On 15.11.19 06:59, lixinhai.lxh@gmail.com wrote:
> >> On 2019-11-15 at 11:18 Li Xinhai wrote:
> >>> PageAnon() just checking on PAGE_MAPPING_ANON bit would cause page,
> >>> with PageKsm as true, been wrongly considered as PageAnon. Now,
> >>> checking the whole PAGE_MAPPING_FLAGS to avoid this error.
> >>>
> >>> Reported from:
> >>> https://lore.kernel.org/linux-mm/20191113000651.20677-1-rcampbell@nvidia.com/
> >>>
> >>> Reported-by: Ralph Campbell <rcampbell@nvidia.com>
> >>> Cc: Andrew Morton <akpm@linux-foundation.org>
> >>> Cc: Michal Hocko <mhocko@suse.com>
> >>> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
> >>> ---
> >>> include/linux/page-flags.h | 3 ++-
> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> >>> index 1bf83c8..1849fc3 100644
> >>> --- a/include/linux/page-flags.h
> >>> +++ b/include/linux/page-flags.h
> >>> @@ -461,7 +461,8 @@ static __always_inline int PageMappingFlags(struct page *page)
> >>> static __always_inline int PageAnon(struct page *page)
> >>> {
> >>> page = compound_head(page);
> >>> - return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0;
> >>> + return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
> >>> + PAGE_MAPPING_ANON;
> >>> }
> >>>
> >>> static __always_inline int __PageMovable(struct page *page)
> >>> --
> >>> 1.8.3.1
> >>>
> >>
> >> The current semantics of PageAnon() for both KSM and !KSM are used in many
> >> places, so can't change it alone without change other code.
> >> Need skip this patch.
> >
> >... I assume that was intended because KSM only merges anonymous pages?
> >If it was not intended, it would scream for a cleanup.
> >
> Yes, I think that was intended to keep the origial version of PageAnon() when add the
> PageKsm(). So all places where used to have PageAnon() didn't requre change, but other
> places for new code we have some thing like (PageAnon() && !PageKsm()) check, or do
> checking in correct sequence.
>
> One thing I am not quite sure is about couting of anonymous page, in case page is KSM
> or !KSM, hope those old codes has been correctly handled.
I am not really sure I understand what do you like to achieve here. Yes
there are different checks representing different classes of pages. In
this particular case the ordering of the check was just wrong. That is
trivial to be fixed. PageAnon works as intented AFAIK. If that is not
the case then please be explicit why.
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS
2019-11-15 13:25 ` Michal Hocko
@ 2019-11-15 15:01 ` lixinhai.lxh
0 siblings, 0 replies; 6+ messages in thread
From: lixinhai.lxh @ 2019-11-15 15:01 UTC (permalink / raw)
To: mhocko; +Cc: David Hildenbrand, linux-mm, akpm
On 2019-11-15 at 21:25 Michal Hocko wrote:
>On Fri 15-11-19 21:09:55, lixinhai.lxh@gmail.com wrote:
>> On 2019-11-15 at 20:50 David Hildenbrand wrote:
>> >On 15.11.19 06:59, lixinhai.lxh@gmail.com wrote:
>> >> On 2019-11-15 at 11:18 Li Xinhai wrote:
>> >>> PageAnon() just checking on PAGE_MAPPING_ANON bit would cause page,
>> >>> with PageKsm as true, been wrongly considered as PageAnon. Now,
>> >>> checking the whole PAGE_MAPPING_FLAGS to avoid this error.
>> >>>
>> >>> Reported from:
>> >>> https://lore.kernel.org/linux-mm/20191113000651.20677-1-rcampbell@nvidia.com/
>> >>>
>> >>> Reported-by: Ralph Campbell <rcampbell@nvidia.com>
>> >>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> >>> Cc: Michal Hocko <mhocko@suse.com>
>> >>> Signed-off-by: Li Xinhai <lixinhai.lxh@gmail.com>
>> >>> ---
>> >>> include/linux/page-flags.h | 3 ++-
>> >>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
>> >>> index 1bf83c8..1849fc3 100644
>> >>> --- a/include/linux/page-flags.h
>> >>> +++ b/include/linux/page-flags.h
>> >>> @@ -461,7 +461,8 @@ static __always_inline int PageMappingFlags(struct page *page)
>> >>> static __always_inline int PageAnon(struct page *page)
>> >>> {
>> >>> page = compound_head(page);
>> >>> - return ((unsigned long)page->mapping & PAGE_MAPPING_ANON) != 0;
>> >>> + return ((unsigned long)page->mapping & PAGE_MAPPING_FLAGS) ==
>> >>> + PAGE_MAPPING_ANON;
>> >>> }
>> >>>
>> >>> static __always_inline int __PageMovable(struct page *page)
>> >>> --
>> >>> 1.8.3.1
>> >>>
>> >>
>> >> The current semantics of PageAnon() for both KSM and !KSM are used in many
>> >> places, so can't change it alone without change other code.
>> >> Need skip this patch.
>> >
>> >... I assume that was intended because KSM only merges anonymous pages?
>> >If it was not intended, it would scream for a cleanup.
>> >
>> Yes, I think that was intended to keep the origial version of PageAnon() when add the
>> PageKsm(). So all places where used to have PageAnon() didn't requre change, but other
>> places for new code we have some thing like (PageAnon() && !PageKsm()) check, or do
>> checking in correct sequence.
>>
>> One thing I am not quite sure is about couting of anonymous page, in case page is KSM
>> or !KSM, hope those old codes has been correctly handled.
>
>I am not really sure I understand what do you like to achieve here. Yes
>there are different checks representing different classes of pages. In
>this particular case the ordering of the check was just wrong. That is
>trivial to be fixed. PageAnon works as intented AFAIK. If that is not
>the case then please be explicit why.
I was thinking about make changes so to using PageAnon() for Anonymous&!KSM and
PageKsm() for Anonymous&KSM, then don't require consider sequence. Now, I realized
that PageAnon() is intended for cover KSM and !KSM cases and it seems no big benefits
to change the semantics of PageAnon().
>--
>Michal Hocko
>SUSE Labs
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-15 15:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-15 3:18 [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS Li Xinhai
2019-11-15 5:59 ` lixinhai.lxh
2019-11-15 12:50 ` David Hildenbrand
2019-11-15 13:09 ` lixinhai.lxh
2019-11-15 13:25 ` Michal Hocko
2019-11-15 15:01 ` lixinhai.lxh
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox