[Crash-utility] Re: [PATCH v9 0/5] Improve stack unwind on ppc64

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Aditya,

On Fri, Feb 23, 2024 at 5:53 PM Aditya Gupta <adityag@xxxxxxxxxxxxx> wrote:
>
> Hi Tao Liu,
>
> > > > <...snip...>
> > > >
> > > > Currently I'm still struggling with some failing cases of x86_64
> > > > unwinding. So I didn't arrange my patchsets, along with the patch
> > > > commit log well, since they are all "trial" patches.
> > > >
> > > > I agree the patch "ppc64 arbitrary task stack unwind support" is
> > > > better to go with the ppc patch series. But I suggest we make some
> > > > modifications for it:
> > > >
> > > > 1) I'm OK with it being a stand alone patch, or merging the code
> > > > changes of this one into your previous patches, but I prefer the
> > > > latter one :)
> > > >
> > > > 2) If you'd like to go with a stand alone patch, could you please
> > > > rewrite a commit log and title for this one?
> > >
> > > Sure, I am okay with either. Let's go with the latter one you suggested,
> > > in that case I will add the line adding 'need_free' in 'bt_info'
> > > structure also, to make that work. By the way, shouldn't it be
> > > 'FREEBUF(bt_info.stackbuf)' instead of 'FREEBUF(pt_regs)' ?
> > >
> > > Also, should I add you as a co-author of the patch ?
> >
> > Thanks for the reminder! I just found some bugs and made a little
> > improvement for the patch. So I guess "Suggested-by:" is better than
> > "co-author", what do you think?
>
> Sure, I will pull your latest changes. Regarding the tag,
> "improved-by" okay with you ? Also, I am thinking to add the Cc: list in
> my patch itself, that seems good idea for a discussion this long, and
> easier to manage with git-send-email.

OK, it is OK with me, thanks!

Thanks,
Tao Liu

>
> Btw, I will post the next version, after Lianbo replies to the splitting
> the series suggestion.
>
> Thanks,
> Aditya Gupta
>
> >
> > Thanks,
> > Tao Liu
> >
> > >
> > > Thanks,
> > > Aditya Gupta
> > >
> > > >
> > > > Thanks,
> > > > Tao Liu
> > > >
> > > >
> > > > >
> > > > > To test various gdb passthroughs:
> > > > >
> > > > >         (crash) set
> > > > >         (crash) set gdb on
> > > > >         gdb> thread
> > > > >         gdb> bt
> > > > >         gdb> info threads
> > > > >         gdb> info threads
> > > > >         gdb> info locals
> > > > >         gdb> info variables irq_rover_lock
> > > > >         gdb> info args
> > > > >         gdb> thread 2
> > > > >         gdb> set gdb off
> > > > >         (crash) set
> > > > >         (crash) set -c 6
> > > > >         (crash) gdb thread
> > > > >         (crash) bt
> > > > >         (crash) gdb bt
> > > > >         (crash) frame
> > > > >         (crash) gdb up
> > > > >         (crash) gdb down
> > > > >         (crash) info locals
> > > > >
> > > > > Known Issues:
> > > > > =============
> > > > >
> > > > > 1. In gdb mode, 'bt' might fail to show backtrace in few vmcores collected
> > > > >    from older kernels. This is a known issue due to register mismatch, and
> > > > >    its fix has been merged upstream:
> > > > >
> > > > >    This can also cause some 'invalid kernel virtual address' errors during gdb
> > > > >    unwinding the stack registers
> > > > >
> > > > > Commit: https://github.com/torvalds/linux/commit/b684c09f09e7a6af3794d4233ef785819e72db79
> > > > >
> > > > > Fixing GDB passthroughs on other architectures
> > > > > ==============================================
> > > > >
> > > > > Much of the work for making gdb passthroughs like 'gdb bt', 'gdb
> > > > > thread', 'gdb info locals' etc. has been done by the patches introducing
> > > > > 'machdep->get_cpu_reg' and this series fixing some issues in that.
> > > > >
> > > > > Other architectures should be able to fix these gdb functionalities by
> > > > > simply implementing 'machdep->get_cpu_reg (cpu, regno, ...)'.
> > > > >
> > > > > The reasoning behind that has been explained with a diagram in commit
> > > > > description of patch #1
> > > > >
> > > > > I will assist with my findings/observations fixing it on ppc64 whenever needed.
> > > > >
> > > > > Changelog:
> > > > > ==========
> > > > >
> > > > > V9:
> > > > > + minor change in patch #5: sync gdb context on a 'set' and 'set -p'
> > > > > + add taoliu's patch for using current context, and fixes in ppc64_get_cpu_reg
> > > > >
> > > > > V8:
> > > > > + use get_active_task instead of depending on CURRENT_CONTEXT in ppc64_get_cpu_reg
> > > > > + rebase to upstream/master (5977936c0a91)
> > > > >
> > > > > V7:
> > > > > + move changes in gdb-10.2.patch to the end (minor change in patch #3,4,5)
> > > > > + fix a memory leak in ppc64_get_cpu_reg (minor change in patch #1)
> > > > > + use ascii diagram in patch #1 description
> > > > >
> > > > > V6:
> > > > > + changes in patch #5: fix bug introduced in v5 that caused initial gdb thread
> > > > >   to be thread 1
> > > > >
> > > > > V5:
> > > > > + changes in patch #1: made ppc64_get_cpu_reg static, and remove unreachable
> > > > >   code
> > > > > + changes in patch #3: fixed typo 'ppc64_renum' instead of 'ppc64_regnum',
> > > > >   remove unneeded if condition
> > > > > + changes in patch #5: implement refresh regcache on per thread, instead of all
> > > > >   threads at once
> > > > >
> > > > > V4:
> > > > > + fix segmentation fault in live debugging (change in patch #1)
> > > > > + mention live debugging not supported in cover letter and patch #1
> > > > > + fixed some checkpatch warnings (change in patch #5)
> > > > >
> > > > > V3:
> > > > > + default gdb thread will be the crashing thread, instead of being
> > > > >   thread '0'
> > > > > + synchronise crash cpu and gdb thread context
> > > > > + fix bug in gdb_interface, that replaced gdb's output stream, losing
> > > > >   output in some cases, such as info threads and extra output in info
> > > > >   variables
> > > > > + fix 'info threads'
> > > > >
> > > > > RFC V2:
> > > > >   - removed patch implementing 'frame', 'up', 'down' in crash
> > > > >   - updated the cover letter by removing the mention of those commands other
> > > > >         than the respective gdb passthrough
> > > > >
> > > > > Aditya Gupta (5):
> > > > >   ppc64: correct gdb passthroughs by implementing machdep->get_cpu_reg
> > > > >   remove 'frame' from prohibited commands list
> > > > >   synchronise cpu context changes between crash/gdb
> > > > >   fix gdb_interface: restore gdb's output streams at end of
> > > > >     gdb_interface
> > > > >   fix 'info threads' command
> > > > >
> > > > >  crash_target.c  |  44 ++++++++++++++++
> > > > >  defs.h          | 130 +++++++++++++++++++++++++++++++++++++++++++++++-
> > > > >  gdb-10.2.patch  | 110 +++++++++++++++++++++++++++++++++++++++-
> > > > >  gdb_interface.c |   2 +-
> > > > >  kernel.c        |  47 +++++++++++++++--
> > > > >  ppc64.c         |  95 +++++++++++++++++++++++++++++++++--
> > > > >  task.c          |  14 ++++++
> > > > >  tools.c         |   2 +-
> > > > >  8 files changed, 434 insertions(+), 10 deletions(-)
> > > > >
> > > > > --
> > > > > 2.41.0
> > > > >
> > > >
> > >
> >
>
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux