lkml.org 
[lkml]   [2020]   [Mar]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] mm, trivial: Simplify swap related code in try_to_unmap_one()
Date
Michal Hocko <mhocko@kernel.org> writes:

> On Tue 31-03-20 16:46:13, Huang, Ying wrote:
>> From: Huang Ying <ying.huang@intel.com>
>>
>> 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

\
 
 \ /
  Last update: 2020-04-01 03:13    [W:0.050 / U:2.816 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site