<div dir="ltr"><div dir="ltr">On Thu, Jun 8, 2023 at 5:56 PM lijiang <<a href="mailto:lijiang@redhat.com">lijiang@redhat.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr">On Thu, Jun 8, 2023 at 5:02 PM HAGIO KAZUHITO(萩尾 一仁) <<a href="mailto:k-hagio-ab@nec.com" target="_blank">k-hagio-ab@nec.com</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2023/06/08 13:31, Lianbo Jiang wrote:<br>
> Crash tool will fail to load vmcore with the following error:<br>
> <br>
> #crash vmlinux /var/crash/127.0.0.1-2023-06-07-22\:03\:24/vmcore -s<br>
> crash: invalid structure size: note_buf<br>
> FILE: diskdump.c LINE: 121 FUNCTION: have_crash_notes()<br>
> <br>
> [./crash] error trace: 101859ac => 10291798 => 10291450 => 10266038<br>
> <br>
> 10266038: SIZE_verify+156<br>
> 10291450: have_crash_notes+308<br>
> 10291798: map_cpus_to_prstatus_kdump_cmprs+448<br>
> 101859ac: task_init+11980<br>
> <br>
> The reason is that the note_buf is not intialized before using the<br>
> SIZE(note_buf) in the have_crash_notes(). Let's initialize the variable<br>
> note_buf in the ppc64_init() to fix this issue.<br>
<br>
Thanks for the patch.<br>
<br>
Most of the architectures already have it, but some still do not have,</blockquote><div><br></div><div>I don't have a 4 cpus s390x machine on hand, and am still trying to find it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
e.g. is s390x ok? It might be better to put it in task_init() because</blockquote><div> </div><div>It was initialized in the machdep_init(POST_GDB), see the following code:</div><div>...</div><div> read_in_kernel_config(IKCFG_INIT);<br> kernel_init();<br> machdep_init(POST_GDB); <---<br> vm_init();<br> machdep_init(POST_VM);<br> module_init();<br> help_init();<br> task_init();<br> vfs_init();<br> net_init();<br> dev_init();<br> machdep_init(POST_INIT);<br></div><div>...</div><div>I would suggest putting it at the end of the kernel_init(), so that the original initialization order will basically no be changed</div></div></div></blockquote><div> </div><div>Sorry, It's a typo. I mean that:</div><div><br></div><div>I would suggest putting it at the end of the kernel_init(), so that the original initialization order will basically not be changed, and which does not cause additional risks.<br></div><div><br></div><div>Thanks.</div><div>Lianbo</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> <br>
it's used there?<br></blockquote><div> </div><div>Do you mean we need to move the existing initialization(all) to the task_init()? </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
$ git grep 'STRUCT_SIZE_INIT(note_buf'<br>
arm.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
arm64.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
mips.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
mips64.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
ppc.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
riscv64.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
x86.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
x86_64.c: STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
xen_hyper.c: XEN_HYPER_STRUCT_SIZE_INIT(note_buf_t, "note_buf_t");<br>
<br>
Thanks,<br>
Kazu<br>
<br>
> <br>
> Fixes: db8c030857b4 ("diskdump/netdump: fix segmentation fault caused by failure of stopping CPUs")<br>
> Signed-off-by: Lianbo Jiang <<a href="mailto:lijiang@redhat.com" target="_blank">lijiang@redhat.com</a>><br>
> ---<br>
> ppc64.c | 1 +<br>
> 1 file changed, 1 insertion(+)<br>
> <br>
> diff --git a/ppc64.c b/ppc64.c<br>
> index b95a621d8fe4..ff7f0fca3a95 100644<br>
> --- a/ppc64.c<br>
> +++ b/ppc64.c<br>
> @@ -695,6 +695,7 @@ ppc64_init(int when)<br>
> }<br>
> <br>
> ppc64_init_paca_info();<br>
> + STRUCT_SIZE_INIT(note_buf, "note_buf_t");<br>
> <br>
> if (!machdep->hz) {<br>
> machdep->hz = HZ;</blockquote></div></div>
</blockquote></div></div>