Re: [PATCH] fix of unnecessary clocksource change after exiting from KDB console

On Fri, 03 Aug 2007 18:05:05 +1000
Keith Owens <[email protected]> wrote:
> 
> Thanks for the patch and sorry for the delay in replying.  I am trying
> to catch up on a backlog of kdb patches.
> 
> When kdb was written, it was the only kernel debugger and we made the
> mistake of adding kdb_*() helper functions in the rest of the kernel.
> Now we have choices for debuggers, and all of the debuggers have the
> same problems when they stop the system.  So make the change work for
> all debuggers.  Does this work for you?  Against
> kdb-v4.4-2.6.23-rc1-common-1.
> 

I'm afraid that previous patch won't work with SMP kernel with high
resolution timer enabled, because of hres_timers_resume(): /*
 * During resume we might have to reprogram the high resolution timer
 * interrupt (on the local CPU):
 */
void hres_timers_resume(void)
{
	WARN_ON_ONCE(num_online_cpus() > 1);

	/* Retrigger the CPU local events: */
	retrigger_next_event(NULL);
}
When we call hres_timers_resume() from timekeeping_resume() other CPUs should be in offline.
AFAIK, KDB doesn't put CPUs to offline.

Here Is new version on fix. Patch against 2.6.23-rc1. I've tested it on
UP pentium4.
Soft Lockup fix was taken from KGDB mailing list.
To prevent unnecessary clocksource change I call
clocksource_resume_watchdog() instead of suspending whole timekeeping 
system.

Signed-off-by: Konstantin Baydarov <[email protected]>
Signed-off-by: Milind Dumbare <[email protected]>
Signed-off-by: Jason Wessel <[email protected]>
Signed-off-by: Sergey Shtylyov <[email protected]>

 include/linux/clocksource.h |    1 +
 include/linux/kdb.h         |    1 +
 kdb/kdbmain.c               |    8 ++++++++
 kernel/softlockup.c         |    6 ++++++
 kernel/time/clocksource.c   |    5 +++--
 kernel/timer.c              |    6 ++++++
 6 files changed, 25 insertions(+), 2 deletions(-)

Index: linux-2.6.23-rc1/kdb/kdbmain.c
===================================================================
--- linux-2.6.23-rc1.orig/kdb/kdbmain.c
+++ linux-2.6.23-rc1/kdb/kdbmain.c
@@ -41,12 +41,15 @@
 #endif
 #include <linux/cpu.h>
 #include <linux/kdebug.h>
+#include <linux/clocksource.h>
 
 #include <acpi/acpi_bus.h>
 
 #include <asm/system.h>
 #include <asm/kdebug.h>
 
+atomic_t  kdb_sync_softlockup[NR_CPUS] = {ATOMIC_INIT(0)};
+
 /*
  * Kernel debugger state flags
  */
@@ -1904,6 +1907,8 @@ kdb(kdb_reason_t reason, int error, stru
 		KDB_STATE_SET(KDB_CONTROL);
 	}
 
+	atomic_set(&kdb_sync_softlockup[raw_smp_processor_id()], 1);
+
 	/*
 	 * If not entering the debugger due to CPU switch or single step
 	 * reentry, serialize access here.
@@ -2056,6 +2061,9 @@ kdb(kdb_reason_t reason, int error, stru
 	}
 
 	KDB_DEBUG_STATE("kdb 14", result);
+#ifdef CONFIG_CLOCKSOURCE_WATCHDOG
+	clocksource_resume_watchdog();
+#endif
 	kdba_restoreint(&int_state);
 #ifdef  CONFIG_CPU_XSCALE
 	if ( smp_processor_id() == kdb_initial_cpu &&
Index: linux-2.6.23-rc1/kernel/softlockup.c
===================================================================
--- linux-2.6.23-rc1.orig/kernel/softlockup.c
+++ linux-2.6.23-rc1/kernel/softlockup.c
@@ -14,6 +14,9 @@
 #include <linux/kthread.h>
 #include <linux/notifier.h>
 #include <linux/module.h>
+#ifdef CONFIG_KDB
+#include <linux/kdb.h>
+#endif
 
 static DEFINE_SPINLOCK(print_lock);
 
@@ -48,6 +51,9 @@ static unsigned long get_timestamp(void)
 void touch_softlockup_watchdog(void)
 {
 	__raw_get_cpu_var(touch_timestamp) = get_timestamp();
+#ifdef CONFIG_KDB
+	atomic_set(&kdb_sync_softlockup[raw_smp_processor_id()], 0);
+#endif
 }
 EXPORT_SYMBOL(touch_softlockup_watchdog);
 
Index: linux-2.6.23-rc1/kernel/timer.c
===================================================================
--- linux-2.6.23-rc1.orig/kernel/timer.c
+++ linux-2.6.23-rc1/kernel/timer.c
@@ -36,6 +36,9 @@
 #include <linux/delay.h>
 #include <linux/tick.h>
 #include <linux/kallsyms.h>
+#ifdef CONFIG_KDB
+#include <linux/kdb.h>
+#endif
 
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -897,6 +900,9 @@ static void run_timer_softirq(struct sof
 void run_local_timers(void)
 {
 	raise_softirq(TIMER_SOFTIRQ);
+#ifdef CONFIG_KDB
+	if(!atomic_read(&kdb_sync_softlockup[smp_processor_id()]))
+#endif
 	softlockup_tick();
 }
 
Index: linux-2.6.23-rc1/kernel/time/clocksource.c
===================================================================
--- linux-2.6.23-rc1.orig/kernel/time/clocksource.c
+++ linux-2.6.23-rc1/kernel/time/clocksource.c
@@ -147,7 +147,8 @@ static void clocksource_watchdog(unsigne
 	}
 	spin_unlock(&watchdog_lock);
 }
-static void clocksource_resume_watchdog(void)
+
+void clocksource_resume_watchdog(void)
 {
 	set_bit(0, &watchdog_resumed);
 }
@@ -199,7 +200,7 @@ static void clocksource_check_watchdog(s
 		cs->flags |= CLOCK_SOURCE_VALID_FOR_HRES;
 }
 
-static inline void clocksource_resume_watchdog(void) { }
+void clocksource_resume_watchdog(void) { }
 #endif
 
 /**
Index: linux-2.6.23-rc1/include/linux/clocksource.h
===================================================================
--- linux-2.6.23-rc1.orig/include/linux/clocksource.h
+++ linux-2.6.23-rc1/include/linux/clocksource.h
@@ -218,6 +218,7 @@ extern int clocksource_register(struct c
 extern struct clocksource* clocksource_get_next(void);
 extern void clocksource_change_rating(struct clocksource *cs, int rating);
 extern void clocksource_resume(void);
+extern void clocksource_resume_watchdog(void);
 
 #ifdef CONFIG_GENERIC_TIME_VSYSCALL
 extern void update_vsyscall(struct timespec *ts, struct clocksource *c);
Index: linux-2.6.23-rc1/include/linux/kdb.h
===================================================================
--- linux-2.6.23-rc1.orig/include/linux/kdb.h
+++ linux-2.6.23-rc1/include/linux/kdb.h
@@ -162,5 +162,6 @@ int kdb_process_cpu(const struct task_st
 }
 
 extern const char kdb_serial_str[];
+extern atomic_t kdb_sync_softlockup[NR_CPUS];
 
 #endif	/* !_KDB_H */
---------------------------
Use http://oss.sgi.com/ecartis to modify your settings or to unsubscribe.