| Message ID | 20131016181117.GP13608@pd.tnic (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Wed, Oct 16, 2013 at 08:11:17PM +0200, Borislav Petkov wrote: > Date: Wed, 16 Oct 2013 20:11:17 +0200 > From: Borislav Petkov <bp@alien8.de> > To: Steven Rostedt <rostedt@goodmis.org>, "Chen, Gong" > <gong.chen@linux.intel.com> > Cc: tony.luck@intel.com, joe@perches.com, naveen.n.rao@linux.vnet.ibm.com, > arozansk@redhat.com, linux-acpi@vger.kernel.org, > linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 0/9] Extended H/W error log driver > User-Agent: Mutt/1.5.21 (2010-09-15) > > On Wed, Oct 16, 2013 at 08:00:38PM +0200, Borislav Petkov wrote: > > Right, the only difference I can see is that include/ras/ras_event.h > > doesn't have those below: > > > > #undef TRACE_INCLUDE_PATH > > #undef TRACE_INCLUDE_FILE > > #define TRACE_INCLUDE_PATH . > > > > Perhaps that is the problem? > > > > Gong, what is exactly the issue you're observing? > > Ok, I think I know what the issue is: > > Gong has > > diff --git a/drivers/acpi/extlog_trace.c b/drivers/acpi/extlog_trace.c > new file mode 100644 > index 000000000000..28640807fb09 > --- /dev/null > +++ b/drivers/acpi/extlog_trace.c > @@ -0,0 +1,107 @@ > +#include <linux/export.h> > +#include <linux/dmi.h> > +#include "extlog_trace.h" > + > +#define CREATE_TRACE_POINTS > +#define TRACE_INCLUDE_PATH ../../include/ras > +#include <ras/ras_event.h> > > for the ras tracepoint although this is done already in > drivers/edac/edac_mc.c > Sorry I don't express clearly enough. The patch [v2 9/9] in this patch seris can work well. The bogus one is in myself reply for patch [v2 0/9]. In this patch series I keep trace interface always builtin, so it can work for module & builtin, whether CREATE_TRACE_POINTS is defined multi-times or not. The weird thing for bogus patch is if it is compiled as a module, I can find the trace_xxx function is called definitely and paramerters are expected but nothing output via trace interface, just like below: # tracer: nop # # entries-in-buffer/entries-written: 0/0 #P:120 # # _-----=> irqs-off # / _----=> need-resched # | / _---=> hardirq/softirq # || / _--=> preempt-depth # ||| / delay # TASK-PID CPU# |||| TIMESTAMP FUNCTION # | | | |||| | | I highly suspect my trace_xxx function is compiled as an empty function if following my bogus patch. > Gong, can you try moving the CREATE_TRACE_POINTS line to a new file - > arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move > it away from edac_mc.c. Does that help? In current kernel we haven't arch/x86/ras/ras.c. You mean I create a new one there and just add some trace macro definition? > > Also, see Documentation/trace/tracepoints.txt for more info. > > HTH. > > -- > Regards/Gruss, > Boris. > > Sent from a fat crate under my desk. Formatting is fine. > --
On Thu, 17 Oct 2013 10:33:48 -0400 Chen Gong <gong.chen@linux.intel.com> wrote: > > Gong, can you try moving the CREATE_TRACE_POINTS line to a new file - > > arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move > > it away from edac_mc.c. Does that help? > > In current kernel we haven't arch/x86/ras/ras.c. You mean I create > a new one there and just add some trace macro definition? The CREATE_TRACE_POINTS will cause the TRACE_EVENT() macro to define the functions used for tracing. If you have it defined more than once, it will either cause a kernel compile error (variables declared more than once), or if used in modules, will cause confusion (as well as bloat) because different functions and variables will be defined for the same tracepoint. If you need to create a separate file that you can have it defined in a single place, then please do so! -- Steve -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 17, 2013 at 11:25:41AM -0400, Steven Rostedt wrote: > On Thu, 17 Oct 2013 10:33:48 -0400 > Chen Gong <gong.chen@linux.intel.com> wrote: > > > > > Gong, can you try moving the CREATE_TRACE_POINTS line to a new file - > > > arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move > > > it away from edac_mc.c. Does that help? > > > > In current kernel we haven't arch/x86/ras/ras.c. You mean I create > > a new one there and just add some trace macro definition? > > The CREATE_TRACE_POINTS will cause the TRACE_EVENT() macro to define > the functions used for tracing. If you have it defined more than once, > it will either cause a kernel compile error (variables declared more > than once), or if used in modules, will cause confusion (as well as > bloat) because different functions and variables will be defined for > the same tracepoint. > > If you need to create a separate file that you can have it defined in a > single place, then please do so! Yes, the plan was this all along to have arch/x86/ras/ras.c or arch/x86/ras/core.c where CREATE_TRACE_POINTS is done, among other things. Btw, Gong, there's an easier way to test this, if you wanna: simply make sure EDAC is disabled in the kernel with your patches - i.e., CONFIG_EDAC is not set in your config. It should work then. But, the final solution should be what Steve says with the file I'm suggesting :) Thanks.
diff --git a/drivers/acpi/extlog_trace.c b/drivers/acpi/extlog_trace.c new file mode 100644 index 000000000000..28640807fb09 --- /dev/null +++ b/drivers/acpi/extlog_trace.c @@ -0,0 +1,107 @@ +#include <linux/export.h> +#include <linux/dmi.h> +#include "extlog_trace.h" + +#define CREATE_TRACE_POINTS +#define TRACE_INCLUDE_PATH ../../include/ras +#include <ras/ras_event.h> for the ras tracepoint although this is done already in drivers/edac/edac_mc.c Gong, can you try moving the CREATE_TRACE_POINTS line to a new file - arch/x86/ras/ras.c and define it there and not anywhere else, i.e. move it away from edac_mc.c. Does that help? Also, see Documentation/trace/tracepoints.txt for more info. HTH. -- Regards/Gruss, Boris. Sent from a fat crate under my desk. Formatting is fine. -- -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html