Skip to contentSkip to conversation

KGDB: various refactorings

RFC PATCH Applied Jan Kiszka 2 participants 4 messages Jan 27, 2008 – Jan 28, 2008

Subsystems (2)
lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

RFC PATCH 1/1 KGDB: various refactorings

Jason,

this is a RFC about another round of refactoring for the KGDB version in x86-git. It's a single patch so far, but I can break out individual hunks on request.

Things I changed:
- Reduce EXPORT_SYMBOLs, switch the rest to EXPORT_SYMBOL_GPL
- Drop code that is (yet) unused
- Rename variables, refactor their types (personal taste, for sure)
- Remove "extern" for function prototypes
- Remove MODULE_LICENSE/DESCRIPTION from kgdb core

Just cherry-pick what you like.

Jan

---
 arch/x86/kernel/kgdb.c |   10 +-
 include/linux/kgdb.h   |  109 ++++++++++++-----------
 kernel/kgdb.c          |  230 +++++++++++++++++++++----------------------------
 3 files changed, 162 insertions(+), 187 deletions(-)
include/linux/kgdb.h +58 -51
Index: b/include/linux/kgdb.h
===================================================================
@@ -12,7 +12,6 @@
  * version 2. This program is licensed "as is" without any warranty of any
  * kind, whether express or implied.
  */
-#ifdef __KERNEL__
 #ifndef _KGDB_H_
 #define _KGDB_H_
 
@@ -34,10 +33,10 @@ struct task_struct;
 struct uart_port;
 
 /* To enter the debugger explicitly. */
-extern void breakpoint(void);
+void breakpoint(void);
+
 extern int kgdb_connected;
 extern int kgdb_may_fault;
-extern struct tasklet_struct kgdb_tasklet_breakpoint;
 
 extern atomic_t kgdb_setting_breakpoint;
 extern atomic_t cpu_doing_single_step;
@@ -45,19 +44,27 @@ extern int kgdb_softlock_skip[NR_CPUS];
 
 extern struct task_struct *kgdb_usethread, *kgdb_contthread;
 
+enum kgdb_initstate {
+	KGDB_UNINITIALIZED = 0,
+	KGDB_SEMI_INITIALIZED,
+	KGDB_FULLY_INITIALIZED
+};
+
+extern enum kgdb_initstate kgdb_state;
+
 enum kgdb_bptype {
-	bp_breakpoint = '0',
-	bp_hardware_breakpoint,
-	bp_write_watchpoint,
-	bp_read_watchpoint,
-	bp_access_watchpoint
+	BP_BREAKPOINT = 0,
+	BP_HARDWARE_BREAKPOINT,
+	BP_WRITE_WATCHPOINT,
+	BP_READ_WATCHPOINT,
+	BP_ACCESS_WATCHPOINT
 };
 
 enum kgdb_bpstate {
-	bp_none = 0,
-	bp_removed,
-	bp_set,
-	bp_active
+	BP_UNDEFINED = 0,
+	BP_REMOVED,
+	BP_SET,
+	BP_ACTIVE
 };
 
 struct kgdb_bkpt {
@@ -68,10 +75,10 @@ struct kgdb_bkpt {
 };
 
 /* The maximum number of KGDB I/O modules that can be loaded */
-#define MAX_KGDB_IO_HANDLERS 3
+#define KGDB_MAX_IO_HANDLERS 3
 
-#ifndef MAX_BREAKPOINTS
-#define MAX_BREAKPOINTS		1000
+#ifndef KGDB_MAX_BREAKPOINTS
+#define KGDB_MAX_BREAKPOINTS	1000
 #endif
 
 #define KGDB_HW_BREAKPOINT	1
@@ -83,20 +90,20 @@ struct kgdb_bkpt {
  *	This function will handle the initalization of any architecture
  *	specific hooks.
  */
-extern int kgdb_arch_init(void);
+int kgdb_arch_init(void);
 
 /**
- *	regs_to_gdb_regs - Convert ptrace regs to GDB regs
+ *	pt_regs_to_gdb_regs - Convert ptrace regs to GDB regs
  *	@gdb_regs: A pointer to hold the registers in the order GDB wants.
  *	@regs: The &struct pt_regs of the current process.
  *
  *	Convert the pt_regs in @regs into the format for registers that
  *	GDB expects, stored in @gdb_regs.
  */
-extern void regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs);
+void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs);
 
 /**
- *	sleeping_regs_to_gdb_regs - Convert ptrace regs to GDB regs
+ *	sleeping_thread_to_gdb_regs - Convert ptrace regs to GDB regs
  *	@gdb_regs: A pointer to hold the registers in the order GDB wants.
  *	@p: The &struct task_struct of the desired process.
  *
@@ -107,18 +114,18 @@ extern void regs_to_gdb_regs(unsigned lo
  *	@gdb_regs with what has	been saved in &struct thread_struct
  *	thread field during switch_to.
  */
-extern void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs,
-					struct task_struct *p);
+void sleeping_thread_to_gdb_regs(unsigned long *gdb_regs,
+				 struct task_struct *p);
 
 /**
- *	gdb_regs_to_regs - Convert GDB regs to ptrace regs.
+ *	gdb_regs_to_pt_regs - Convert GDB regs to ptrace regs.
  *	@gdb_regs: A pointer to hold the registers we've received from GDB.
  *	@regs: A pointer to a &struct pt_regs to hold these values in.
  *
  *	Convert the GDB regs in @gdb_regs into the pt_regs, and store them
  *	in @regs.
  */
-extern void gdb_regs_to_regs(unsigned long *gdb_regs, struct pt_regs *regs);
+void gdb_regs_to_pt_regs(unsigned long *gdb_regs, struct pt_regs *regs);
 
 /**
  *	kgdb_arch_handle_exception - Handle architecture specific GDB packets.
@@ -136,10 +143,10 @@ extern void gdb_regs_to_regs(unsigned lo
  *	process more packets, and a %0 or %1 if it wants to exit from the
  *	kgdb hook.
  */
-extern int kgdb_arch_handle_exception(int vector, int signo, int err_code,
-				      char *remcom_in_buffer,
-				      char *remcom_out_buffer,
-				      struct pt_regs *regs);
+int kgdb_arch_handle_exception(int vector, int signo, int err_code,
+			       char *remcom_in_buffer,
+			       char *remcom_out_buffer,
+			       struct pt_regs *regs);
 
 /**
  * 	kgdb_roundup_cpus - Get other CPUs into a holding pattern
@@ -157,7 +164,7 @@ extern int kgdb_arch_handle_exception(in
  *
  *	On non-SMP systems, this is not called.
  */
-extern void kgdb_roundup_cpus(unsigned long flags);
+void kgdb_roundup_cpus(unsigned long flags);
 
 #ifndef JMP_REGS_ALIGNMENT
 #define JMP_REGS_ALIGNMENT
@@ -173,7 +180,7 @@ extern unsigned long kgdb_fault_jmp_regs
  *	cause a fault.  When this happens, we trap it, restore state to
  *	this call, and let ourself know that something bad has happened.
  */
-extern asmlinkage int kgdb_fault_setjmp(unsigned long *curr_context);
+asmlinkage int kgdb_fault_setjmp(unsigned long *curr_context);
 
 /**
  *	kgdb_fault_longjmp - Restore state when we have faulted.
@@ -183,12 +190,12 @@ extern asmlinkage int kgdb_fault_setjmp(
  *	restore the known good state, and set the return value to 1, so
  *	we know something bad happened.
  */
-extern asmlinkage void kgdb_fault_longjmp(unsigned long *curr_context);
+asmlinkage void kgdb_fault_longjmp(unsigned long *curr_context);
 
 /* Optional functions. */
-extern int kgdb_validate_break_address(unsigned long addr);
-extern int kgdb_arch_set_breakpoint(unsigned long addr, char *saved_instr);
-extern int kgdb_arch_remove_breakpoint(unsigned long addr, char *bundle);
+int kgdb_validate_break_address(unsigned long addr);
+int kgdb_arch_set_breakpoint(unsigned long addr, char *saved_instr);
+int kgdb_arch_remove_breakpoint(unsigned long addr, char *bundle);
 
 /**
  * struct kgdb_arch - Describe architecture specific values.
@@ -259,31 +266,31 @@ struct kgdb_io {
 
 extern struct kgdb_io kgdb_io_ops;
 extern struct kgdb_arch arch_kgdb_ops;
-extern int kgdb_initialized;
 
-extern int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops);
-extern void kgdb_unregister_io_module(struct kgdb_io *local_kgdb_io_ops);
+int kgdb_register_io_module(struct kgdb_io *local_kgdb_io_ops);
+void kgdb_unregister_io_module(struct kgdb_io *local_kgdb_io_ops);
 
-extern void kgdb8250_add_port(int i, struct uart_port *serial_req);
-extern void __init kgdb8250_add_platform_port(int i,
+void kgdb8250_add_port(int i, struct uart_port *serial_req);
+void __init kgdb8250_add_platform_port(int i,
 	struct plat_serial8250_port *serial_req);
 
-extern int kgdb_hex2long(char **ptr, long *long_val);
-extern char *kgdb_mem2hex(char *mem, char *buf, int count);
-extern char *kgdb_hex2mem(char *buf, char *mem, int count);
-extern char *kgdb_get_mem(char *addr, unsigned char *buf, int count);
-extern char *kgdb_set_mem(char *addr, unsigned char *buf, int count);
+int kgdb_hex2long(char **ptr, long *long_val);
+char *kgdb_mem2hex(char *mem, char *buf, int count);
+char *kgdb_hex2mem(char *buf, char *mem, int count);
+char *kgdb_get_mem(char *addr, unsigned char *buf, int count);
+char *kgdb_set_mem(char *addr, unsigned char *buf, int count);
 
 int kgdb_isremovedbreak(unsigned long addr);
 
-extern int kgdb_handle_exception(int ex_vector, int signo, int err_code,
-				struct pt_regs *regs);
-extern int kgdb_nmihook(int cpu, void *regs);
+int kgdb_handle_exception(int ex_vector, int signo, int err_code,
+			  struct pt_regs *regs);
+int kgdb_nmihook(int cpu, void *regs);
+
 extern int debugger_step;
 extern atomic_t debugger_active;
-#else
-/* Stubs for when KGDB is not set. */
+
+#else /* !CONFIG_KGDB */
 static const atomic_t debugger_active = ATOMIC_INIT(0);
-#endif				/* CONFIG_KGDB */
-#endif				/* _KGDB_H_ */
-#endif				/* __KERNEL__ */
+#endif /* !CONFIG_KGDB */
+
+#endif /* _KGDB_H_ */
kernel/kgdb.c +99 -131

562 lines not on this page · raw message

arch/x86/kernel/kgdb.c +5 -5
Index: b/arch/x86/kernel/kgdb.c
===================================================================
@@ -57,7 +57,7 @@ static int gdb_x86errcode;
    number through the usual means, and that's not very specific).  */
 static int gdb_x86vector = -1;
 
-void regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
+void pt_regs_to_gdb_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 {
 	gdb_regs[_AX] = regs->ax;
 	gdb_regs[_BX] = regs->bx;
@@ -122,7 +122,7 @@ void sleeping_thread_to_gdb_regs(unsigne
 	gdb_regs[_SP] = p->thread.sp;
 }
 
-void gdb_regs_to_regs(unsigned long *gdb_regs, struct pt_regs *regs)
+void gdb_regs_to_pt_regs(unsigned long *gdb_regs, struct pt_regs *regs)
 {
 	regs->ax = gdb_regs[_AX];
 	regs->bx = gdb_regs[_BX];
@@ -241,14 +241,14 @@ static int kgdb_set_hw_break(unsigned lo
 		return -1;
 
 	switch (bptype) {
-	case bp_hardware_breakpoint:
+	case BP_HARDWARE_BREAKPOINT:
 		type = 0;
 		len  = 1;
 		break;
-	case bp_write_watchpoint:
+	case BP_WRITE_WATCHPOINT:
 		type = 1;
 		break;
-	case bp_access_watchpoint:
+	case BP_ACCESS_WATCHPOINT:
 		type = 3;
 		break;
 	default:
author
#1 lore raw
Quoted 6 lines · Jan Kiszka

Jason,

this is a RFC about another round of refactoring for the KGDB version in x86-git. It's a single patch so far, but I can break out individual hunks on request.

Quoted 6 lines

Things I changed:
- Reduce EXPORT_SYMBOLs, switch the rest to EXPORT_SYMBOL_GPL
- Drop code that is (yet) unused
- Rename variables, refactor their types (personal taste, for sure)
- Remove "extern" for function prototypes
- Remove MODULE_LICENSE/DESCRIPTION from kgdb core

Sorry, forgot to refresh for this hunk:

- rename CHECK_EXCEPTION_STACK to EXCEPTION_STACK_READY

---
 include/asm-x86/kgdb.h |    2 +-
 include/linux/kgdb.h   |    4 ++--
 kernel/kgdb.c          |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
include/asm-x86/kgdb.h +0 -1
Index: b/include/asm-x86/kgdb.h
===================================================================
@@ -77,7 +77,7 @@ enum regnames { _AX,	/* 0 */
 #ifndef __ASSEMBLY__
 #define BREAKPOINT()		asm("   int $3");
 #ifndef CONFIG_X86_32
-#define CHECK_EXCEPTION_STACK()

((&__get_cpu_var(init_tss))[0].x86_tss.ist[0])
+#define EXCEPTION_STACK_READY()
((&__get_cpu_var(init_tss))[0].x86_tss.ist[0])
#endif /* ! CONFIG_X86_32 */
#define BREAK_INSTR_SIZE 1
#define CACHE_FLUSH_IS_SAFE 1

include/linux/kgdb.h +2 -2
Index: b/include/linux/kgdb.h
===================================================================
@@ -23,8 +23,8 @@
 #include <linux/linkage.h>
 #include <linux/init.h>
 
-#ifndef CHECK_EXCEPTION_STACK
-#define CHECK_EXCEPTION_STACK()	1
+#ifndef EXCEPTION_STACK_READY
+#define EXCEPTION_STACK_READY()	1
 #endif
 
 struct tasklet_struct;
kernel/kgdb.c +1 -1
Index: b/kernel/kgdb.c
===================================================================
@@ -1940,7 +1940,7 @@ static void __init kgdb_early_entry(void
 	 * architecture to hook in and look at us when they are ready.
 	 */
 
-	if (!CHECK_EXCEPTION_STACK()) {
+	if (!EXCEPTION_STACK_READY()) {
 		kgdb_state = KGDB_SEMI_INITIALIZED;
 		/* any kind of break point is deferred to late_init */
 		return;
Applied
#2 lore raw
Quoted 3 lines · Jan Kiszka

Sorry, forgot to refresh for this hunk:

- rename CHECK_EXCEPTION_STACK to EXCEPTION_STACK_READY

thanks, applied to x86.git.

minor nit:

Quoted 5 lines

#ifndef CONFIG_X86_32
-#define CHECK_EXCEPTION_STACK()
((&__get_cpu_var(init_tss))[0].x86_tss.ist[0])
+#define EXCEPTION_STACK_READY()
((&__get_cpu_var(init_tss))[0].x86_tss.ist[0])

not only was the mail in quoted-printable format, but the above part also had linewrapping damage :-/ Fixed it up manually but please read:

Documentation/email-clients.txt

Ingo

Applied
#3 lore raw
In reply to Jan Kiszka
Quoted 6 lines · Jan Kiszka

Things I changed:
- Reduce EXPORT_SYMBOLs, switch the rest to EXPORT_SYMBOL_GPL
- Drop code that is (yet) unused
- Rename variables, refactor their types (personal taste, for sure)
- Remove "extern" for function prototypes
- Remove MODULE_LICENSE/DESCRIPTION from kgdb core

thanks, applied - these look like very sensible cleanups prior merge.

Ingo