Messages in this thread | | | Date | Wed, 17 Feb 2010 08:20:40 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH 2/2] nmi_watchdog: properly configure for software events |
| |
* Don Zickus <dzickus@redhat.com> wrote:
> Paul Mackerras brought up a good point that when fallbacking to > software events, I may have been lucky in my configuration. > > Modified the code to explicit provide a new configuration for > software events. > > Signed-off-by: Don Zickus <dzickus@redhat.com> > --- > kernel/nmi_watchdog.c | 23 ++++++++++++++++++----- > 1 files changed, 18 insertions(+), 5 deletions(-)
Applied, thanks Don!
btw., a few minor code cleanliness nits:
> > diff --git a/kernel/nmi_watchdog.c b/kernel/nmi_watchdog.c > index 633b230..3c75cbf 100644 > --- a/kernel/nmi_watchdog.c > +++ b/kernel/nmi_watchdog.c > @@ -61,7 +61,7 @@ static int __init setup_nmi_watchdog(char *str) > } > __setup("nmi_watchdog=", setup_nmi_watchdog); > > -struct perf_event_attr wd_attr = { > +struct perf_event_attr wd_hw_attr = { > .type = PERF_TYPE_HARDWARE, > .config = PERF_COUNT_HW_CPU_CYCLES, > .size = sizeof(struct perf_event_attr), > @@ -69,6 +69,14 @@ struct perf_event_attr wd_attr = { > .disabled = 1, > }; > > +struct perf_event_attr wd_sw_attr = { > + .type = PERF_TYPE_SOFTWARE, > + .config = PERF_COUNT_SW_CPU_CLOCK, > + .size = sizeof(struct perf_event_attr), > + .pinned = 1, > + .disabled = 1, > +};
Please use the vertically aligned initializer style we are using in arch/x86 and in the perf code as well:
struct perf_event_attr wd_sw_attr = { .type = PERF_TYPE_SOFTWARE, .config = PERF_COUNT_SW_CPU_CLOCK, .size = sizeof(struct perf_event_attr), .pinned = 1, .disabled = 1, };
Plus see below the errors and warnings that scripts/checkpatch -f kernel/nmi_watchdog.c found. Most of those (except the col 80 warnings which you should ignore) should be fixed i suspect.
Thanks,
Ingo
ERROR: code indent should use tabs where possible #53: FILE: nmi_watchdog.c:53: + if (!strncmp(str, "panic", 5)) {$
ERROR: code indent should use tabs where possible #54: FILE: nmi_watchdog.c:54: + panic_on_timeout = 1;$
ERROR: code indent should use tabs where possible #55: FILE: nmi_watchdog.c:55: + str = strchr(str, ',');$
ERROR: code indent should use tabs where possible #56: FILE: nmi_watchdog.c:56: + if (!str)$
ERROR: code indent should use tabs where possible #57: FILE: nmi_watchdog.c:57: + return 1;$
ERROR: code indent should use tabs where possible #58: FILE: nmi_watchdog.c:58: + ++str;$
ERROR: code indent should use tabs where possible #59: FILE: nmi_watchdog.c:59: + }$
ERROR: code indent should use tabs where possible #60: FILE: nmi_watchdog.c:60: + return 1;$
CHECK: __setup appears un-documented -- check Documentation/kernel-parameters.txt #62: FILE: nmi_watchdog.c:62: +__setup("nmi_watchdog=", setup_nmi_watchdog);
ERROR: space required after that ',' (ctx:VxV) #98: FILE: nmi_watchdog.c:98: + per_cpu(alert_counter,cpu) += 1; ^
ERROR: space required after that ',' (ctx:VxV) #99: FILE: nmi_watchdog.c:99: + if (per_cpu(alert_counter,cpu) == 5) { ^
WARNING: braces {} are not necessary for any arm of this statement #100: FILE: nmi_watchdog.c:100: + if (panic_on_timeout) { [...] + } else { [...]
WARNING: line over 80 characters #101: FILE: nmi_watchdog.c:101: + panic("NMI Watchdog detected LOCKUP on cpu %d", cpu);
WARNING: line over 80 characters #103: FILE: nmi_watchdog.c:103: + WARN(1, "NMI Watchdog detected LOCKUP on cpu %d", cpu);
ERROR: space required after that ',' (ctx:VxV) #107: FILE: nmi_watchdog.c:107: + per_cpu(alert_counter,cpu) = 0; ^
WARNING: line over 80 characters #126: FILE: nmi_watchdog.c:126: + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, wd_overflow);
WARNING: line over 80 characters #128: FILE: nmi_watchdog.c:128: + /* hardware doesn't exist or not supported, fallback to software events */
WARNING: printk() should include KERN_ facility level #129: FILE: nmi_watchdog.c:129: + printk("nmi_watchdog: hardware not available, trying software events\n");
WARNING: line over 80 characters #132: FILE: nmi_watchdog.c:132: + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, wd_overflow);
WARNING: line over 80 characters #134: FILE: nmi_watchdog.c:134: + printk(KERN_ERR "nmi watchdog failed to create perf event on %i: %p\n", cpu, event);
WARNING: printk() should include KERN_ facility level #185: FILE: nmi_watchdog.c:185: + printk("NMI watchdog failed configuration, "
CHECK: __setup appears un-documented -- check Documentation/kernel-parameters.txt #238: FILE: nmi_watchdog.c:238: +__setup("nonmi_watchdog", nonmi_watchdog_setup);
total: 11 errors, 9 warnings, 2 checks, 258 lines checked
kernel/nmi_watchdog.c has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS.
| |