linux-kernel - Re: [PATCH 3/3] mm/swap: remove redundant check for swap_slot_cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <9b45c120-e369-d436-a369-6c80e3d6ee2e@linux.intel.com>
Date:   Fri, 1 May 2020 15:35:27 -0700
From:   Tim Chen <tim.c.chen@...ux.intel.com>
To:     Zhen Lei <thunder.leizhen@...wei.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        linux-mm <linux-mm@...ck.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] mm/swap: remove redundant check for
 swap_slot_cache_initialized



On 4/29/20 11:11 PM, Zhen Lei wrote:
> Because enable_swap_slots_cache can only become true in
> enable_swap_slots_cache(), and depends on swap_slot_cache_initialized is
> true before. That means, when enable_swap_slots_cache is true,
> swap_slot_cache_initialized is true also.
> 
> So the condition:
> "swap_slot_cache_enabled && swap_slot_cache_initialized"
> can be reduced to "swap_slot_cache_enabled"
> 
> And in mathematics:
> "!swap_slot_cache_enabled || !swap_slot_cache_initialized"
> is equal to "!(swap_slot_cache_enabled && swap_slot_cache_initialized)"
> 
> So no functional change.
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@...wei.com>
> ---
>  mm/swap_slots.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/mm/swap_slots.c b/mm/swap_slots.c
> index b40394473a3c..3e6453573a89 100644
> --- a/mm/swap_slots.c
> +++ b/mm/swap_slots.c
> @@ -46,8 +46,7 @@ static void __drain_swap_slots_cache(unsigned int type);
>  static void deactivate_swap_slots_cache(void);
>  static void reactivate_swap_slots_cache(void);
>  
> -#define use_swap_slot_cache (swap_slot_cache_active && \
> -		swap_slot_cache_enabled && swap_slot_cache_initialized)
> +#define use_swap_slot_cache (swap_slot_cache_active && swap_slot_cache_enabled)

Yes, swap_slot_cache_enabled does imply swap_slot_cache_initialized
in current code.  So checking swap_slot_cache_enabled is
enough here.

>  #define SLOTS_CACHE 0x1
>  #define SLOTS_CACHE_RET 0x2
>  
> @@ -94,7 +93,7 @@ static bool check_cache_active(void)
>  {
>  	long pages;
>  
> -	if (!swap_slot_cache_enabled || !swap_slot_cache_initialized)
> +	if (!swap_slot_cache_enabled)

This simplification is okay.   !swap_slot_cache_initialize implies !swap_slot_cache_enabled.
So only !swap_slot_cache_enabled needs to be checked.

>  		return false;
>  
>  	pages = get_nr_swap_pages();
> 

Acked-by: Tim Chen <tim.c.chen@...ux.intel.com>