Hi Lianbo,
I have checked my vmlinux again, and found that not contained the symbol
"vmcoreinfo_data" and "vmcoreinfo_size", but it has "kasan_enable_current".
So I think check "kasan_enable_current" when have symol "vmcoreinfo_data"
won't work as excepted.
/* 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 */
Maybe the previous patch is more suitable for this situation ?
+ if (kernel_symbol_exists("kasan_enable_current")) {
> + min_thread_shift += 1;
> + thread_shift = (min_thread_shift < machdep->pageshift) ?
> machdep->pageshift : min_thread_shift;
> + } else { // tbnz method
>
I found that my vmlinux not set CONFIG_KEXEC and other related kernel dump
configurations.
Maybe this is the reason that not contained "vmcoreinfo_data" symbol.
By the way, my kernel was running as a guest OS, and the dump file was
generated by the hypervisor.
Thanks
yeping
lijiang <liji...@redhat.com> 于2024年7月31日周三 17:36写道：
> 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
