linux-kernel - Re: [PATCH] mm/mm_init.c: remove obsolete macro HASH

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: <6d149902-a944-7961-89c1-f07047953c7f@huawei.com>
Date:   Sat, 17 Jun 2023 16:39:44 +0800
From:   Miaohe Lin <linmiaohe@...wei.com>
To:     Mike Rapoport <rppt@...nel.org>
CC:     <akpm@...ux-foundation.org>, <tglx@...utronix.de>,
        <mingo@...hat.com>, <peterz@...radead.org>, <dvhart@...radead.org>,
        <dave@...olabs.net>, <andrealmeid@...lia.com>,
        <linux-mm@...ck.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm/mm_init.c: remove obsolete macro HASH_SMALL

On 2023/6/17 15:56, Mike Rapoport wrote:
> 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. 
> 

Thanks for your quick review.

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

When CONFIG_BASE_SMALL is set, futex_hashsize is set to 16 and alloc_large_system_hash() is called with
numentries == 16 && flags == HASH_SMALL. But in the alloc_large_system_hash(), we have the below logic:

alloc_large_system_hash()
{
  if (!numentries) { /* numentries == 16 here, so this code block is skipped. */
    ...
    if (unlikely(flags & HASH_SMALL)) { /* So as here. */
      ...
  }
  ...
}

So HASH_SMALL is just unused. Or am I miss something?

Thanks.

> 
>> So HASH_SMALL is obsolete and remove it.
> 
>> Signed-off-by: Miaohe Lin <linmiaohe@...wei.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
>>
>