Re: [PATCH] mm: Improve PageAnon() to check the whole PAGE_MAPPING_FLAGS

["lixinhai.lxh@gmail.com" <lixinhai.lxh@gmail.com>]

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
>