Re: [PATCH] arm64: kexec_file: print appropriate variable

[Lukasz Stelmach]

It was <2020-04-30 czw 12:19>, when Mark Rutland wrote:
> On Thu, Apr 30, 2020 at 12:50:34PM +0200, Åukasz Stelmach wrote:
>> Fixes: 4312057681929 ("arm64: kexec_file: load initrd and device-tree")
>
> This commit ID is bogus (doesn't exist in mainline or the arm64 tree).
>
> The upstream commit ID seems to be: 52b2a8af7436044cfcb27e4b0f72c2ce1f3890da

Fixing.

> As will said, this needs a commit message. Please explain what you think
> is wrong here.

Fixing.

> Also, when sending a fix, *please* Cc the author of the original patch.
>
> I've added parties relevant to the original patch (Takahiro and James).

Thank you.

>> Signed-off-by: Åukasz Stelmach <l.stelmach@xxxxxxxxxxx>
>> ---
>>  arch/arm64/kernel/machine_kexec_file.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
>> index b40c3b0def92..2776bdaa83a5 100644
>> --- a/arch/arm64/kernel/machine_kexec_file.c
>> +++ b/arch/arm64/kernel/machine_kexec_file.c
>> @@ -332,7 +332,7 @@ int load_other_segments(struct kimage *image,
>>  	image->arch.dtb_mem = kbuf.mem;
>>  
>>  	pr_debug("Loaded dtb at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
>> -			kbuf.mem, dtb_len, dtb_len);
>> +			kbuf.mem, dtb_len, kbuf.memsz);
>
> It's worth noting that we follow the same pattern repeatedly in this
> file, so if you think this instance is wrong you should consider whether
> the others are correct.
>
> Earlier in this file we have:
>
> |	pr_debug("Loaded elf core header at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> |		 image->arch.elf_headers_mem, headers_sz, headers_sz)
>
> |	pr_debug("Loaded initrd at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
> |		 initrd_load_addr, initrd_len, initrd_len);

Fixing.

> ... and it looks like x86 does similar in kexec-bzimage64.c, for some
> sort of consistency with the old kexec logging.

When I fix it for x86, should I combine changes in one patch or prepare
two separate patches?

> If <foo>_len and kbuf.memsz can differ, we should log that in all cases.
> If not, we should remove the redundant logging.

Yes, memsz is page-aligned in kexec_add_buffer();

Kind regards,
-- 
Åukasz Stelmach
Samsung R&D Institute Poland
Samsung Electronics

Attachment: signature.asc
Description: PGP signature