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

[Mike Kravetz]

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@xxxxxxxxxx>
> ---
>  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;
>  }
>  
>