On Wed, Jul 31, 2024 at 3:34 PM yp z <wonder...@gmail.com> wrote: > Hi Lianbo, > > > Hi, sending a patch as attachment is bad, you can refer to the kernel > doc: > > Thank to Dave for pointing that out. I apologize for not realizing this > before, and resend it as following. > > > I think you are right, the else branch can work well regardless of > whether vmcoreinfo is present or not. So I changed the execution order of > the code: > > Step 1. Try to get THREAD_SHIFT from tbnz instruction. > > Step 2. If cannot get THREAD_SHIFT by step 1, change the thread_shift > when KASAN is enabled and with vmcoreinfo. > > Please help review this new patch. >
Hmm, No. I would recommend doing it as below: ... /* Having the vmcoreinfo data and enabling KASAN */ if (kernel_symbol_exists("vmcoreinfo_data") && kernel_symbol_exists("vmcoreinfo_size") && kernel_symbol_exists("kasan_enable_current")) { min_thread_shift += 1; thread_shift = (min_thread_shift < machdep->pageshift) ? machdep->pageshift : min_thread_shift; } else { sprintf(buf1, "x/32i vectors"); ... } ... if (!thread_shift) return 0; return ((1UL) << thread_shift); } When having the vmcoreinfo data and disabling KASAN, crash tool can work well without your patches, It should be like this: ... /* Having the vmcoreinfo data */ if (kernel_symbol_exists("vmcoreinfo_data") && kernel_symbol_exists("vmcoreinfo_size") { /* enabling KASAN? */ if (kernel_symbol_exists("kasan_enable_current")) { min_thread_shift += 1; thread_shift = (min_thread_shift < machdep->pageshift) ? machdep->pageshift : min_thread_shift; } else thread_shift = 0; /* Still use the value of ARM64_IRQ_STACK_SIZE */ } else { sprintf(buf1, "x/32i vectors"); ... } ... if (!thread_shift) return 0; return ((1UL) << thread_shift); } Just an idea. Can you help double check? Yeping. Thanks Lianbo > > --- > arm64.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-- > 1 file changed, 47 insertions(+), 2 deletions(-) > > diff --git a/arm64.c b/arm64.c > index 78e6609..49799e6 100644 > --- a/arm64.c > +++ b/arm64.c > @@ -94,6 +94,7 @@ static int arm64_is_uvaddr(ulong, struct task_context *); > static void arm64_calc_KERNELPACMASK(void); > static void arm64_recalc_KERNELPACMASK(void); > static int arm64_get_vmcoreinfo(unsigned long *vaddr, const char *label, > int base); > +static ulong arm64_set_irq_stack_size(void); > > struct kernel_range { > unsigned long modules_vaddr, modules_end; > @@ -2234,8 +2235,10 @@ arm64_irq_stack_init(void) > if (MEMBER_EXISTS("thread_union", "stack")) { > if ((sz = MEMBER_SIZE("thread_union", "stack")) > > 0) > ms->irq_stack_size = sz; > - } else > - ms->irq_stack_size = ARM64_IRQ_STACK_SIZE; > + } else { > + ulong res = arm64_set_irq_stack_size(); > + ms->irq_stack_size = (res > 0) ? res : > ARM64_IRQ_STACK_SIZE; > + } > > machdep->flags |= IRQ_STACKS; > > @@ -4950,6 +4953,48 @@ static void arm64_recalc_KERNELPACMASK(void){ > } > } > > +static ulong arm64_set_irq_stack_size(void) > +{ > + int min_thread_shift = 14; > + ulong thread_shift = 0; > + char buf1[BUFSIZE]; > + char *pos1, *pos2; > + int errflag = 0; > + > + sprintf(buf1, "x/32i vectors"); > + open_tmpfile(); > + if (!gdb_pass_through(buf1, pc->tmpfile, GNU_RETURN_ON_ERROR)) { > + goto out; > + } > + rewind(pc->tmpfile); > + while (fgets(buf1, BUFSIZE, pc->tmpfile)) { > + if ((pos1 = strstr(buf1, "tbnz"))) { > + if ((pos2 = strchr(pos1, '#'))) { > + pos2 += 1; > + for(pos1=pos2; *pos2!='\0' && *pos2!=','; > pos2++); > + *pos2 = '\0'; > + thread_shift = stol(pos1, > RETURN_ON_ERROR|QUIET, &errflag); > + if (errflag) { > + thread_shift = 0; > + } > + break; > + } > + } > + } > +out: > + close_tmpfile(); > + > + if ((!thread_shift) && > kernel_symbol_exists("kasan_enable_current")) { > + min_thread_shift += 1; > + thread_shift = (min_thread_shift < machdep->pageshift) ? > machdep->pageshift : min_thread_shift; > + } > + > + if (!thread_shift) > + return 0; > + else > + return ((1UL) << thread_shift); > +} > + > #endif /* ARM64 */ > > > Thanks > yeping. > > Dave Young <dyo...@redhat.com> 于2024年7月31日周三 14:58写道: > >> > Please help review this new patch. >> >> Hi, sending a patch as attachment is bad, you can refer to the kernel >> doc: >> https://www.kernel.org/doc/html/v4.14/process/email-clients.html >> >> "Patches for the Linux kernel are submitted via email, preferably as >> inline text in the body of the email. Some maintainers accept >> attachments, but then the attachments should have content-type >> text/plain. However, attachments are generally frowned upon because it >> makes quoting portions of the patch more difficult in the patch review >> process." >> >> Crash is different from kernel but the email process is similar, I >> think you can resend it with a proper format instead of maintainers >> manually copying them in the email body for review. >> >> Thanks >> Dave >> >>
-- Crash-utility mailing list -- devel@lists.crash-utility.osci.io To unsubscribe send an email to devel-le...@lists.crash-utility.osci.io https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ Contribution Guidelines: https://github.com/crash-utility/crash/wiki