linux-kernel - Re: [PATCH] mm: change the type to bool for page_has

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] [day] [month] [year] [list]

Message-Id: <20220915144502.31e7241c6144f23ce197428b@linux-foundation.org>
Date:   Thu, 15 Sep 2022 14:45:02 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     "zhaoyang.huang" <zhaoyang.huang@...soc.com>
Cc:     Zhaoyang Huang <huangzhaoyang@...il.com>, <linux-mm@...ck.org>,
        <linux-kernel@...r.kernel.org>, <ke.wang@...soc.com>
Subject: Re: [PATCH] mm: change the type to bool for page_has_xxx's val

On Thu, 15 Sep 2022 10:56:40 +0800 "zhaoyang.huang" <zhaoyang.huang@...soc.com> wrote:

> From: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> 
> It is proper to return bool value for such functions
> 
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@...soc.com>
> ---
>  include/linux/page-flags.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index e66f7aa..9a46c8b 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -946,9 +946,9 @@ static inline bool is_page_hwpoison(struct page *page)
>  #define PageType(page, flag)						\
>  	((page->page_type & (PAGE_TYPE_BASE | flag)) == PAGE_TYPE_BASE)
>  
> -static inline int page_has_type(struct page *page)
> +static inline bool page_has_type(struct page *page)
>  {
> -	return (int)page->page_type < PAGE_MAPCOUNT_RESERVE;
> +	return !!((int)page->page_type < PAGE_MAPCOUNT_RESERVE);

Much nicer to simply do this:

	return page->page_type < PAGE_MAPCOUNT_RESERVE;

>  }
>  
>  #define PAGE_TYPE_OPS(uname, lname)					\
> @@ -1081,7 +1081,7 @@ static __always_inline void __ClearPageAnonExclusive(struct page *page)
>   * Determine if a page has private stuff, indicating that release routines
>   * should be invoked upon it.
>   */
> -static inline int page_has_private(struct page *page)
> +static inline bool page_has_private(struct page *page)
>  {
>  	return !!(page->flags & PAGE_FLAGS_PRIVATE);
>  }

Maybe.  This might cause the compiler to emit more code, to convert the
integer to a bool having value 0 or 1.  It would do so if the code was
uninlined.

But given that it's inlined, the compiler hopefully has the brains to
avoid doing that and to test the integer directly.  Please check this -
the above change shouldn't increase the .o file's text section size.