[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87tv24j9hq.fsf@yhuang-dev.intel.com>
Date: Wed, 01 Apr 2020 09:11:13 +0800
From: "Huang\, Ying" <ying.huang@...el.com>
To: Michal Hocko <mhocko@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>, Minchan Kim <minchan@...nel.org>,
"Hugh Dickins" <hughd@...gle.com>, Rik van Riel <riel@...riel.com>
Subject: Re: [PATCH] mm, trivial: Simplify swap related code in try_to_unmap_one()
Michal Hocko <mhocko@...nel.org> writes:
> On Tue 31-03-20 16:46:13, Huang, Ying wrote:
>> From: Huang Ying <ying.huang@...el.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
Powered by blists - more mailing lists