| Message ID | 20200430105034.17513-1-l.stelmach@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: kexec_file: print appropriate variable | expand |
On Thu, Apr 30, 2020 at 12:50:34PM +0200, Łukasz Stelmach wrote: > Fixes: 4312057681929 ("arm64: kexec_file: load initrd and device-tree") > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> -ENOCOMMITMSG > --- > 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); I guess it would make sense to use kbuf.bufsz instead of dtb_len too. (assuming this is useful to somebody?) Will
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 As will said, this needs a commit message. Please explain what you think is wrong here. 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). > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> > --- > 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); ... and it looks like x86 does similar in kexec-bzimage64.c, for some sort of consistency with the old kexec logging. If <foo>_len and kbuf.memsz can differ, we should log that in all cases. If not, we should remove the redundant logging. Thanks, Mark.
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@samsung.com> >> --- >> 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,
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); return 0;
Fixes: 4312057681929 ("arm64: kexec_file: load initrd and device-tree") Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com> --- arch/arm64/kernel/machine_kexec_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)