Re: [patch -mm 10/28] highres: Improve debug output
On Mon, 2007-06-25 at 17:14 -0700, Andrew Morton wrote: > On Sat, 23 Jun 2007 13:32:35 - > Thomas Gleixner <[EMAIL PROTECTED]> wrote: > > > if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT) || > > - !tick_device_is_functional(dev)) > > + !tick_device_is_functional(dev)) { > > + > > + printk(KERN_INFO "Clockevents: " > > + "could not switch to one-shot mode:"); > > + if (!dev) { > > + printk(" no tick device\n"); > > + } else { > > + if (!tick_device_is_functional(dev)) > > + printk(" %s is not functional.\n", dev->name); > > + else if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT)) > > + printk(" %s does not support one-shot mode.\n", > > + dev->name); > > + } > > There is a logic path through here where the printk doesn't get its \n > termination? And it will fail to print the reason for the failure, too. > > Maybe that's a can't-happen, in which case the CLOCK_EVT_FEAT_ONESHOT test > is superfluous? Right, the "else if (...)" test is bogus. A simple "else" is sufficient. tglx - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch -mm 10/28] highres: Improve debug output
On Mon, 2007-06-25 at 17:14 -0700, Andrew Morton wrote: On Sat, 23 Jun 2007 13:32:35 - Thomas Gleixner [EMAIL PROTECTED] wrote: if (!dev || !(dev-features CLOCK_EVT_FEAT_ONESHOT) || - !tick_device_is_functional(dev)) + !tick_device_is_functional(dev)) { + + printk(KERN_INFO Clockevents: + could not switch to one-shot mode:); + if (!dev) { + printk( no tick device\n); + } else { + if (!tick_device_is_functional(dev)) + printk( %s is not functional.\n, dev-name); + else if (!(dev-features CLOCK_EVT_FEAT_ONESHOT)) + printk( %s does not support one-shot mode.\n, + dev-name); + } There is a logic path through here where the printk doesn't get its \n termination? And it will fail to print the reason for the failure, too. Maybe that's a can't-happen, in which case the CLOCK_EVT_FEAT_ONESHOT test is superfluous? Right, the else if (...) test is bogus. A simple else is sufficient. tglx - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch -mm 10/28] highres: Improve debug output
On Sat, 23 Jun 2007 13:32:35 - Thomas Gleixner <[EMAIL PROTECTED]> wrote: > if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT) || > - !tick_device_is_functional(dev)) > + !tick_device_is_functional(dev)) { > + > + printk(KERN_INFO "Clockevents: " > +"could not switch to one-shot mode:"); > + if (!dev) { > + printk(" no tick device\n"); > + } else { > + if (!tick_device_is_functional(dev)) > + printk(" %s is not functional.\n", dev->name); > + else if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT)) > + printk(" %s does not support one-shot mode.\n", > +dev->name); > + } There is a logic path through here where the printk doesn't get its \n termination? And it will fail to print the reason for the failure, too. Maybe that's a can't-happen, in which case the CLOCK_EVT_FEAT_ONESHOT test is superfluous? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch -mm 10/28] highres: Improve debug output
On Sat, 23 Jun 2007 13:32:35 - Thomas Gleixner [EMAIL PROTECTED] wrote: if (!dev || !(dev-features CLOCK_EVT_FEAT_ONESHOT) || - !tick_device_is_functional(dev)) + !tick_device_is_functional(dev)) { + + printk(KERN_INFO Clockevents: +could not switch to one-shot mode:); + if (!dev) { + printk( no tick device\n); + } else { + if (!tick_device_is_functional(dev)) + printk( %s is not functional.\n, dev-name); + else if (!(dev-features CLOCK_EVT_FEAT_ONESHOT)) + printk( %s does not support one-shot mode.\n, +dev-name); + } There is a logic path through here where the printk doesn't get its \n termination? And it will fail to print the reason for the failure, too. Maybe that's a can't-happen, in which case the CLOCK_EVT_FEAT_ONESHOT test is superfluous? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch -mm 10/28] highres: Improve debug output
From: Ingo Molnar <[EMAIL PROTECTED]> Add some more debug information to the hrtimer and clock events code. Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]> Signed-off-by: Thomas Gleixner <[EMAIL PROTECTED]> --- arch/i386/kernel/apic.c|3 +++ kernel/hrtimer.c |5 - kernel/time/tick-oneshot.c | 15 ++- 3 files changed, 21 insertions(+), 2 deletions(-) Index: linux-2.6.22-rc4-mm/arch/i386/kernel/apic.c === --- linux-2.6.22-rc4-mm.orig/arch/i386/kernel/apic.c2007-06-23 14:38:58.0 +0200 +++ linux-2.6.22-rc4-mm/arch/i386/kernel/apic.c 2007-06-23 14:38:58.0 +0200 @@ -524,6 +524,9 @@ void __init setup_boot_APIC_clock(void) */ if (nmi_watchdog != NMI_IO_APIC) lapic_clockevent.features &= ~CLOCK_EVT_FEAT_DUMMY; + else + printk(KERN_WARNING "APIC timer registered as dummy," + " due to nmi_watchdog=1!\n"); } /* Setup the lapic or request the broadcast */ Index: linux-2.6.22-rc4-mm/kernel/hrtimer.c === --- linux-2.6.22-rc4-mm.orig/kernel/hrtimer.c 2007-06-23 14:38:56.0 +0200 +++ linux-2.6.22-rc4-mm/kernel/hrtimer.c2007-06-23 14:38:58.0 +0200 @@ -558,7 +558,8 @@ static inline int hrtimer_enqueue_reprog */ static int hrtimer_switch_to_hres(void) { - struct hrtimer_cpu_base *base = &__get_cpu_var(hrtimer_bases); + int cpu = smp_processor_id(); + struct hrtimer_cpu_base *base = _cpu(hrtimer_bases, cpu); unsigned long flags; if (base->hres_active) @@ -568,6 +569,8 @@ static int hrtimer_switch_to_hres(void) if (tick_init_highres()) { local_irq_restore(flags); + printk(KERN_WARNING "Could not switch to high resolution " + "mode on CPU %d\n", cpu); return 0; } base->hres_active = 1; Index: linux-2.6.22-rc4-mm/kernel/time/tick-oneshot.c === --- linux-2.6.22-rc4-mm.orig/kernel/time/tick-oneshot.c 2007-06-23 14:38:56.0 +0200 +++ linux-2.6.22-rc4-mm/kernel/time/tick-oneshot.c 2007-06-23 14:38:58.0 +0200 @@ -73,8 +73,21 @@ int tick_switch_to_oneshot(void (*handle struct clock_event_device *dev = td->evtdev; if (!dev || !(dev->features & CLOCK_EVT_FEAT_ONESHOT) || - !tick_device_is_functional(dev)) + !tick_device_is_functional(dev)) { + + printk(KERN_INFO "Clockevents: " + "could not switch to one-shot mode:"); + if (!dev) { + printk(" no tick device\n"); + } else { + if (!tick_device_is_functional(dev)) + printk(" %s is not functional.\n", dev->name); + else if (!(dev->features & CLOCK_EVT_FEAT_ONESHOT)) + printk(" %s does not support one-shot mode.\n", + dev->name); + } return -EINVAL; + } td->mode = TICKDEV_MODE_ONESHOT; dev->event_handler = handler; -- - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[patch -mm 10/28] highres: Improve debug output
From: Ingo Molnar [EMAIL PROTECTED] Add some more debug information to the hrtimer and clock events code. Signed-off-by: Ingo Molnar [EMAIL PROTECTED] Signed-off-by: Thomas Gleixner [EMAIL PROTECTED] --- arch/i386/kernel/apic.c|3 +++ kernel/hrtimer.c |5 - kernel/time/tick-oneshot.c | 15 ++- 3 files changed, 21 insertions(+), 2 deletions(-) Index: linux-2.6.22-rc4-mm/arch/i386/kernel/apic.c === --- linux-2.6.22-rc4-mm.orig/arch/i386/kernel/apic.c2007-06-23 14:38:58.0 +0200 +++ linux-2.6.22-rc4-mm/arch/i386/kernel/apic.c 2007-06-23 14:38:58.0 +0200 @@ -524,6 +524,9 @@ void __init setup_boot_APIC_clock(void) */ if (nmi_watchdog != NMI_IO_APIC) lapic_clockevent.features = ~CLOCK_EVT_FEAT_DUMMY; + else + printk(KERN_WARNING APIC timer registered as dummy, + due to nmi_watchdog=1!\n); } /* Setup the lapic or request the broadcast */ Index: linux-2.6.22-rc4-mm/kernel/hrtimer.c === --- linux-2.6.22-rc4-mm.orig/kernel/hrtimer.c 2007-06-23 14:38:56.0 +0200 +++ linux-2.6.22-rc4-mm/kernel/hrtimer.c2007-06-23 14:38:58.0 +0200 @@ -558,7 +558,8 @@ static inline int hrtimer_enqueue_reprog */ static int hrtimer_switch_to_hres(void) { - struct hrtimer_cpu_base *base = __get_cpu_var(hrtimer_bases); + int cpu = smp_processor_id(); + struct hrtimer_cpu_base *base = per_cpu(hrtimer_bases, cpu); unsigned long flags; if (base-hres_active) @@ -568,6 +569,8 @@ static int hrtimer_switch_to_hres(void) if (tick_init_highres()) { local_irq_restore(flags); + printk(KERN_WARNING Could not switch to high resolution + mode on CPU %d\n, cpu); return 0; } base-hres_active = 1; Index: linux-2.6.22-rc4-mm/kernel/time/tick-oneshot.c === --- linux-2.6.22-rc4-mm.orig/kernel/time/tick-oneshot.c 2007-06-23 14:38:56.0 +0200 +++ linux-2.6.22-rc4-mm/kernel/time/tick-oneshot.c 2007-06-23 14:38:58.0 +0200 @@ -73,8 +73,21 @@ int tick_switch_to_oneshot(void (*handle struct clock_event_device *dev = td-evtdev; if (!dev || !(dev-features CLOCK_EVT_FEAT_ONESHOT) || - !tick_device_is_functional(dev)) + !tick_device_is_functional(dev)) { + + printk(KERN_INFO Clockevents: + could not switch to one-shot mode:); + if (!dev) { + printk( no tick device\n); + } else { + if (!tick_device_is_functional(dev)) + printk( %s is not functional.\n, dev-name); + else if (!(dev-features CLOCK_EVT_FEAT_ONESHOT)) + printk( %s does not support one-shot mode.\n, + dev-name); + } return -EINVAL; + } td-mode = TICKDEV_MODE_ONESHOT; dev-event_handler = handler; -- - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/