* Re: [PATCH] arm64: traps: add tracepoints for unusal exception cases [not found] ` <20210305123635.27492-1-sangmoon.kim@samsung.com> @ 2021-03-08 13:31 ` Mark Brown [not found] ` <CGME20210316134622epcas1p488fe019ee343dd156dc077c6df9322da@epcas1p4.samsung.com> 2021-03-16 15:28 ` Steven Rostedt 0 siblings, 2 replies; 3+ messages in thread From: Mark Brown @ 2021-03-08 13:31 UTC (permalink / raw) To: Sangmoon Kim Cc: Catalin Marinas, Will Deacon, jordan.lim, Steven Rostedt, Ingo Molnar, Dave Martin, Mark Rutland, Dmitry Safonov, Amit Daniel Kachhap, Peter Collingbourne, Gavin Shan, linux-arm-kernel, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 2387 bytes --] On Fri, Mar 05, 2021 at 09:36:30PM +0900, Sangmoon Kim wrote: > When kernel panic occurs, a kernel module can use either the > panic_notifier or die_notifier to obtain the debugging information. > However, in case of these exceptions like do_undefinstr(), regs and > esr data are not passed on. Although a module might be able to find > those data in the console messages, parsing text messages is very > expensive behavior for a module especially on mobile devices. > These bare tracepoints allow a module to probe regs and esr information > for debugging purpose. _tp suffix comes from bare tracepoints of > sched/core.c This use case sounds a lot like what the enterprise and Android people do via pstore - it seems like it would be better for this to integrate via the interfaces that other systems are using for similar purposes and then ensure that whatever information is useful is getting passed through in a format that makes sense. That'd be more structured and more readily usable by a wider range of systems than something that's more of a building block, going via the trace infrastructure seems like a bit of an indirection. > @@ -832,6 +846,7 @@ void __noreturn arm64_serror_panic(struct pt_regs *regs, u32 esr) > if (regs) > __show_regs(regs); > > + trace_traps_serror_panic_tp(regs, esr); > nmi_panic(regs, "Asynchronous SError Interrupt"); One of the concerns people have with adding tracepoints is that they can end up defining ABI so if we *are* going to add any then we need to think carefully about how they're defined. As things currently stand they'll pass in the full pt_regs struct which includes not only what's defined by the hardware but also additional software defined information we store along with it like the stackframe which would be even more of a problem if it ends up getting used by someone in a way that ends up as ABI. These are defined as bare tracehooks which does mitigate against things ending up getting used in ways that cause problems but people are still going to worry about things ending up getting relied on one way or another. That said it's not clear to me that this will record anything beyond the pointer directly in the trace buffer so the value might not be useful for terribly long, that itself feels like it might not be as robust an interface as it should be. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 176 bytes --] _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <CGME20210316134622epcas1p488fe019ee343dd156dc077c6df9322da@epcas1p4.samsung.com>]
* Re: [PATCH] arm64: traps: add tracepoints for unusal exception cases [not found] ` <CGME20210316134622epcas1p488fe019ee343dd156dc077c6df9322da@epcas1p4.samsung.com> @ 2021-03-16 13:37 ` Sangmoon Kim 0 siblings, 0 replies; 3+ messages in thread From: Sangmoon Kim @ 2021-03-16 13:37 UTC (permalink / raw) To: broonie Cc: 0x7f454c46, Dave.Martin, amit.kachhap, catalin.marinas, gshan, jordan.lim, linux-arm-kernel, linux-kernel, mark.rutland, mingo, pcc, rostedt, sangmoon.kim, will > -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Monday, March 8, 2021 10:32 PM > Subject: Re: [PATCH] arm64: traps: add tracepoints for unusal exception cases > > On Fri, Mar 05, 2021 at 09:36:30PM +0900, Sangmoon Kim wrote: > > > When kernel panic occurs, a kernel module can use either the > > panic_notifier or die_notifier to obtain the debugging information. > > > However, in case of these exceptions like do_undefinstr(), regs and > > esr data are not passed on. Although a module might be able to find > > those data in the console messages, parsing text messages is very > > expensive behavior for a module especially on mobile devices. > > > These bare tracepoints allow a module to probe regs and esr information > > for debugging purpose. _tp suffix comes from bare tracepoints of > > sched/core.c > > This use case sounds a lot like what the enterprise and Android people > do via pstore - it seems like it would be better for this to integrate > via the interfaces that other systems are using for similar purposes and > then ensure that whatever information is useful is getting passed > through in a format that makes sense. That'd be more structured and > more readily usable by a wider range of systems than something that's > more of a building block, going via the trace infrastructure seems like > a bit of an indirection. > > > @@ -832,6 +846,7 @@ void __noreturn arm64_serror_panic(struct pt_regs *regs, u32 esr) > > if (regs) > > __show_regs(regs); > > > > + trace_traps_serror_panic_tp(regs, esr); > > nmi_panic(regs, "Asynchronous SError Interrupt"); > > One of the concerns people have with adding tracepoints is that they can > end up defining ABI so if we *are* going to add any then we need to > think carefully about how they're defined. As things currently stand > they'll pass in the full pt_regs struct which includes not only what's > defined by the hardware but also additional software defined information > we store along with it like the stackframe which would be even more of a > problem if it ends up getting used by someone in a way that ends up as > ABI. These are defined as bare tracehooks which does mitigate against > things ending up getting used in ways that cause problems but people are > still going to worry about things ending up getting relied on one way or > another. > > That said it's not clear to me that this will record anything beyond the > pointer directly in the trace buffer so the value might not be useful > for terribly long, that itself feels like it might not be as robust an > interface as it should be. Dear Mark, Thank you for your review. I learned a lot about the concerns when using tracepoint. Thanks, Sangmoon _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] arm64: traps: add tracepoints for unusal exception cases 2021-03-08 13:31 ` [PATCH] arm64: traps: add tracepoints for unusal exception cases Mark Brown [not found] ` <CGME20210316134622epcas1p488fe019ee343dd156dc077c6df9322da@epcas1p4.samsung.com> @ 2021-03-16 15:28 ` Steven Rostedt 1 sibling, 0 replies; 3+ messages in thread From: Steven Rostedt @ 2021-03-16 15:28 UTC (permalink / raw) To: Mark Brown Cc: Sangmoon Kim, Catalin Marinas, Will Deacon, jordan.lim, Ingo Molnar, Dave Martin, Mark Rutland, Dmitry Safonov, Amit Daniel Kachhap, Peter Collingbourne, Gavin Shan, linux-arm-kernel, linux-kernel On Mon, 8 Mar 2021 13:31:49 +0000 Mark Brown <broonie@kernel.org> wrote: > > @@ -832,6 +846,7 @@ void __noreturn arm64_serror_panic(struct pt_regs *regs, u32 esr) > > if (regs) > > __show_regs(regs); > > > > + trace_traps_serror_panic_tp(regs, esr); > > nmi_panic(regs, "Asynchronous SError Interrupt"); > > One of the concerns people have with adding tracepoints is that they can > end up defining ABI so if we *are* going to add any then we need to > think carefully about how they're defined. As things currently stand > they'll pass in the full pt_regs struct which includes not only what's > defined by the hardware but also additional software defined information > we store along with it like the stackframe which would be even more of a > problem if it ends up getting used by someone in a way that ends up as > ABI. These are defined as bare tracehooks which does mitigate against > things ending up getting used in ways that cause problems but people are > still going to worry about things ending up getting relied on one way or > another. > > That said it's not clear to me that this will record anything beyond the > pointer directly in the trace buffer so the value might not be useful > for terribly long, that itself feels like it might not be as robust an > interface as it should be. BTW, we are working on an "event probe". That is similar to kprobe event, but attaches to the output of an event to create another event. Thus, if you had a trace event that was like this: trace_regs(pt_regs *regs); Where you save the regs pointer for output: TP_STRUCT__entry( __field(void *, regs ) ), TP_fast_assign( __entry->regs = regs; ) Then you would be able to get access to all the regs for that tracepoint! # echo 'e:pt_regs regs ip=+8(regs):x64' > /sys/kernel/tracing/kprobe_events Where "e:" denotes that this connects to a trace event, "pt_regs" is the event name created under kprobes subsystem, "regs" is the trace event to attach to, "ip=" is what will be printed, and "+8(regs):x64" is a way to dereference the regs pointer at the time of the trace event is executed. That is, it will dereference 8 bytes from the pointer and return a x64 hex number. Thus, trace events like this may be very useful in the near future. -- Steve _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-03-16 15:29 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20210305124537epcas1p1930302083680f1b1cf87e37630556460@epcas1p1.samsung.com> [not found] ` <20210305123635.27492-1-sangmoon.kim@samsung.com> 2021-03-08 13:31 ` [PATCH] arm64: traps: add tracepoints for unusal exception cases Mark Brown [not found] ` <CGME20210316134622epcas1p488fe019ee343dd156dc077c6df9322da@epcas1p4.samsung.com> 2021-03-16 13:37 ` Sangmoon Kim 2021-03-16 15:28 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).