Michal Hocko <mhocko@xxxxxxxxxx> writes:
> On Tue 31-03-20 16:46:13, Huang, Ying wrote:
>> From: Huang Ying <ying.huang@xxxxxxxxx>
>>
>> Because PageSwapCache() will always return false if PageSwapBacked() returns
>> false, and PageSwapBacked() will be check for MADV_FREE pages in
>> try_to_unmap_one(). The swap related code in try_to_unmap_one() can be
>> simplified to improve the readability.
>
> My understanding is that this is a sanity check to let us know if
> something breaks. Do we really want to get rid of it? Maybe it is not
> really useful but if that is the case then the changelog should reflect
> this fact.
Now the definition of PageSwapCache() is,
static __always_inline int PageSwapCache(struct page *page)
{
#ifdef CONFIG_THP_SWAP
page = compound_head(page);
#endif
return PageSwapBacked(page) && test_bit(PG_swapcache, &page->flags);
}
So, if PageSwapBacked() returns false, PageSwapCache() will always
return false. The original checking,
- if (unlikely(PageSwapBacked(page) != PageSwapCache(page))) {
is equivalent to
- if (unlikely(PageSwapBacked(page) && !PageSwapCache(page))) {
Then what is the check !PageSwapBacked() && PageSwapCache() for? To
prevent someone to change the definition of PageSwapCache() in the
future to break this?
Best Regards,
Huang, Ying
