| Message ID | 1400142646-10127-7-git-send-email-gong.chen@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, May 15, 2014 at 04:30:45AM -0400, Chen, Gong wrote: > To avoid saving two copies for one H/W event, add a new > file under debugfs to control how to save event log. > Once this file is opened, the perf/trace will be used, > in the meanwhile, kernel will stop to print event log > to the console. On the other hand, if this file is closed, > kernel will print event log to the console again. > > v3 -> v2: minor adjustment to make flow cleanly. > v2 -> v1: move counter operation from *read* to *open*. > > Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> > --- > drivers/acpi/acpi_extlog.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c > index b1dcb5b..c1dab37 100644 > --- a/drivers/acpi/acpi_extlog.c > +++ b/drivers/acpi/acpi_extlog.c > @@ -12,6 +12,7 @@ > #include <linux/cper.h> > #include <linux/ratelimit.h> > #include <linux/edac.h> > +#include <linux/ras.h> > #include <asm/cpu.h> > #include <asm/mce.h> > > @@ -185,7 +186,11 @@ static int extlog_print(struct notifier_block *nb, unsigned long val, > estatus->block_status = 0; > > tmp = (struct acpi_generic_status *)elog_buf; > - print_extlog_rcd(NULL, tmp, cpu); > + > + if (ras_userspace_consumers() == 0) { if (!ras_userspace_consumers()) > + print_extlog_rcd(NULL, tmp, cpu); > + goto out; > + } > > /* log event via trace */ > err_count++; > @@ -202,6 +207,7 @@ static int extlog_print(struct notifier_block *nb, unsigned long val, > gdata->error_severity, mem_err); > } > > +out: > return NOTIFY_STOP; > } > > -- > 2.0.0.rc0 > >
On Wed, May 21, 2014 at 01:06:31PM +0200, Borislav Petkov wrote: > > + if (ras_userspace_consumers() == 0) { > > if (!ras_userspace_consumers()) > No, it is not a pointer so I don't think it is very meaningful just to save some bytes.
On Wed, May 21, 2014 at 07:46:26PM -0400, Chen, Gong wrote: > On Wed, May 21, 2014 at 01:06:31PM +0200, Borislav Petkov wrote: > > > + if (ras_userspace_consumers() == 0) { > > > > if (!ras_userspace_consumers()) > > > No, it is not a pointer so I don't think it is very > meaningful just to save some bytes. Btw, this is exactly why your patches take too long to review - you like to debate more instead of listening to the maintainers. Next time you want to speed up the process, just think about that. I think the amount of time I wasted to explain all the crap to you is more than I've spent actually reviewing your patches. How about you do what you're told for a change, not change agreed upon stuff after review because then I have to go and review it all over again from the beginning and thus make both our lives easier? As to the question why you should listen to the maintainers: that's because we get to maintain your code after you go and do something else so it better be readable to us. Now to answer your direct question: if (!ras_userspace_consumers()) reads straight away as "if there are no ras userspace consumers" instead of "if the number of the ras userspace consumers is zero". Got it?!
On Thu, May 22, 2014 at 01:11:40PM +0200, Borislav Petkov wrote: > Date: Thu, 22 May 2014 13:11:40 +0200 > From: Borislav Petkov <bp@alien8.de> > To: "Chen, Gong" <gong.chen@linux.intel.com> > Cc: tony.luck@intel.com, m.chehab@samsung.com, linux-acpi@vger.kernel.org > Subject: Re: [PATCH 6/7 v3] trace, eMCA: Add a knob to adjust where to save > event log > User-Agent: Mutt/1.5.23 (2014-03-12) > > On Wed, May 21, 2014 at 07:46:26PM -0400, Chen, Gong wrote: > > On Wed, May 21, 2014 at 01:06:31PM +0200, Borislav Petkov wrote: > > > > + if (ras_userspace_consumers() == 0) { > > > > > > if (!ras_userspace_consumers()) > > > > > No, it is not a pointer so I don't think it is very > > meaningful just to save some bytes. > > Btw, this is exactly why your patches take too long to review - you like > to debate more instead of listening to the maintainers. Next time you > want to speed up the process, just think about that. > Sorry, I'm afraid I can't agree with you. Why should I just follow you blindly without any my own voice? Even if some times I act like an idiot. If I can't say anything for so small thing, do you expect I should shout aloud for something like kernel infrastructure? OK, I will change soon, as you wish.
diff --git a/drivers/acpi/acpi_extlog.c b/drivers/acpi/acpi_extlog.c index b1dcb5b..c1dab37 100644 --- a/drivers/acpi/acpi_extlog.c +++ b/drivers/acpi/acpi_extlog.c @@ -12,6 +12,7 @@ #include <linux/cper.h> #include <linux/ratelimit.h> #include <linux/edac.h> +#include <linux/ras.h> #include <asm/cpu.h> #include <asm/mce.h> @@ -185,7 +186,11 @@ static int extlog_print(struct notifier_block *nb, unsigned long val, estatus->block_status = 0; tmp = (struct acpi_generic_status *)elog_buf; - print_extlog_rcd(NULL, tmp, cpu); + + if (ras_userspace_consumers() == 0) { + print_extlog_rcd(NULL, tmp, cpu); + goto out; + } /* log event via trace */ err_count++; @@ -202,6 +207,7 @@ static int extlog_print(struct notifier_block *nb, unsigned long val, gdata->error_severity, mem_err); } +out: return NOTIFY_STOP; }
To avoid saving two copies for one H/W event, add a new file under debugfs to control how to save event log. Once this file is opened, the perf/trace will be used, in the meanwhile, kernel will stop to print event log to the console. On the other hand, if this file is closed, kernel will print event log to the console again. v3 -> v2: minor adjustment to make flow cleanly. v2 -> v1: move counter operation from *read* to *open*. Signed-off-by: Chen, Gong <gong.chen@linux.intel.com> --- drivers/acpi/acpi_extlog.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)