| Message ID | 1393924997-8992-2-git-send-email-gong.chen@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Tue, Mar 04, 2014 at 04:23:16AM -0500, Chen, Gong wrote: > To avoid the confuision of usage for RAS related trace event, add > an unified RAS trace event stub. > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > --- > drivers/edac/edac_mc.c | 3 --- > kernel/trace/Makefile | 1 + > kernel/trace/ras-traces.c | 12 ++++++++++++ > 3 files changed, 13 insertions(+), 3 deletions(-) > create mode 100644 kernel/trace/ras-traces.c > > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > index 33edd67..28c1695 100644 > --- a/drivers/edac/edac_mc.c > +++ b/drivers/edac/edac_mc.c > @@ -33,9 +33,6 @@ > #include <asm/edac.h> > #include "edac_core.h" > #include "edac_module.h" > - > -#define CREATE_TRACE_POINTS > -#define TRACE_INCLUDE_PATH ../../include/ras > #include <ras/ras_event.h> > > /* lock to memory controller's control array */ > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile > index 1378e84..167193a 100644 > --- a/kernel/trace/Makefile > +++ b/kernel/trace/Makefile > @@ -52,6 +52,7 @@ endif > obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o > obj-$(CONFIG_EVENT_TRACING) += trace_events_trigger.o > obj-$(CONFIG_KPROBE_EVENT) += trace_kprobe.o > +obj-$(CONFIG_TRACEPOINTS) += ras-traces.o Actually I was thinking of slowly concentrating all the RAS stuff into arch/x86/ras/. Just add arch/x86/ras/trace.c instead please.
Em Thu, 06 Mar 2014 12:18:52 +0100 Borislav Petkov <bp@alien8.de> escreveu: > On Tue, Mar 04, 2014 at 04:23:16AM -0500, Chen, Gong wrote: > > To avoid the confuision of usage for RAS related trace event, add > > an unified RAS trace event stub. > > > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > > --- > > drivers/edac/edac_mc.c | 3 --- > > kernel/trace/Makefile | 1 + > > kernel/trace/ras-traces.c | 12 ++++++++++++ > > 3 files changed, 13 insertions(+), 3 deletions(-) > > create mode 100644 kernel/trace/ras-traces.c > > > > diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c > > index 33edd67..28c1695 100644 > > --- a/drivers/edac/edac_mc.c > > +++ b/drivers/edac/edac_mc.c > > @@ -33,9 +33,6 @@ > > #include <asm/edac.h> > > #include "edac_core.h" > > #include "edac_module.h" > > - > > -#define CREATE_TRACE_POINTS > > -#define TRACE_INCLUDE_PATH ../../include/ras > > #include <ras/ras_event.h> > > > > /* lock to memory controller's control array */ > > diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile > > index 1378e84..167193a 100644 > > --- a/kernel/trace/Makefile > > +++ b/kernel/trace/Makefile > > @@ -52,6 +52,7 @@ endif > > obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o > > obj-$(CONFIG_EVENT_TRACING) += trace_events_trigger.o > > obj-$(CONFIG_KPROBE_EVENT) += trace_kprobe.o > > +obj-$(CONFIG_TRACEPOINTS) += ras-traces.o > > Actually I was thinking of slowly concentrating all the RAS stuff into > arch/x86/ras/. > > Just add arch/x86/ras/trace.c instead please. I would prefer to keep this out of arch/, because there are some parts of the ras infra that aren't x86 specific. So, it would be better to put those at /drivers/ras, and add a "depends on X86" for the x86 specifics. Going further, it also make sense to move the EDAC drivers into it. Regards, Mauro -- 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, Mar 06, 2014 at 08:43:20AM -0300, Mauro Carvalho Chehab wrote: > I would prefer to keep this out of arch/, because there are some parts > of the ras infra that aren't x86 specific. Those are? > Going further, it also make sense to move the EDAC drivers into it. Why?
Em Thu, 06 Mar 2014 13:17:14 +0100 Borislav Petkov <bp@alien8.de> escreveu: > On Thu, Mar 06, 2014 at 08:43:20AM -0300, Mauro Carvalho Chehab wrote: > > I would prefer to keep this out of arch/, because there are some parts > > of the ras infra that aren't x86 specific. > > Those are? For example PCIe and memory errors are not x86-specific. Also, as ACPI may also be used on ARM, we may also start to have APEI errors there: https://lwn.net/Articles/574439/ https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI So, better to think on that on a long term. > > Going further, it also make sense to move the EDAC drivers into it. > > Why? In order to put all RAS drivers under the same place. We may eventually have a subdir there for EDAC, and one per RAS report mechanism, in order to keep it cleaner. Regards, Mauro -- 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, Mar 06, 2014 at 10:06:53AM -0300, Mauro Carvalho Chehab wrote: > For example PCIe and memory errors are not x86-specific. Also, as ACPI > may also be used on ARM, we may also start to have APEI errors there: > https://lwn.net/Articles/574439/ > https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI > > So, better to think on that on a long term. kernel/ras/ could also be used in that case but I guess drivers/ras/ is fine too. > In order to put all RAS drivers under the same place. We may > eventually have a subdir there for EDAC, and one per RAS report > mechanism, in order to keep it cleaner. That doesn't bring any advantages - edac drivers are just fine in drivers/edac/. And without benefits for a move, it would be a senseless code churn only.
Em Thu, 06 Mar 2014 16:26:33 +0100 Borislav Petkov <bp@alien8.de> escreveu: > On Thu, Mar 06, 2014 at 10:06:53AM -0300, Mauro Carvalho Chehab wrote: > > For example PCIe and memory errors are not x86-specific. Also, as ACPI > > may also be used on ARM, we may also start to have APEI errors there: > > https://lwn.net/Articles/574439/ > > https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI > > > > So, better to think on that on a long term. > > kernel/ras/ could also be used in that case but I guess drivers/ras/ is > fine too. Both work for me, although drivers/ras seems more adequate, IMHO, as I expect that we'll have there both subsystem code and drivers. > > > In order to put all RAS drivers under the same place. We may > > eventually have a subdir there for EDAC, and one per RAS report > > mechanism, in order to keep it cleaner. > > That doesn't bring any advantages - edac drivers are just fine in > drivers/edac/. And without benefits for a move, it would be a senseless > code churn only. No, it won't bring any technical advantage. Err... if an EDAC driver or core would depend on something at /drivers/ras, then we may need to add some extra early init glue, in order to be sure that the code at /drivers/ras will be initialized before /drivers/edac, or otherwise it would fail with both are compiled builtin.
On Thu, Mar 06, 2014 at 12:39:15PM -0300, Mauro Carvalho Chehab wrote: > Date: Thu, 06 Mar 2014 12:39:15 -0300 > From: Mauro Carvalho Chehab <m.chehab@samsung.com> > To: Borislav Petkov <bp@alien8.de> > Cc: "Chen, Gong" <gong.chen@linux.intel.com>, tony.luck@intel.com, > arozansk@redhat.com, linux-acpi@vger.kernel.org > Subject: Re: [PATCH 1/2] trace, RAS: Add basic RAS trace event > X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) > > Em Thu, 06 Mar 2014 16:26:33 +0100 > Borislav Petkov <bp@alien8.de> escreveu: > > > On Thu, Mar 06, 2014 at 10:06:53AM -0300, Mauro Carvalho Chehab wrote: > > > For example PCIe and memory errors are not x86-specific. Also, as ACPI > > > may also be used on ARM, we may also start to have APEI errors there: > > > https://lwn.net/Articles/574439/ > > > https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI > > > > > > So, better to think on that on a long term. > > > > kernel/ras/ could also be used in that case but I guess drivers/ras/ is > > fine too. > > Both work for me, although drivers/ras seems more adequate, IMHO, > as I expect that we'll have there both subsystem code and drivers. > OK, I will move this trace stub to drivers/ras in next version. BTW, any other comments for 2nd patch? I thought Mauro will have some comments for that patch. :-)
Em Fri, 07 Mar 2014 01:21:54 -0500 "Chen, Gong" <gong.chen@linux.intel.com> escreveu: > On Thu, Mar 06, 2014 at 12:39:15PM -0300, Mauro Carvalho Chehab wrote: > > Date: Thu, 06 Mar 2014 12:39:15 -0300 > > From: Mauro Carvalho Chehab <m.chehab@samsung.com> > > To: Borislav Petkov <bp@alien8.de> > > Cc: "Chen, Gong" <gong.chen@linux.intel.com>, tony.luck@intel.com, > > arozansk@redhat.com, linux-acpi@vger.kernel.org > > Subject: Re: [PATCH 1/2] trace, RAS: Add basic RAS trace event > > X-Mailer: Claws Mail 3.9.3 (GTK+ 2.24.22; x86_64-redhat-linux-gnu) > > > > Em Thu, 06 Mar 2014 16:26:33 +0100 > > Borislav Petkov <bp@alien8.de> escreveu: > > > > > On Thu, Mar 06, 2014 at 10:06:53AM -0300, Mauro Carvalho Chehab wrote: > > > > For example PCIe and memory errors are not x86-specific. Also, as ACPI > > > > may also be used on ARM, we may also start to have APEI errors there: > > > > https://lwn.net/Articles/574439/ > > > > https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI > > > > > > > > So, better to think on that on a long term. > > > > > > kernel/ras/ could also be used in that case but I guess drivers/ras/ is > > > fine too. > > > > Both work for me, although drivers/ras seems more adequate, IMHO, > > as I expect that we'll have there both subsystem code and drivers. > > > OK, I will move this trace stub to drivers/ras in next version. Thanks! > BTW, any other comments for 2nd patch? I thought Mauro will > have some comments for that patch. :-) Well, the comment is the same: I still think that the better would be to map this via EDAC, in order to allow userspace to associate the DIMM labels with the DIMMs that would be reported, as the association of card, module, rank number, valid device into the corresponding DIMM slot, as marked at the board's silkscreen is not trivial. If you're not willing to do that, the better would then to add such association logic inside the rasdaemon. Patches are welcomed. Regards, Mauro
diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c index 33edd67..28c1695 100644 --- a/drivers/edac/edac_mc.c +++ b/drivers/edac/edac_mc.c @@ -33,9 +33,6 @@ #include <asm/edac.h> #include "edac_core.h" #include "edac_module.h" - -#define CREATE_TRACE_POINTS -#define TRACE_INCLUDE_PATH ../../include/ras #include <ras/ras_event.h> /* lock to memory controller's control array */ diff --git a/kernel/trace/Makefile b/kernel/trace/Makefile index 1378e84..167193a 100644 --- a/kernel/trace/Makefile +++ b/kernel/trace/Makefile @@ -52,6 +52,7 @@ endif obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o obj-$(CONFIG_EVENT_TRACING) += trace_events_trigger.o obj-$(CONFIG_KPROBE_EVENT) += trace_kprobe.o +obj-$(CONFIG_TRACEPOINTS) += ras-traces.o obj-$(CONFIG_TRACEPOINTS) += power-traces.o ifeq ($(CONFIG_PM_RUNTIME),y) obj-$(CONFIG_TRACEPOINTS) += rpm-traces.o diff --git a/kernel/trace/ras-traces.c b/kernel/trace/ras-traces.c new file mode 100644 index 0000000..b0c6ed1 --- /dev/null +++ b/kernel/trace/ras-traces.c @@ -0,0 +1,12 @@ +/* + * Copyright (C) 2014 Intel Corporation + * + * Authors: + * Chen, Gong <gong.chen@linux.intel.com> + */ + +#define CREATE_TRACE_POINTS +#define TRACE_INCLUDE_PATH ../../include/ras +#include <ras/ras_event.h> + +EXPORT_TRACEPOINT_SYMBOL_GPL(mc_event);
To avoid the confuision of usage for RAS related trace event, add an unified RAS trace event stub. Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> --- drivers/edac/edac_mc.c | 3 --- kernel/trace/Makefile | 1 + kernel/trace/ras-traces.c | 12 ++++++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) create mode 100644 kernel/trace/ras-traces.c