linux-kernel - Re: [PATCH] hugetlbfs: make hugepage size conversion more readable

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: <668845df-e654-ecdc-c32a-b50a22098333@oracle.com>
Date:   Thu, 21 Jan 2021 11:00:16 -0800
From:   Mike Kravetz <mike.kravetz@...cle.com>
To:     Miaohe Lin <linmiaohe@...wei.com>, akpm@...ux-foundation.org
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] hugetlbfs: make hugepage size conversion more readable

On 1/20/21 1:23 AM, Miaohe Lin wrote:
> The calculation 1U << (h->order + PAGE_SHIFT - 10) is actually equal to
> (PAGE_SHIFT << (h->order)) >> 10. So we can make it more readable by
> replace it with huge_page_size(h) / SZ_1K.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@...wei.com>
> ---
>  fs/hugetlbfs/inode.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index 25c1857ff45d..f94b8f6553fa 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -1519,8 +1519,8 @@ static struct vfsmount *__init mount_one_hugetlbfs(struct hstate *h)
>  		put_fs_context(fc);
>  	}
>  	if (IS_ERR(mnt))
> -		pr_err("Cannot mount internal hugetlbfs for page size %uK",
> -		       1U << (h->order + PAGE_SHIFT - 10));
> +		pr_err("Cannot mount internal hugetlbfs for page size %luK",
> +		       huge_page_size(h) / SZ_1K);

I appreciate the effort to make the code more readable.  The existing
calculation does take a minute to understand.  However, it is correct and
anyone modifying the code should be able to understand.

With my compiler, your proposed change adds an additional instruction to
the routine mount_one_hugetlbfs.  I know this is not significant, but still
it does increase the kernel size for a change that is of questionable value.

In the kernel, size in KB is often calculated as (size << (PAGE_SHIFT - 10)).
If you change the calculation in the hugetlb code to be:

			huge_page_size(h) << (PAGE_SHIFT - 10)

my compiler will actually reduce the size of the routine by one instruction.
-- 
Mike Kravetz

>  	return mnt;
>  }
>  
>