Hi Lianbo, Thank you for your input. It looks good to me, and it tests just fine. Sorry, not calling the routine twice should

have come to my mind before submitting. Would you like for me to resubmit a patch with the modifications, or would you like to take care of the next steps? This is the first patch I've submitted so I'm not totally sure.

Hopefully more to come in the future!

Best regards, Lucas On Wed, Dec 18, 2024 at 10:46 PM lijiang <lijiang(a)redhat.com&gt; wrote: > Hi, Lucas > Thank you for the fix. > On Tue, Dec 17, 2024 at 12:14 PM < > devel-request(a)lists.crash-utility.osci.io&gt; wrote: > >> Date: Sat, 14 Dec 2024 18:01:14 -0500 >> From: Lucas Oakley <soakley(a)redhat.com&gt; >> Subject: [Crash-utility] [PATCH] Fix incorrect 'bt -v' output >> suggesting overflow >> To: devel(a)lists.crash-utility.osci.io >> Message-ID: <20241214230114.2854910-1-soakley(a)redhat.com&gt; >> Content-Type: text/plain; charset="US-ASCII"; x-default=true >> >> Change check_stack_overflow() to check if the thread_info's cpu >> member is smaller than possible existing CPUs, rather than the >> kernel table's cpu number (kt->cpus). The kernel table's cpu number >> is changed on some architectures to reflect the highest numbered >> online cpu + 1. This can cause a false positive in >> check_stack_overflow() if the cpu member of a parked task's >> thread_info structure, assigned to an offlined cpu, is larger than >> the kt->cpus but lower than the number of existing logical cpus. >> An example of this is RHEL 7 on s390x or RHEL 8 on ppc64le when >> the highest numbered CPU is offlined. >> >> Signed-off-by: Lucas Oakley <soakley(a)redhat.com&gt; >> --- >> task.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/task.c b/task.c >> index 33de7da..93dab0e 100644 >> --- a/task.c >> +++ b/task.c >> @@ -11253,12 +11253,12 @@ check_stack_overflow(void) >> cpu = 0; >> break; >> } >> - if (cpu >= kt->cpus) { >> + if (cpu >= get_cpus_present()) { >> if (!overflow) >> print_task_header(fp, tc, 0); >> fprintf(fp, >> " possible stack overflow: >> thread_info.cpu: %d >= %d\n", >> - cpu, kt->cpus); >> + cpu, get_cpus_present()); >> overflow++; total++; >> } >> } >> > > To avoid calling get_cpus_present() twice, I would tend to modify it as > below: > > diff --git a/task.c b/task.c > index 33de7da2a692..49f771e275c1 100644 > --- a/task.c > +++ b/task.c > @@ -11238,6 +11238,8 @@ check_stack_overflow(void) > } > > if (VALID_MEMBER(thread_info_cpu)) { > + int cpus = get_cpus_present(); > + > switch (cpu_size) > { > case 1: > @@ -11253,12 +11255,12 @@ check_stack_overflow(void) > cpu = 0; > break; > } > - if (cpu >= kt->cpus) { > + if (cpu >= cpus) { > if (!overflow) > print_task_header(fp, tc, 0); > fprintf(fp, > " possible stack overflow: > thread_info.cpu: %d >= %d\n", > - cpu, kt->cpus); > + cpu, cpus); > overflow++; total++; > } > } > > What do you think? > > > Lianbo > > -- >> 2.47.1 >> >