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;
Message-ID: <f83025e2-d516-72f2-7053-4c5f9626dcf0@redhat.com>
Date:   Fri, 9 Sep 2022 13:34:52 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.0
Subject: Re: [PATCH 12/16] mm/page_alloc: use helper macro SZ_1{K,M}
Content-Language: en-US
To:     Miaohe Lin <linmiaohe@huawei.com>, akpm@linux-foundation.org
Cc:     osalvador@suse.de, linux-mm@kvack.org, linux-kernel@vger.kernel.org
References: <20220909092451.24883-1-linmiaohe@huawei.com>
 <20220909092451.24883-13-linmiaohe@huawei.com>
From:   David Hildenbrand <david@redhat.com>
Organization: Red Hat
In-Reply-To: <20220909092451.24883-13-linmiaohe@huawei.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 09.09.22 11:24, Miaohe Lin wrote:
> Use helper macro SZ_1K and SZ_1M to do the size conversion. Minor
> readability improvement.
> 
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>   mm/page_alloc.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 7a8a6bb08a15..e1c7f98cff96 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7053,7 +7053,7 @@ static int zone_batchsize(struct zone *zone)
>   	 * size is striking a balance between allocation latency
>   	 * and zone lock contention.
>   	 */
> -	batch = min(zone_managed_pages(zone) >> 10, (1024 * 1024) / PAGE_SIZE);
> +	batch = min(zone_managed_pages(zone) >> 10, SZ_1M / PAGE_SIZE);
>   	batch /= 4;		/* We effectively *= 4 below */
>   	if (batch < 1)
>   		batch = 1;
> @@ -8528,8 +8528,8 @@ void __init mem_init_print_info(void)
>   #endif
>   		")\n",
>   		K(nr_free_pages()), K(physpages),
> -		codesize >> 10, datasize >> 10, rosize >> 10,
> -		(init_data_size + init_code_size) >> 10, bss_size >> 10,
> +		codesize / SZ_1K, datasize / SZ_1K, rosize / SZ_1K,
> +		(init_data_size + init_code_size) / SZ_1K, bss_size / SZ_1K,
>   		K(physpages - totalram_pages() - totalcma_pages),
>   		K(totalcma_pages)
>   #ifdef	CONFIG_HIGHMEM
> @@ -9055,7 +9055,7 @@ void *__init alloc_large_system_hash(const char *tablename,
>   
>   		/* It isn't necessary when PAGE_SIZE >= 1MB */

Huh, how could we ever have that. Smells like dead code.

>   		if (PAGE_SHIFT < 20)

What about adjusting that as well? The it exactly matches the comment

if (PAGE_SIZE >= SZ_1M)

> -			numentries = round_up(numentries, (1<<20)/PAGE_SIZE);
> +			numentries = round_up(numentries, SZ_1M / PAGE_SIZE);
>   
>   #if __BITS_PER_LONG > 32
>   		if (!high_limit) {

-- 
Thanks,

David / dhildenb