linux-kernel - Re: [PATCH] mm: migrate: Use bool instead of int for the return value of PageMovable

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20160711002605.GD31817@bbox>
Date:	Mon, 11 Jul 2016 09:26:05 +0900
From:	Minchan Kim <minchan@...nel.org>
To:	<chengang@...ndsoft.com.cn>
CC:	<akpm@...ux-foundation.org>, <vbabka@...e.cz>,
	<mgorman@...hsingularity.net>, <mhocko@...e.com>,
	<gi-oh.kim@...fitbricks.com>, <iamjoonsoo.kim@....com>,
	<hillf.zj@...baba-inc.com>, <rientjes@...gle.com>,
	<linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>,
	Chen Gang <gang.chen.5i5j@...il.com>
Subject: Re: [PATCH] mm: migrate: Use bool instead of int for the return
 value of PageMovable

On Sat, Jul 09, 2016 at 11:55:04PM +0800, chengang@...ndsoft.com.cn wrote:
> From: Chen Gang <chengang@...ndsoft.com.cn>
> 
> For pure bool function's return value, bool is a little better more or
> less than int.
> 
> And return boolean result directly, since 'if' statement is also for
> boolean checking, and return boolean result, too.

I just wanted to consistent with other PageXXX flags functions, PageAnon,
PageMappingFlags which returns int rather than bool. Although I agree bool
is natural, I want to be consistent with others rather than breaking at
the moment.

Maybe if you feel it's really helpful, you might be able to handle all
of places I mentioned for better readability and keeping consistency.
But I doubt it's worth.

Thanks.

> 
> Signed-off-by: Chen Gang <gang.chen.5i5j@...il.com>

> ---
>  include/linux/migrate.h | 4 ++--
>  mm/compaction.c         | 9 +++------
>  2 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/migrate.h b/include/linux/migrate.h
> index ae8d475..0e366f8 100644
> --- a/include/linux/migrate.h
> +++ b/include/linux/migrate.h
> @@ -72,11 +72,11 @@ static inline int migrate_huge_page_move_mapping(struct address_space *mapping,
>  #endif /* CONFIG_MIGRATION */
>  
>  #ifdef CONFIG_COMPACTION
> -extern int PageMovable(struct page *page);
> +extern bool PageMovable(struct page *page);
>  extern void __SetPageMovable(struct page *page, struct address_space *mapping);
>  extern void __ClearPageMovable(struct page *page);
>  #else
> -static inline int PageMovable(struct page *page) { return 0; };
> +static inline bool PageMovable(struct page *page) { return false; };
>  static inline void __SetPageMovable(struct page *page,
>  				struct address_space *mapping)
>  {
> diff --git a/mm/compaction.c b/mm/compaction.c
> index 0bd53fb..cfcfe88 100644
> --- a/mm/compaction.c
> +++ b/mm/compaction.c
> @@ -95,19 +95,16 @@ static inline bool migrate_async_suitable(int migratetype)
>  
>  #ifdef CONFIG_COMPACTION
>  
> -int PageMovable(struct page *page)
> +bool PageMovable(struct page *page)
>  {
>  	struct address_space *mapping;
>  
>  	VM_BUG_ON_PAGE(!PageLocked(page), page);
>  	if (!__PageMovable(page))
> -		return 0;
> +		return false;
>  
>  	mapping = page_mapping(page);
> -	if (mapping && mapping->a_ops && mapping->a_ops->isolate_page)
> -		return 1;
> -
> -	return 0;
> +	return mapping && mapping->a_ops && mapping->a_ops->isolate_page;
>  }
>  EXPORT_SYMBOL(PageMovable);
>  
> -- 
> 1.9.3
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@...ck.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@...ck.org"> email@...ck.org </a>