Messages in this thread |  | | From | "Huang\, Ying" <> | Subject | Re: [PATCH] mm, trivial: Simplify swap related code in try_to_unmap_one() | Date | Wed, 01 Apr 2020 09:11:13 +0800 |
| |
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
|  |