[BUG] Module load relocation hashtable size can become huge and unallocatable

[BUG] Module load relocation hashtable size can become huge and unallocatable

[William Pierce]

Hello, I have a kernel module that requires a huge number
of relocations upon loading.

On other architectures, it loads into the kernel just fine.
However, on riscv, while allocating memory for its relocation
hashtable, it ends up requesting 2097152 elements to kmalloc_array.

Right here I've dumped the values by recompiling the kernel:
num_relocations: 1630985, hastable_size: 2097152
https://elixir.bootlin.com/linux/v6.14-rc6/source/arch/riscv/kernel/module.c#L734

which kmalloc_array->...->___kmalloc_large_node->...->
__alloc_frozen_pages_noprof can't handle and returns
"Cannot allocate memory" on insmod.

How would it be recommended to fix this?
I could imagine allocating a more discontiguous data structure
to track the relocations or otherwise reworking the tracking to
do the relocation tracking inline.

Thank you,
Will Pierce

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [BUG] Module load relocation hashtable size can become huge and unallocatable

[William Pierce]

Using vmalloc_array/vfree on the relocation_hashtable
allows the kernel module to load just fine. I can send this
as a patch if the fix sounds good.

On 3/30/25 1:12 PM, William Pierce wrote:
> Hello, I have a kernel module that requires a huge number
> of relocations upon loading.
> 
> On other architectures, it loads into the kernel just fine.
> However, on riscv, while allocating memory for its relocation
> hashtable, it ends up requesting 2097152 elements to kmalloc_array.
> 
> Right here I've dumped the values by recompiling the kernel:
> num_relocations: 1630985, hastable_size: 2097152
> https://elixir.bootlin.com/linux/v6.14-rc6/source/arch/riscv/kernel/module.c#L734
> 
> which kmalloc_array->...->___kmalloc_large_node->...->
> __alloc_frozen_pages_noprof can't handle and returns
> "Cannot allocate memory" on insmod.
> 
> How would it be recommended to fix this?
> I could imagine allocating a more discontiguous data structure
> to track the relocations or otherwise reworking the tracking to
> do the relocation tracking inline.
> 
> Thank you,
> Will Pierce


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [BUG] Module load relocation hashtable size can become huge and unallocatable

[Clément Léger]

On 01/04/2025 05:10, William Pierce wrote:
> Using vmalloc_array/vfree on the relocation_hashtable
> allows the kernel module to load just fine. I can send this
> as a patch if the fix sounds good.

Hey William,

Using vmalloc sounds reasonable since there is no need to have
physically contiguous memory.

However, there is also a new API named kvmalloc which uses kmalloc for
small sizes and vmalloc for large allocations exceeding kmalloc
capabilities, potentially leading to better performances than using
vmalloc only.

Clément

> 
> On 3/30/25 1:12 PM, William Pierce wrote:
>> Hello, I have a kernel module that requires a huge number
>> of relocations upon loading.
>>
>> On other architectures, it loads into the kernel just fine.
>> However, on riscv, while allocating memory for its relocation
>> hashtable, it ends up requesting 2097152 elements to kmalloc_array.
>>
>> Right here I've dumped the values by recompiling the kernel:
>> num_relocations: 1630985, hastable_size: 2097152
>> https://elixir.bootlin.com/linux/v6.14-rc6/source/arch/riscv/kernel/module.c#L734
>>
>> which kmalloc_array->...->___kmalloc_large_node->...->
>> __alloc_frozen_pages_noprof can't handle and returns
>> "Cannot allocate memory" on insmod.
>>
>> How would it be recommended to fix this?
>> I could imagine allocating a more discontiguous data structure
>> to track the relocations or otherwise reworking the tracking to
>> do the relocation tracking inline.
>>
>> Thank you,
>> Will Pierce
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [BUG] Module load relocation hashtable size can become huge and unallocatable

[William Pierce]

On 4/1/25 12:55 AM, Clément Léger wrote:
> 
> 
> On 01/04/2025 05:10, William Pierce wrote:
>> Using vmalloc_array/vfree on the relocation_hashtable
>> allows the kernel module to load just fine. I can send this
>> as a patch if the fix sounds good.
> 
> Hey William,
> 
> Using vmalloc sounds reasonable since there is no need to have
> physically contiguous memory.
> 
> However, there is also a new API named kvmalloc which uses kmalloc for
> small sizes and vmalloc for large allocations exceeding kmalloc
> capabilities, potentially leading to better performances than using
> vmalloc only.
> 
> Clément
> 

Hi Clément,

Thank you for checking. Yes I expect kvmalloc should also work just fine.

I can send out a patch based on that, unless someone wants to just take
the change. I'll do so in the next day or so.

- Will

>>
>> On 3/30/25 1:12 PM, William Pierce wrote:
>>> Hello, I have a kernel module that requires a huge number
>>> of relocations upon loading.
>>>
>>> On other architectures, it loads into the kernel just fine.
>>> However, on riscv, while allocating memory for its relocation
>>> hashtable, it ends up requesting 2097152 elements to kmalloc_array.
>>>
>>> Right here I've dumped the values by recompiling the kernel:
>>> num_relocations: 1630985, hastable_size: 2097152
>>> https://elixir.bootlin.com/linux/v6.14-rc6/source/arch/riscv/kernel/module.c#L734
>>>
>>> which kmalloc_array->...->___kmalloc_large_node->...->
>>> __alloc_frozen_pages_noprof can't handle and returns
>>> "Cannot allocate memory" on insmod.
>>>
>>> How would it be recommended to fix this?
>>> I could imagine allocating a more discontiguous data structure
>>> to track the relocations or otherwise reworking the tracking to
>>> do the relocation tracking inline.
>>>
>>> Thank you,
>>> Will Pierce
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH] riscv: Use kvmalloc_array on relocation_hashtable

[wgpierce17]

From: Will Pierce <wgpierce17@gmail.com>

The number of relocations may be a huge value that is unallocatable
by kmalloc. Use kvmalloc instead so that it does not fail.

Signed-off-by: Will Pierce <wgpierce17@gmail.com>
---
 arch/riscv/kernel/module.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 47d0ebeec93c..bae3db50647c 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -648,7 +648,7 @@ process_accumulated_relocations(struct module *me,
 		kfree(bucket_iter);
 	}
 
-	kfree(*relocation_hashtable);
+	kvfree(*relocation_hashtable);
 }
 
 static int add_relocation_to_accumulate(struct module *me, int type,
@@ -752,7 +752,8 @@ initialize_relocation_hashtable(unsigned int num_relocations,
 
 	hashtable_size <<= should_double_size;
 
-	*relocation_hashtable = kmalloc_array(hashtable_size,
+	/* Number of relocations may be large, so kvmalloc it */
+	*relocation_hashtable = kvmalloc_array(hashtable_size,
 					      sizeof(**relocation_hashtable),
 					      GFP_KERNEL);
 	if (!*relocation_hashtable)
-- 
2.49.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Use kvmalloc_array on relocation_hashtable

[Alexandre Ghiti]

Hi Will,

On 02/04/2025 07:34, wgpierce17@gmail.com wrote:
> From: Will Pierce <wgpierce17@gmail.com>
>
> The number of relocations may be a huge value that is unallocatable
> by kmalloc. Use kvmalloc instead so that it does not fail.
>
> Signed-off-by: Will Pierce <wgpierce17@gmail.com>
> ---
>   arch/riscv/kernel/module.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 47d0ebeec93c..bae3db50647c 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -648,7 +648,7 @@ process_accumulated_relocations(struct module *me,
>   		kfree(bucket_iter);
>   	}
>   
> -	kfree(*relocation_hashtable);
> +	kvfree(*relocation_hashtable);
>   }
>   
>   static int add_relocation_to_accumulate(struct module *me, int type,
> @@ -752,7 +752,8 @@ initialize_relocation_hashtable(unsigned int num_relocations,
>   
>   	hashtable_size <<= should_double_size;
>   
> -	*relocation_hashtable = kmalloc_array(hashtable_size,
> +	/* Number of relocations may be large, so kvmalloc it */
> +	*relocation_hashtable = kvmalloc_array(hashtable_size,
>   					      sizeof(**relocation_hashtable),
>   					      GFP_KERNEL);
>   	if (!*relocation_hashtable)


Let's add a fixes tag:

Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")

Since Clément suggested this solution:

Suggested-by: Clément Léger <cleger@rivosinc.com>

And finally, this looks good to me so:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

It's a bit late for rc1 so I'll take this one for a next rc.

Thanks for the patch!

Alex



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH] riscv: Use kvmalloc_array on relocation_hashtable

[William Pierce]

Hi Alexandre,

On 4/1/25 11:56 PM, Alexandre Ghiti wrote:
> Hi Will,
> 
> On 02/04/2025 07:34, wgpierce17@gmail.com wrote:
>> From: Will Pierce <wgpierce17@gmail.com>
>>
>> The number of relocations may be a huge value that is unallocatable
>> by kmalloc. Use kvmalloc instead so that it does not fail.
>>
>> Signed-off-by: Will Pierce <wgpierce17@gmail.com>
>> ---
>>   arch/riscv/kernel/module.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
>> index 47d0ebeec93c..bae3db50647c 100644
>> --- a/arch/riscv/kernel/module.c
>> +++ b/arch/riscv/kernel/module.c
>> @@ -648,7 +648,7 @@ process_accumulated_relocations(struct module *me,
>>           kfree(bucket_iter);
>>       }
>>   -    kfree(*relocation_hashtable);
>> +    kvfree(*relocation_hashtable);
>>   }
>>     static int add_relocation_to_accumulate(struct module *me, int type,
>> @@ -752,7 +752,8 @@ initialize_relocation_hashtable(unsigned int num_relocations,
>>         hashtable_size <<= should_double_size;
>>   -    *relocation_hashtable = kmalloc_array(hashtable_size,
>> +    /* Number of relocations may be large, so kvmalloc it */
>> +    *relocation_hashtable = kvmalloc_array(hashtable_size,
>>                             sizeof(**relocation_hashtable),
>>                             GFP_KERNEL);
>>       if (!*relocation_hashtable)
> 
> 
> Let's add a fixes tag:
> 
> Fixes: 8fd6c5142395 ("riscv: Add remaining module relocations")
> 
> Since Clément suggested this solution:
> 
> Suggested-by: Clément Léger <cleger@rivosinc.com>
> 
> And finally, this looks good to me so:
> 
> Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> 
> It's a bit late for rc1 so I'll take this one for a next rc.
> 
> Thanks for the patch!
> 
> Alex
> 
> 

Thank you for checking. I will send a v2 with these additions and
a minor whitespace fix right now.

- Will

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv