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:   Sat, 17 Jun 2023 10:56:44 +0300
From:   Mike Rapoport <rppt@kernel.org>
To:     Miaohe Lin <linmiaohe@huawei.com>
Cc:     akpm@linux-foundation.org, tglx@linutronix.de, mingo@redhat.com,
        peterz@infradead.org, dvhart@infradead.org, dave@stgolabs.net,
        andrealmeid@igalia.com, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm/mm_init.c: remove obsolete macro HASH_SMALL
Message-ID: <20230617075644.GV52412@kernel.org>
References: <20230617070955.1751393-1-linmiaohe@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230617070955.1751393-1-linmiaohe@huawei.com>
Precedence: bulk

On Sat, Jun 17, 2023 at 03:09:55PM +0800, Miaohe Lin wrote:
> HASH_SMALL only works when parameter numentries is 0. But the sole caller
> futex_init() never calls alloc_large_system_hash() with numentries set to
> 0. 

Doesn't it? 
What happens when CONFIG_BASE_SMALL is set?

> So HASH_SMALL is obsolete and remove it.

> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
>  include/linux/memblock.h |  2 --
>  kernel/futex/core.c      |  3 +--
>  mm/mm_init.c             | 10 +---------
>  3 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> index f71ff9f0ec81..346d80809517 100644
> --- a/include/linux/memblock.h
> +++ b/include/linux/memblock.h
> @@ -581,8 +581,6 @@ extern void *alloc_large_system_hash(const char *tablename,
>  				     unsigned long high_limit);
>  
>  #define HASH_EARLY	0x00000001	/* Allocating during early boot? */
> -#define HASH_SMALL	0x00000002	/* sub-page allocation allowed, min
> -					 * shift passed via *_hash_shift */
>  #define HASH_ZERO	0x00000004	/* Zero allocated hash table */
>  
>  /* Only NUMA needs hash distribution. 64bit NUMA architectures have
> diff --git a/kernel/futex/core.c b/kernel/futex/core.c
> index 514e4582b863..f10587d1d481 100644
> --- a/kernel/futex/core.c
> +++ b/kernel/futex/core.c
> @@ -1132,8 +1132,7 @@ static int __init futex_init(void)
>  #endif
>  
>  	futex_queues = alloc_large_system_hash("futex", sizeof(*futex_queues),
> -					       futex_hashsize, 0,
> -					       futex_hashsize < 256 ? HASH_SMALL : 0,
> +					       futex_hashsize, 0, 0,
>  					       &futex_shift, NULL,
>  					       futex_hashsize, futex_hashsize);
>  	futex_hashsize = 1UL << futex_shift;
> diff --git a/mm/mm_init.c b/mm/mm_init.c
> index d393631599a7..fab3c4649d5b 100644
> --- a/mm/mm_init.c
> +++ b/mm/mm_init.c
> @@ -2492,15 +2492,7 @@ void *__init alloc_large_system_hash(const char *tablename,
>  		else
>  			numentries <<= (PAGE_SHIFT - scale);
>  
> -		/* Make sure we've got at least a 0-order allocation.. */
> -		if (unlikely(flags & HASH_SMALL)) {
> -			/* Makes no sense without HASH_EARLY */
> -			WARN_ON(!(flags & HASH_EARLY));
> -			if (!(numentries >> *_hash_shift)) {
> -				numentries = 1UL << *_hash_shift;
> -				BUG_ON(!numentries);
> -			}
> -		} else if (unlikely((numentries * bucketsize) < PAGE_SIZE))
> +		if (unlikely((numentries * bucketsize) < PAGE_SIZE))
>  			numentries = PAGE_SIZE / bucketsize;
>  	}
>  	numentries = roundup_pow_of_two(numentries);
> -- 
> 2.27.0
> 

-- 
Sincerely yours,
Mike.