Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] nmi_watchdog: checkpatch.pl cleanups from earlier patches

2 views
Skip to first unread message

Don Zickus

unread,
Feb 22, 2010, 6:10:03 PM2/22/10
to
Mostly copy/paste whitespace damage with a couple of nitpicks by the script.
Fix the struct definition as requested by Ingo too.

Signed-off-by: Don Zickus <dzi...@redhat.com>
--
Ingo,

I don't know how you want this patch. Like this to put on top or do you
want me to repost everything with the cleanups inside the appropriate patch?
---
arch/x86/kernel/apic/hw_nmi.c | 14 +++++-----
arch/x86/kernel/traps.c | 6 ++--
include/linux/nmi.h | 2 +-
kernel/nmi_watchdog.c | 51 ++++++++++++++++++++---------------------
4 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/arch/x86/kernel/apic/hw_nmi.c b/arch/x86/kernel/apic/hw_nmi.c
index 0b4d205..e8b78a0 100644
--- a/arch/x86/kernel/apic/hw_nmi.c
+++ b/arch/x86/kernel/apic/hw_nmi.c
@@ -38,15 +38,15 @@ static inline unsigned int get_timer_irqs(int cpu)
irqs += per_cpu(irq_stat, cpu).apic_timer_irqs;
#endif

- return irqs;
+ return irqs;
}

static inline int mce_in_progress(void)
{
#if defined(CONFIG_X86_MCE)
- return atomic_read(&mce_entry) > 0;
+ return atomic_read(&mce_entry) > 0;
#endif
- return 0;
+ return 0;
}

int hw_nmi_is_cpu_stuck(struct pt_regs *regs)
@@ -69,9 +69,9 @@ int hw_nmi_is_cpu_stuck(struct pt_regs *regs)
}

/* if we are doing an mce, just assume the cpu is not stuck */
- /* Could check oops_in_progress here too, but it's safer not to */
- if (mce_in_progress())
- return 0;
+ /* Could check oops_in_progress here too, but it's safer not to */
+ if (mce_in_progress())
+ return 0;

/* We determine if the cpu is stuck by checking whether any
* interrupts have happened since we last checked. Of course
@@ -89,7 +89,7 @@ int hw_nmi_is_cpu_stuck(struct pt_regs *regs)

u64 hw_nmi_get_sample_period(void)
{
- return cpu_khz * 1000;
+ return cpu_khz * 1000;
}

#ifdef ARCH_HAS_NMI_WATCHDOG
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 973cbc4..bdc7fab 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -402,9 +402,9 @@ static notrace __kprobes void default_do_nmi(struct pt_regs *regs)
return;

#ifdef CONFIG_X86_LOCAL_APIC
- if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT)
- == NOTIFY_STOP)
- return;
+ if (notify_die(DIE_NMI, "nmi", regs, reason, 2, SIGINT)
+ == NOTIFY_STOP)
+ return;

#ifndef CONFIG_NMI_WATCHDOG
/*
diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index 794e735..22cc796 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -57,7 +57,7 @@ u64 hw_nmi_get_sample_period(void);
extern int nmi_watchdog_enabled;
struct ctl_table;
extern int proc_nmi_enabled(struct ctl_table *, int ,
- void __user *, size_t *, loff_t *);
+ void __user *, size_t *, loff_t *);
#endif

#endif
diff --git a/kernel/nmi_watchdog.c b/kernel/nmi_watchdog.c
index 3c75cbf..0a6f57f 100644
--- a/kernel/nmi_watchdog.c
+++ b/kernel/nmi_watchdog.c
@@ -50,31 +50,31 @@ void touch_all_nmi_watchdog(void)

static int __init setup_nmi_watchdog(char *str)
{
- if (!strncmp(str, "panic", 5)) {
- panic_on_timeout = 1;
- str = strchr(str, ',');
- if (!str)
- return 1;
- ++str;
- }
- return 1;
+ if (!strncmp(str, "panic", 5)) {
+ panic_on_timeout = 1;
+ str = strchr(str, ',');
+ if (!str)
+ return 1;
+ ++str;
+ }
+ return 1;
}
__setup("nmi_watchdog=", setup_nmi_watchdog);

struct perf_event_attr wd_hw_attr = {
- .type = PERF_TYPE_HARDWARE,
- .config = PERF_COUNT_HW_CPU_CYCLES,
- .size = sizeof(struct perf_event_attr),
- .pinned = 1,
- .disabled = 1,
+ .type = PERF_TYPE_HARDWARE,
+ .config = PERF_COUNT_HW_CPU_CYCLES,
+ .size = sizeof(struct perf_event_attr),
+ .pinned = 1,
+ .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,
+ .type = PERF_TYPE_SOFTWARE,
+ .config = PERF_COUNT_SW_CPU_CLOCK,
+ .size = sizeof(struct perf_event_attr),
+ .pinned = 1,
+ .disabled = 1,
};

void wd_overflow(struct perf_event *event, int nmi,
@@ -95,16 +95,15 @@ void wd_overflow(struct perf_event *event, int nmi,
* Ayiee, looks like this CPU is stuck ...
* wait a few IRQs (5 seconds) before doing the oops ...
*/
- per_cpu(alert_counter,cpu) += 1;
- if (per_cpu(alert_counter,cpu) == 5) {
- if (panic_on_timeout) {
+ per_cpu(alert_counter, cpu) += 1;
+ if (per_cpu(alert_counter, cpu) == 5) {
+ if (panic_on_timeout)
panic("NMI Watchdog detected LOCKUP on cpu %d", cpu);
- } else {
+ else
WARN(1, "NMI Watchdog detected LOCKUP on cpu %d", cpu);
- }
}
} else {
- per_cpu(alert_counter,cpu) = 0;
+ per_cpu(alert_counter, cpu) = 0;
}

return;
@@ -126,7 +125,7 @@ static int enable_nmi_watchdog(int cpu)
event = perf_event_create_kernel_counter(wd_attr, cpu, -1, wd_overflow);
if (IS_ERR(event)) {
/* hardware doesn't exist or not supported, fallback to software events */
- printk("nmi_watchdog: hardware not available, trying software events\n");
+ printk(KERN_INFO "nmi_watchdog: hardware not available, trying software events\n");
wd_attr = &wd_sw_attr;
wd_attr->sample_period = NSEC_PER_SEC;
event = perf_event_create_kernel_counter(wd_attr, cpu, -1, wd_overflow);
@@ -182,7 +181,7 @@ int proc_nmi_enabled(struct ctl_table *table, int write,
if (nmi_watchdog_enabled) {
for_each_online_cpu(cpu)
if (enable_nmi_watchdog(cpu)) {
- printk("NMI watchdog failed configuration, "
+ printk(KERN_ERR "NMI watchdog failed configuration, "
" can not be enabled\n");
}
} else {
--
1.6.6.83.gc9a2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Cyrill Gorcunov

unread,
Feb 22, 2010, 6:30:02 PM2/22/10
to
On Mon, Feb 22, 2010 at 06:09:03PM -0500, Don Zickus wrote:
...

Hi Don!

> diff --git a/kernel/nmi_watchdog.c b/kernel/nmi_watchdog.c
> index 3c75cbf..0a6f57f 100644
> --- a/kernel/nmi_watchdog.c
> +++ b/kernel/nmi_watchdog.c
> @@ -50,31 +50,31 @@ void touch_all_nmi_watchdog(void)
>
> static int __init setup_nmi_watchdog(char *str)
> {
> - if (!strncmp(str, "panic", 5)) {
> - panic_on_timeout = 1;
> - str = strchr(str, ',');
> - if (!str)
> - return 1;
> - ++str;
> - }
> - return 1;
> + if (!strncmp(str, "panic", 5)) {
> + panic_on_timeout = 1;

If I understand all the things correct -- you don't need to check for ','
after panic. It seems so. Because we're switching to perf_events I suppose
we may drop expecting "lapic,ioapic" or whatever here? Or there is an idea
to parse say "panic,software" (ie to switch to software events by default)?

Though strictly speaking my question in rather unrelated to this patch
agenda :)

> + str = strchr(str, ',');
> + if (!str)
> + return 1;
> + ++str;
> + }
> + return 1;
> }
> __setup("nmi_watchdog=", setup_nmi_watchdog);

...

-- Cyrill

Cyrill Gorcunov

unread,
Feb 22, 2010, 6:40:01 PM2/22/10
to

If I'm right -- the fix could be done later, after this patch applied.

Don Zickus

unread,
Feb 23, 2010, 10:10:02 AM2/23/10
to
On Tue, Feb 23, 2010 at 02:24:27AM +0300, Cyrill Gorcunov wrote:
> On Mon, Feb 22, 2010 at 06:09:03PM -0500, Don Zickus wrote:
> ...
>
> Hi Don!
>
> > diff --git a/kernel/nmi_watchdog.c b/kernel/nmi_watchdog.c
> > index 3c75cbf..0a6f57f 100644
> > --- a/kernel/nmi_watchdog.c
> > +++ b/kernel/nmi_watchdog.c
> > @@ -50,31 +50,31 @@ void touch_all_nmi_watchdog(void)
> >
> > static int __init setup_nmi_watchdog(char *str)
> > {
> > - if (!strncmp(str, "panic", 5)) {
> > - panic_on_timeout = 1;
> > - str = strchr(str, ',');
> > - if (!str)
> > - return 1;
> > - ++str;
> > - }
> > - return 1;
> > + if (!strncmp(str, "panic", 5)) {
> > + panic_on_timeout = 1;
>
> If I understand all the things correct -- you don't need to check for ','
> after panic. It seems so. Because we're switching to perf_events I suppose
> we may drop expecting "lapic,ioapic" or whatever here? Or there is an idea

ah good point. I was just cutting and pasting things and didn't think to
much about it.

> to parse say "panic,software" (ie to switch to software events by default)?

interesting idea, could be useful.

Thanks,
Don

Ingo Molnar

unread,
Feb 25, 2010, 6:50:03 AM2/25/10
to

* Don Zickus <dzi...@redhat.com> wrote:

> Mostly copy/paste whitespace damage with a couple of nitpicks by the script.
> Fix the struct definition as requested by Ingo too.
>
> Signed-off-by: Don Zickus <dzi...@redhat.com>
> --
> Ingo,
>
> I don't know how you want this patch. Like this to put on top or do you
> want me to repost everything with the cleanups inside the appropriate patch?

It's fine this way - i've put them on top - the base patches have a reasonable
amount of testing track record already, we dont want to destroy that via a
rebase.

(in the future it's useful to run checkpatch as part of your patch submission
checks. In my experience it finds real issues most of the time. Some of the
warnings have to be ignored.)

Ingo

tip-bot for Don Zickus

unread,
Feb 25, 2010, 7:10:02 AM2/25/10
to
Commit-ID: 47195d57636604ff6048b0d7aa3e4ed9643f6073
Gitweb: http://git.kernel.org/tip/47195d57636604ff6048b0d7aa3e4ed9643f6073
Author: Don Zickus <dzi...@redhat.com>
AuthorDate: Mon, 22 Feb 2010 18:09:03 -0500
Committer: Ingo Molnar <mi...@elte.hu>
CommitDate: Thu, 25 Feb 2010 12:40:50 +0100

nmi_watchdog: Clean up various small details

Mostly copy/paste whitespace damage with a couple of nitpicks by

the checkpatch script. Fix the struct definition as requested by Ingo too.

Signed-off-by: Don Zickus <dzi...@redhat.com>
Cc: pet...@infradead.org
Cc: gorc...@gmail.com
Cc: ar...@redhat.com
LKML-Reference: <1266880143-24943-1-g...@redhat.com>
Signed-off-by: Ingo Molnar <mi...@elte.hu>
--
arch/x86/kernel/apic/hw_nmi.c | 14 +++++------
arch/x86/kernel/traps.c | 6 ++--
include/linux/nmi.h | 2 -
kernel/nmi_watchdog.c | 51 ++++++++++++++++++++----------------------

tip-bot for Ingo Molnar

unread,
Feb 28, 2010, 3:00:02 PM2/28/10
to
Commit-ID: c99c30feadf664ccd8590b96259cb9f3954bd246
Gitweb: http://git.kernel.org/tip/c99c30feadf664ccd8590b96259cb9f3954bd246
Author: Ingo Molnar <mi...@elte.hu>
AuthorDate: Sun, 28 Feb 2010 20:49:00 +0100
Committer: Ingo Molnar <mi...@elte.hu>
CommitDate: Sun, 28 Feb 2010 20:49:00 +0100

nmi_watchdog: Turn it off by default

It was nice to enable it by default for testing - but before we
push it upstream we want it to be off - so that people can
opt-in gradually.

Cc: Don Zickus <dzi...@redhat.com>


Cc: pet...@infradead.org
Cc: gorc...@gmail.com
Cc: ar...@redhat.com
LKML-Reference: <1266880143-24943-1-g...@redhat.com>
Signed-off-by: Ingo Molnar <mi...@elte.hu>

---
lib/Kconfig.debug | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 01a4d85..e2e73cc 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -173,7 +173,6 @@ config DETECT_SOFTLOCKUP
config NMI_WATCHDOG
bool "Detect Hard Lockups with an NMI Watchdog"
depends on DEBUG_KERNEL && PERF_EVENTS && PERF_EVENTS_NMI
- default y
help
Say Y here to enable the kernel to use the NMI as a watchdog
to detect hard lockups. This is useful when a cpu hangs for no

0 new messages