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. So HASH_SMALL is obsolete and remove it.
Signed-off-by: Miaohe Lin <[email protected]>
---
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
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 <[email protected]>
> ---
> 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.
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 <[email protected]>
>> ---
>> 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
>>
>
On Sat, Jun 17, 2023 at 04:39:44PM +0800, Miaohe Lin wrote:
> 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?
You are right, I've missed that.
> Thanks.
>
> >
> >> So HASH_SMALL is obsolete and remove it.
> >
> >> Signed-off-by: Miaohe Lin <[email protected]>
> >> ---
> >> 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 */
Can you update HASH_ZERO to 0x2?
> >>
> >> /* 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.
On 2023/6/17 18:51, Mike Rapoport wrote:
> On Sat, Jun 17, 2023 at 04:39:44PM +0800, Miaohe Lin wrote:
>> 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?
>
> You are right, I've missed that.
>
>> Thanks.
>>
>>>
>>>> So HASH_SMALL is obsolete and remove it.
>>>
>>>> Signed-off-by: Miaohe Lin <[email protected]>
>>>> ---
>>>> 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 */
>
> Can you update HASH_ZERO to 0x2?
Will do. Thanks.