From: Cyrill Gorcunov on
On Tue, Mar 30, 2010 at 10:52:38AM -0400, Aristeu Sergio Rozanski Filho wrote:
> > On Sat, Mar 27, 2010 at 10:46:50PM -0400, Aristeu Sergio Rozanski Filho wrote:
> > > Hi Don,
> > > > +/* deprecated */
> > > > +static int __init nosoftlockup_setup(char *str)
> > > > +{
> > > > + no_watchdog = 1;
> > > > + return 1;
> > > > +}
> > > > +__setup("nosoftlockup", nosoftlockup_setup);
> > > > +static int __init nonmi_watchdog_setup(char *str)
> > > > +{
> > > > + no_watchdog = 1;
> > > > + return 1;
> > > > +}
> > > > +__setup("nonmi_watchdog", nonmi_watchdog_setup);
> > > didn't you just add nonmi_watchdog parameter? I don't think there's a reason
> > > to keep compatibility here.
> >
> > Hmm, I think you are right. I thought I added that because it existed in
> > the old nmi_watchdog setup but I can't find it. So yeah, I can drop that.
> you could provide a nmi_watchdog=0 backwards compatibility and warn about
> values != 0
>
> --
> Aristeu
>

Sorry for a long delay, I think we might need to inform a user that "lapic",
"ioapic" is no longer used (perf-nmi is supposed to substitute the former nmi
code in a long term right?) so that for some time period, say the whole release
cycle, if lapic or ioapic, or numbers are passed to nmi_watchdog= setup option
we would just print out that the parameters are deprecated and better to not
use them any longer. Hm?

-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on
On Tue, Mar 23, 2010 at 05:33:38PM -0400, Don Zickus wrote:
> +/* Callback function for perf event subsystem */
> +void watchdog_overflow_callback(struct perf_event *event, int nmi,
> + struct perf_sample_data *data,
> + struct pt_regs *regs)
> +{
> + int this_cpu = smp_processor_id();
> + unsigned long touch_ts = per_cpu(watchdog_touch_ts, this_cpu);
> + int duration;
> +
> + if (touch_ts == 0) {
> + __touch_watchdog();
> + return;
> + }
> +
> + /* check for a hardlockup
> + * This is done by making sure our timer interrupt
> + * is incrementing. The timer interrupt should have
> + * fired multiple times before we overflow'd. If it hasn't
> + * then this is a good indication the cpu is stuck
> + */
> + if (is_hardlockup(this_cpu)) {
> + if (hardlockup_panic)
> + panic("Watchdog detected hard LOCKUP on cpu %d", this_cpu);
> + else
> + WARN(1, "Watchdog detected hard LOCKUP on cpu %d", this_cpu);



panic is going to endless loop so this is fine.
But if you only warn, the path continues, and if you have a
hardlockup then you also have a softlockup, which will probably
warn in turn, making the hardlockup report to vanish. But if
any hardlockup, its report is much more important as it is the real point,
a softlockup that follows is only a consequence of the hardlockup.

Btw, you don't have any cpumask that keeps track of the cpus
that have warned already?


> +static int watchdog_enable(int cpu)
> +{
> + struct perf_event_attr *wd_attr;
> + struct perf_event *event = per_cpu(watchdog_ev, cpu);
> + struct task_struct *p = per_cpu(softlockup_watchdog, cpu);
> +
> + /* is it already setup and enabled? */
> + if (event && event->state > PERF_EVENT_STATE_OFF)
> + goto out;
> +
> + /* it is setup but not enabled */
> + if (event != NULL)
> + goto out_enable;
> +
> + /* Try to register using hardware perf events first */
> + wd_attr = &wd_hw_attr;
> + wd_attr->sample_period = hw_nmi_get_sample_period();
> + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
> + if (!IS_ERR(event)) {
> + printk(KERN_INFO "NMI watchdog enabled, takes one hw-pmu counter.\n");
> + goto out_save;
> + }
> +
> + /* hardware doesn't exist or not supported, fallback to software events */
> + printk(KERN_INFO "NMI watchdog: hardware not available, trying software events\n");
> + wd_attr = &wd_sw_attr;
> + wd_attr->sample_period = softlockup_thresh * NSEC_PER_SEC;
> + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);



I fear the cpu clock is not going to help you detecting any hard lockups.
If you're stuck in an interrupt or an irq disabled loop, your cpu clock is
not going to fire.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Cyrill Gorcunov on
On Tue, Apr 06, 2010 at 04:13:30PM +0200, Frederic Weisbecker wrote:
[...]
> > +static int watchdog_enable(int cpu)
> > +{
> > + struct perf_event_attr *wd_attr;
> > + struct perf_event *event = per_cpu(watchdog_ev, cpu);
> > + struct task_struct *p = per_cpu(softlockup_watchdog, cpu);
> > +
> > + /* is it already setup and enabled? */
> > + if (event && event->state > PERF_EVENT_STATE_OFF)
> > + goto out;
> > +
> > + /* it is setup but not enabled */
> > + if (event != NULL)
> > + goto out_enable;
> > +
> > + /* Try to register using hardware perf events first */
> > + wd_attr = &wd_hw_attr;
> > + wd_attr->sample_period = hw_nmi_get_sample_period();
> > + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
> > + if (!IS_ERR(event)) {
> > + printk(KERN_INFO "NMI watchdog enabled, takes one hw-pmu counter.\n");
> > + goto out_save;
> > + }
> > +
> > + /* hardware doesn't exist or not supported, fallback to software events */
> > + printk(KERN_INFO "NMI watchdog: hardware not available, trying software events\n");
> > + wd_attr = &wd_sw_attr;
> > + wd_attr->sample_period = softlockup_thresh * NSEC_PER_SEC;
> > + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
>
> I fear the cpu clock is not going to help you detecting any hard lockups.
> If you're stuck in an interrupt or an irq disabled loop, your cpu clock is
> not going to fire.
>

I guess it's not supposed to. For such cases only nmi irqs may help for which
the perf events are there (/me need to check if we program apic timer for anything
like that). But it should help for other deadlocks. Or I miss something?

-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Don Zickus on
On Tue, Apr 06, 2010 at 04:13:30PM +0200, Frederic Weisbecker wrote:
> On Tue, Mar 23, 2010 at 05:33:38PM -0400, Don Zickus wrote:
> > +/* Callback function for perf event subsystem */
> > +void watchdog_overflow_callback(struct perf_event *event, int nmi,
> > + struct perf_sample_data *data,
> > + struct pt_regs *regs)
> > +{
> > + int this_cpu = smp_processor_id();
> > + unsigned long touch_ts = per_cpu(watchdog_touch_ts, this_cpu);
> > + int duration;
> > +
> > + if (touch_ts == 0) {
> > + __touch_watchdog();
> > + return;
> > + }
> > +
> > + /* check for a hardlockup
> > + * This is done by making sure our timer interrupt
> > + * is incrementing. The timer interrupt should have
> > + * fired multiple times before we overflow'd. If it hasn't
> > + * then this is a good indication the cpu is stuck
> > + */
> > + if (is_hardlockup(this_cpu)) {
> > + if (hardlockup_panic)
> > + panic("Watchdog detected hard LOCKUP on cpu %d", this_cpu);
> > + else
> > + WARN(1, "Watchdog detected hard LOCKUP on cpu %d", this_cpu);
>
>
>
> panic is going to endless loop so this is fine.
> But if you only warn, the path continues, and if you have a
> hardlockup then you also have a softlockup, which will probably
> warn in turn, making the hardlockup report to vanish. But if
> any hardlockup, its report is much more important as it is the real point,
> a softlockup that follows is only a consequence of the hardlockup.

Good point.

>
> Btw, you don't have any cpumask that keeps track of the cpus
> that have warned already?

No I haven't implemented that. I'll add that to my todo list.

>
>
> > +static int watchdog_enable(int cpu)
> > +{
> > + struct perf_event_attr *wd_attr;
> > + struct perf_event *event = per_cpu(watchdog_ev, cpu);
> > + struct task_struct *p = per_cpu(softlockup_watchdog, cpu);
> > +
> > + /* is it already setup and enabled? */
> > + if (event && event->state > PERF_EVENT_STATE_OFF)
> > + goto out;
> > +
> > + /* it is setup but not enabled */
> > + if (event != NULL)
> > + goto out_enable;
> > +
> > + /* Try to register using hardware perf events first */
> > + wd_attr = &wd_hw_attr;
> > + wd_attr->sample_period = hw_nmi_get_sample_period();
> > + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
> > + if (!IS_ERR(event)) {
> > + printk(KERN_INFO "NMI watchdog enabled, takes one hw-pmu counter.\n");
> > + goto out_save;
> > + }
> > +
> > + /* hardware doesn't exist or not supported, fallback to software events */
> > + printk(KERN_INFO "NMI watchdog: hardware not available, trying software events\n");
> > + wd_attr = &wd_sw_attr;
> > + wd_attr->sample_period = softlockup_thresh * NSEC_PER_SEC;
> > + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
>
>
>
> I fear the cpu clock is not going to help you detecting any hard lockups.
> If you're stuck in an interrupt or an irq disabled loop, your cpu clock is
> not going to fire.

Yup. I put that in the changelog of some nmi code I posted a few weeks
ago. I believe Ingo was looking to have a fallback plan in case hw perf
events wasn't available.

With this code, moving to sw perf events, kinda kills the ability to
detect hard lockups, but you can still detect softlockups.

Cheers,
Don
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Frederic Weisbecker on
On Tue, Apr 06, 2010 at 07:31:15PM +0400, Cyrill Gorcunov wrote:
> On Tue, Apr 06, 2010 at 04:13:30PM +0200, Frederic Weisbecker wrote:
> [...]
> > > +static int watchdog_enable(int cpu)
> > > +{
> > > + struct perf_event_attr *wd_attr;
> > > + struct perf_event *event = per_cpu(watchdog_ev, cpu);
> > > + struct task_struct *p = per_cpu(softlockup_watchdog, cpu);
> > > +
> > > + /* is it already setup and enabled? */
> > > + if (event && event->state > PERF_EVENT_STATE_OFF)
> > > + goto out;
> > > +
> > > + /* it is setup but not enabled */
> > > + if (event != NULL)
> > > + goto out_enable;
> > > +
> > > + /* Try to register using hardware perf events first */
> > > + wd_attr = &wd_hw_attr;
> > > + wd_attr->sample_period = hw_nmi_get_sample_period();
> > > + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
> > > + if (!IS_ERR(event)) {
> > > + printk(KERN_INFO "NMI watchdog enabled, takes one hw-pmu counter.\n");
> > > + goto out_save;
> > > + }
> > > +
> > > + /* hardware doesn't exist or not supported, fallback to software events */
> > > + printk(KERN_INFO "NMI watchdog: hardware not available, trying software events\n");
> > > + wd_attr = &wd_sw_attr;
> > > + wd_attr->sample_period = softlockup_thresh * NSEC_PER_SEC;
> > > + event = perf_event_create_kernel_counter(wd_attr, cpu, -1, watchdog_overflow_callback);
> >
> > I fear the cpu clock is not going to help you detecting any hard lockups.
> > If you're stuck in an interrupt or an irq disabled loop, your cpu clock is
> > not going to fire.
> >
>
> I guess it's not supposed to. For such cases only nmi irqs may help for which
> the perf events are there (/me need to check if we program apic timer for anything
> like that). But it should help for other deadlocks. Or I miss something?


Yeah but only a part of the hardlockup classes. Those that have interrupt
enabled.

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