diff mbox series

[v2,2/3] riscv: End kernel region search in setup_bootmem earlier

Message ID b11898805c2f9f01b10867a05701aa0fafeaa886.1581767384.git.jan.kiszka@web.de (mailing list archive)
State New, archived
Headers show
Series riscv: mem= support, ioremap exec fix | expand

Commit Message

Jan Kiszka Feb. 15, 2020, 11:49 a.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

No need to look further when that single region is found.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 arch/riscv/mm/init.c | 2 ++
 1 file changed, 2 insertions(+)

--
2.16.4

Comments

Alexandre Ghiti Feb. 16, 2020, 2:42 p.m. UTC | #1
Hi Jan,

On 2/15/20 6:49 AM, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> No need to look further when that single region is found.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> =2D--
>   arch/riscv/mm/init.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index aec39a56d6cf..a774547e9021 100644
> =2D-- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -160,6 +160,8 @@ void __init setup_bootmem(void)
>   			if (reg->base + mem_size < end)
>   				memblock_remove(reg->base + mem_size,
>   						end - reg->base - mem_size);
> +
> +			break;
>   		}
>   	}
>   	BUG_ON(mem_size =3D=3D 0);
> =2D-
> 2.16.4
> 
> 

I was looking at the test above that determines if the current memblock 
contains the kernel:

if (reg->base <= vmlinux_end && vmlinux_end <= end)

Shouldn't it be:

if (reg->base <= vmlinux_start && vmlinux_end <= end)

?

Otherwise, we can indeed stop as soon as we found the region containing 
the kernel, so feel free to add:

Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>

Thanks,

Alex
Jan Kiszka Feb. 16, 2020, 4:06 p.m. UTC | #2
On 16.02.20 15:42, Alex Ghiti wrote:
> Hi Jan,
>
> On 2/15/20 6:49 AM, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> No need to look further when that single region is found.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> =2D--
>>   arch/riscv/mm/init.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index aec39a56d6cf..a774547e9021 100644
>> =2D-- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -160,6 +160,8 @@ void __init setup_bootmem(void)
>>               if (reg->base + mem_size < end)
>>                   memblock_remove(reg->base + mem_size,
>>                           end - reg->base - mem_size);
>> +
>> +            break;
>>           }
>>       }
>>       BUG_ON(mem_size =3D=3D 0);
>> =2D-
>> 2.16.4
>>
>>
>
> I was looking at the test above that determines if the current memblock
> contains the kernel:
>
> if (reg->base <= vmlinux_end && vmlinux_end <= end)
>
> Shouldn't it be:
>
> if (reg->base <= vmlinux_start && vmlinux_end <= end)
>
> ?

Yes, I think you are right. Would you like to send a patch that fixes this?

>
> Otherwise, we can indeed stop as soon as we found the region containing
> the kernel, so feel free to add:
>
> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
>

Thanks,
Jan
Alexandre Ghiti Feb. 16, 2020, 7:57 p.m. UTC | #3
On 2/16/20 11:06 AM, Jan Kiszka wrote:
> On 16.02.20 15:42, Alex Ghiti wrote:
>> Hi Jan,
>>
>> On 2/15/20 6:49 AM, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> No need to look further when that single region is found.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> =2D--
>>>   arch/riscv/mm/init.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>>> index aec39a56d6cf..a774547e9021 100644
>>> =2D-- a/arch/riscv/mm/init.c
>>> +++ b/arch/riscv/mm/init.c
>>> @@ -160,6 +160,8 @@ void __init setup_bootmem(void)
>>>               if (reg->base + mem_size < end)
>>>                   memblock_remove(reg->base + mem_size,
>>>                           end - reg->base - mem_size);
>>> +
>>> +            break;
>>>           }
>>>       }
>>>       BUG_ON(mem_size =3D=3D 0);
>>> =2D-
>>> 2.16.4
>>>
>>>
>>
>> I was looking at the test above that determines if the current memblock
>> contains the kernel:
>>
>> if (reg->base <= vmlinux_end && vmlinux_end <= end)
>>
>> Shouldn't it be:
>>
>> if (reg->base <= vmlinux_start && vmlinux_end <= end)
>>
>> ?
> 
> Yes, I think you are right. Would you like to send a patch that fixes this?

Thanks for confirming, I'll send a patch tomorrow and cc stable too.

Alex

> 
>>
>> Otherwise, we can indeed stop as soon as we found the region containing
>> the kernel, so feel free to add:
>>
>> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
>>
> 
> Thanks,
> Jan
>
diff mbox series

Patch

diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index aec39a56d6cf..a774547e9021 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -160,6 +160,8 @@  void __init setup_bootmem(void)
 			if (reg->base + mem_size < end)
 				memblock_remove(reg->base + mem_size,
 						end - reg->base - mem_size);
+
+			break;
 		}
 	}
 	BUG_ON(mem_size == 0);