Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 13 Jun 2022 10:33:48 +0200
From:   Oscar Salvador <osalvador@suse.de>
To:     Muchun Song <songmuchun@bytedance.com>
Cc:     mike.kravetz@oracle.com, david@redhat.com,
        akpm@linux-foundation.org, corbet@lwn.net, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [PATCH 6/6] mm: hugetlb_vmemmap: improve hugetlb_vmemmap code
 readability
Message-ID: <Yqb2bA25HhLU/mpM@localhost.localdomain>
References: <20220613063512.17540-1-songmuchun@bytedance.com>
 <20220613063512.17540-7-songmuchun@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220613063512.17540-7-songmuchun@bytedance.com>
Precedence: bulk

On Mon, Jun 13, 2022 at 02:35:12PM +0800, Muchun Song wrote:
> -static __init int hugetlb_vmemmap_sysctls_init(void)
> +static int __init hugetlb_vmemmap_init(void)
>  {
> +	const struct hstate *h;
> +	bool optimizable = false;
> +
>  	/*
> -	 * If "struct page" crosses page boundaries, the vmemmap pages cannot
> -	 * be optimized.
> +	 * There are only (RESERVE_VMEMMAP_SIZE / sizeof(struct page)) struct
> +	 * page structs that can be used when HVO is enabled.
>  	 */
> -	if (is_power_of_2(sizeof(struct page)))
> -		register_sysctl_init("vm", hugetlb_vmemmap_sysctls);
> +	BUILD_BUG_ON(__NR_USED_SUBPAGE >= RESERVE_VMEMMAP_SIZE / sizeof(struct page));

I need to take another look, but from the first glance there is something
here that caught my eye.

> +
> +	for_each_hstate(h) {
> +		char buf[16];
> +		unsigned int size = 0;
> +
> +		if (hugetlb_vmemmap_optimizable(h))
> +			size = hugetlb_vmemmap_size(h) - RESERVE_VMEMMAP_SIZE;
> +		optimizable = size ? true : optimizable;

This feels weird, just use false instead of optimizable.

> +		string_get_size(huge_page_size(h), 1, STRING_UNITS_2, buf,
> +				sizeof(buf));
> +		pr_info("%d KiB vmemmap can be optimized for a %s page\n",
> +			size / SZ_1K, buf);

I do not have a strong opinion but I wonder whether this brings a lot.


-- 
Oscar Salvador
SUSE Labs