[ltt-dev] [PATCH v2] add tracepoints of panic and kexec

[compudj@krystal.dyndns.org (Mathieu Desnoyers)]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> >> * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> >> >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> >> >> Instrumentation of following panic and kexec related events are added:
> >> >> panic
> >> >> kernel_kexec
> >> >> crash_kexec
> >> >> 
> >> >> It is useful for build flight-recorder program based on lttng infrastructure.
> >> >> 
> >> > 
> >> > Hrm, I'm wondering if these are events we are interested to trace or if
> >> > you plan to use these tracepoints as hooks into the kernel to connect
> >> > the tracer infrastructure to it ?
> >> Hello, Mathieu
> >> 
> >> IMHO, both.
> >> 
> >> As a programmer who want to build  flight-recorder based on lttng infrastructure,
> >> he can:
> >> 1: use lttng trace, and get lttng's trace datas from kernel-dump file.
> >> or
> >> 2: use lttng's tracepoint, and record events himself.
> >> 
> >> But as a lttng's programmer, we can make both way possible.
> >> 
> >> What is your opinion?
> >> 
> > 
> > Yes, I think having both is good. So we could add, separately :
> > 
> > A hook in panic and kexec using include/linux/notifier.h so mechanisms
> > other than tracers can connect their hook to it.
> > 
> > Also add a tracepoint so tracers can use this as an event source.
> > 
> > I would do both in separate patches though, because they have different
> > purposes.
> Hello, Mathieu
> 
> Are you means you will do both in next version of lttng?
> Thanks!
> 

Hi Zhaolei,

I won't create the patches myself because I don't see it as a priority
(Linus said he would refuse any instrumentation patch before we get the
data output mechanism into the kernel), but if someone provides patches
to add notifier feature to kexec and panic so we can plug LTTng into
them to extract traces from a crashed kernel, I would be very interested
to add this. OTOH, about the tracepoints, I am not convinced they are
necessary for now, because we already instrument printk which logs some
information on panic(). Maybe kexec would be better suited for
instrumentation.

Also, it would be good to provide the LTTng probes for the tracepoints
you add in a separate patch within the same patchset
(ltt/probes/*-trace.c).

Thanks,

Mathieu

> B.R
> Zhaolei
> 
> > 
> > Best regards,
> > 
> > Mathieu
> > 
> > 
> >> B.R.
> >> Zhaolei
> >> > 
> >> > If you plan to use those as hooks into the kernel, we should probably
> >> > consider adding notifiers instead (notifier.h). This is the "blessed"
> >> > way to be called upon such events.
> >> > 
> >> > Mathieu
> >> > 
> >> >> Signed-off-by: Zhao Lei <zhaolei at cn.fujitsu.com>
> >> >> ---
> >> >>  include/trace/kernel.h |   10 ++++++++++
> >> >>  kernel/kexec.c         |    8 ++++++++
> >> >>  kernel/panic.c         |    7 +++++++
> >> >>  3 files changed, 25 insertions(+), 0 deletions(-)
> >> >> 
> >> >> diff --git a/include/trace/kernel.h b/include/trace/kernel.h
> >> >> index d2c3320..06d585f 100644
> >> >> --- a/include/trace/kernel.h
> >> >> +++ b/include/trace/kernel.h
> >> >> @@ -2,6 +2,7 @@
> >> >>  #define _TRACE_KERNEL_H
> >> >>  
> >> >>  #include <linux/tracepoint.h>
> >> >> +#include <linux/kexec.h>
> >> >>  
> >> >>  DECLARE_TRACE(kernel_printk,
> >> >>  TPPROTO(unsigned long retaddr),
> >> >> @@ -15,5 +16,14 @@ DECLARE_TRACE(kernel_module_free,
> >> >>  DECLARE_TRACE(kernel_module_load,
> >> >>  TPPROTO(struct module *mod),
> >> >>  TPARGS(mod));
> >> >> +DECLARE_TRACE(kernel_panic,
> >> >> + TPPROTO(const char *fmt, va_list args),
> >> >> + TPARGS(fmt, args));
> >> >> +DECLARE_TRACE(kernel_kernel_kexec,
> >> >> + TPPROTO(struct kimage *image),
> >> >> + TPARGS(image));
> >> >> +DECLARE_TRACE(kernel_crash_kexec,
> >> >> + TPPROTO(struct kimage *image, struct pt_regs *regs),
> >> >> + TPARGS(image, regs));
> >> >>  
> >> >>  #endif
> >> >> diff --git a/kernel/kexec.c b/kernel/kexec.c
> >> >> index aef2653..56cdaac 100644
> >> >> --- a/kernel/kexec.c
> >> >> +++ b/kernel/kexec.c
> >> >> @@ -30,6 +30,7 @@
> >> >>  #include <linux/pm.h>
> >> >>  #include <linux/cpu.h>
> >> >>  #include <linux/console.h>
> >> >> +#include <trace/kernel.h>
> >> >>  
> >> >>  #include <asm/page.h>
> >> >>  #include <asm/uaccess.h>
> >> >> @@ -37,6 +38,9 @@
> >> >>  #include <asm/system.h>
> >> >>  #include <asm/sections.h>
> >> >>  
> >> >> +DEFINE_TRACE(kernel_kernel_kexec);
> >> >> +DEFINE_TRACE(kernel_crash_kexec);
> >> >> +
> >> >>  /* Per cpu memory for storing cpu states in case of system crash. */
> >> >>  note_buf_t* crash_notes;
> >> >>  
> >> >> @@ -1062,6 +1066,8 @@ asmlinkage long compat_sys_kexec_load(unsigned long entry,
> >> >>  
> >> >>  void crash_kexec(struct pt_regs *regs)
> >> >>  {
> >> >> + trace_kernel_crash_kexec(kexec_crash_image, regs);
> >> >> +
> >> >>  /* Take the kexec_mutex here to prevent sys_kexec_load
> >> >>  * running on one cpu from replacing the crash kernel
> >> >>  * we are using after a panic on a different cpu.
> >> >> @@ -1428,6 +1434,8 @@ int kernel_kexec(void)
> >> >>  {
> >> >>  int error = 0;
> >> >>  
> >> >> + trace_kernel_kernel_kexec(kexec_image);
> >> >> +
> >> >>  if (!mutex_trylock(&kexec_mutex))
> >> >>  return -EBUSY;
> >> >>  if (!kexec_image) {
> >> >> diff --git a/kernel/panic.c b/kernel/panic.c
> >> >> index 12c5a0a..3fa642e 100644
> >> >> --- a/kernel/panic.c
> >> >> +++ b/kernel/panic.c
> >> >> @@ -21,6 +21,9 @@
> >> >>  #include <linux/debug_locks.h>
> >> >>  #include <linux/random.h>
> >> >>  #include <linux/kallsyms.h>
> >> >> +#include <trace/kernel.h>
> >> >> +
> >> >> +DEFINE_TRACE(kernel_panic);
> >> >>  
> >> >>  int panic_on_oops;
> >> >>  int tainted;
> >> >> @@ -68,6 +71,10 @@ NORET_TYPE void panic(const char * fmt, ...)
> >> >>  unsigned long caller = (unsigned long) __builtin_return_address(0);
> >> >>  #endif
> >> >>  
> >> >> + va_start(args, fmt);
> >> >> + trace_kernel_panic(fmt, args);
> >> >> + va_end(args);
> >> >> +
> >> >>  /*
> >> >>  * It's possible to come here directly from a panic-assertion and not
> >> >>  * have preempt disabled. Some functions called from here want
> >> >> -- 
> >> >> 1.5.5.3
> >> >> 
> >> >> 
> >> >> 
> >> >> _______________________________________________
> >> >> ltt-dev mailing list
> >> >> ltt-dev at lists.casi.polymtl.ca
> >> >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> >> >> 
> >> > 
> >> > -- 
> >> > Mathieu Desnoyers
> >> > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> >> > 
> >> >
> >> _______________________________________________
> >> ltt-dev mailing list
> >> ltt-dev at lists.casi.polymtl.ca
> >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> >> 
> > 
> > -- 
> > Mathieu Desnoyers
> > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> >
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68