Re: [PATCH] mm: change the type to bool for page_has_xxx's val

From: Andrew Morton
Date: Thu Sep 15 2022 - 17:45:14 EST


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

> From: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
>
> It is proper to return bool value for such functions
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@xxxxxxxxxx>
> ---
> 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.