[Crash-utility] Re: [PATCH v1 2/5] Call cmd_bt silently after "set pid"

Hi, Tao and Alex Thank you for working on this. On Fri, Jan 3, 2025 at 1:54 PM <devel-request(a)lists.crash-utility.osci.io&gt; wrote: > > Date: Fri, 3 Jan 2025 18:48:14 +1300 > From: Tao Liu <ltao(a)redhat.com&gt; > Subject: [Crash-utility] [PATCH v1 2/5] Call cmd_bt silently after > "set pid" > To: devel(a)lists.crash-utility.osci.io > Cc: alexey.makhalov(a)broadcom.com > Message-ID: <20250103054817.364053-3-ltao(a)redhat.com&gt; > Content-Type: text/plain; charset="US-ASCII"; x-default=true > > Cmd bt will list multi-stacks of one task. After we "set <pid>" switch > context to one task, we first need a bt call to detect the multi-stacks, > however we don't want any console output from it, so a nullfp is used for > output receive. The silent bt call is only triggered once as part of task > context switch by cmd set. > > A array of user_regs pointers is reserved for each supported arch. If one > extra stack found, a user_regs structure will be allocated for storing regs > value of the stack. > > Co-developed-by: Alexey Makhalov <alexey.makhalov(a)broadcom.com&gt; > Co-developed-by: Tao Liu <ltao(a)redhat.com&gt; > Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; > --- > arm64.c | 4 ++++ > crash_target.c | 5 +++++ > gdb-10.2.patch | 2 +- > kernel.c | 20 ++++++++++++++++++++ > ppc64.c | 4 ++++ > x86_64.c | 3 +++ > 6 files changed, 37 insertions(+), 1 deletion(-) > > diff --git a/arm64.c b/arm64.c > index 1cdde5f..8291301 100644 > --- a/arm64.c > +++ b/arm64.c > @@ -126,6 +126,10 @@ struct user_regs_bitmap_struct { > ulong bitmap[32]; > }; > I have no more comments about the patchset, only two things: [1] This was defined in various architectures such as X86 64, aarch64 and ppc64, can it be defined in a common file and included with define macros such as "#if defined(X86_64) || defined(ARM64) || defined(PPC64)"?

> +#define MAX_EXCEPTION_STACKS 7 > +ulong extra_stacks_idx = 0; > +struct user_regs_bitmap_struct *extra_stacks_regs[MAX_EXCEPTION_STACKS] = {0}; > + And also add code comments to say why it(max exception stacks) is 7.

[2] build warning cc -c -g -DX86_64 -DLZO -DGDB_10_2 kernel.c -I./gdb-10.2/bfd -I./gdb-10.2/include -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wformat-security kernel.c:12009:6: warning: no previous prototype for ‘silent_call_bt’ [-Wmissing-prototypes] 12009 | void silent_call_bt(void) | ^~~~~~~~~~~~~~

> static inline bool is_mte_kvaddr(ulong addr) > { > /* check for ARM64_MTE enabled */ > diff --git a/crash_target.c b/crash_target.c > index c5ad1df..8cfa744 100644 > --- a/crash_target.c > +++ b/crash_target.c > @@ -31,6 +31,9 @@ extern "C" int crash_get_current_task_reg (int regno, const char *regname, > extern "C" int gdb_change_thread_context (void); > extern "C" int gdb_add_substack (int); > extern "C" void crash_get_current_task_info(unsigned long *pid, char **comm); > +#if defined (TARGET_X86_64) || defined (TARGET_ARM64) || defined (TARGET_PPC64) > +extern "C" void silent_call_bt(void); > +#endif > > /* The crash target. */ > > @@ -164,6 +165,10 @@ gdb_change_thread_context (void) > /* 3rd, refresh regcache for tid 0 */ > target_fetch_registers(get_current_regcache(), -1); > reinit_frame_cache(); > +#if defined (TARGET_X86_64) || defined (TARGET_ARM64) || defined (TARGET_PPC64) > + /* 4th, invoke bt silently to refresh the additional stacks */ > + silent_call_bt(); This looks tricky, but I have no idea for the time being, maybe we can improve it in the future.

Other changes are fine to me. Thanks Lianbo > +#endif > return TRUE; > } > > diff --git a/gdb-10.2.patch b/gdb-10.2.patch > index c867660..7baa925 100644 > --- a/gdb-10.2.patch > +++ b/gdb-10.2.patch > @@ -55,7 +55,7 @@ exit 0 > # your system doesn't have fcntl.h in /usr/include (which is where it > # should be according to Posix). > -DEFS = @DEFS@ > -+DEFS = -DCRASH_MERGE @DEFS@ > ++DEFS = -DCRASH_MERGE -DTARGET_${CRASH_TARGET} @DEFS@ > GDB_CFLAGS = -I. -I$(srcdir) -I$(srcdir)/config \ > -DLOCALEDIR="\"$(localedir)\"" $(DEFS) > > diff --git a/kernel.c b/kernel.c > index 8c2e0ca..1c8f500 100644 > --- a/kernel.c > +++ b/kernel.c > @@ -12001,3 +12001,23 @@ int get_linux_banner_from_vmlinux(char *buf, size_t size) > > return TRUE; > } > + > +#if defined(X86_64) || defined(ARM64) || defined(PPC64) > +extern ulong extra_stacks_idx; > +extern void *extra_stacks_regs[]; > +void silent_call_bt(void) > +{ > + FILE *fp_save = fp; > + fp = pc->nullfp; > + > + for (int i = 0; i < extra_stacks_idx; i++) { > + FREEBUF(extra_stacks_regs[i]); > + extra_stacks_regs[i] = NULL; > + } > + sprintf(pc->command_line, "bt\n"); > + argcnt = parse_line(pc->command_line, args); > + optind = 1; > + cmd_bt(); > + fp = fp_save; > +} > +#endif > \ No newline at end of file > diff --git a/ppc64.c b/ppc64.c > index 7ac12fe..532eb3f 100644 > --- a/ppc64.c > +++ b/ppc64.c > @@ -80,6 +80,10 @@ struct user_regs_bitmap_struct { > ulong bitmap[32]; > }; > > +#define MAX_EXCEPTION_STACKS 7 > +ulong extra_stacks_idx = 0; > +struct user_regs_bitmap_struct *extra_stacks_regs[MAX_EXCEPTION_STACKS] = {0}; > + > static int is_opal_context(ulong sp, ulong nip) > { > uint64_t opal_start, opal_end; > diff --git a/x86_64.c b/x86_64.c > index 53ae3ef..e544be8 100644 > --- a/x86_64.c > +++ b/x86_64.c > @@ -160,6 +160,9 @@ struct user_regs_bitmap_struct { > ulong bitmap[32]; > }; > > +ulong extra_stacks_idx = 0; > +struct user_regs_bitmap_struct *extra_stacks_regs[MAX_EXCEPTION_STACKS] = {0}; > + > /* > * Do all necessary machine-specific setup here. This is called several > * times during initialization. > -- > 2.47.0 -- Crash-utility mailing list -- devel(a)lists.crash-utility.osci.io To unsubscribe send an email to devel-leave(a)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