Clean up the die notifier registration routines

PATCH In review Keith Owens 2 participants 3 messages Nov 15, 2005 PowerPC

Share & tools

Keith Owens

author

2005-11-15 04:00 UTC #0 lore raw

PATCH 1/1 Clean up the die notifier registration routines

Remove the extraneous die_notifier_lock, notifier_chain_register() and notifier_chain_unregister already take a lock. Also there is some work being done to make the generic notify code race safe and the die_notifier_lock would get in that way of that rework. See
http://marc.theaimsgroup.com/?l=linux-kernel&m=113175279131346&w=2

Add unregister_die_notifier() and export it on all architectures.

Export register_die_notifier() on some architectures, others already had an export.

Correct include/asm-*/kdebug.h to define the functions as extern.

Remove trailing whitespace in include/asm-x86_64/kdebug.h.

Compile tested on i386 and x86_64, powerpc and sparc64 have only been desk checked. The ia64 version of this code has already been changed in 2.6.15-rc1.

Signed-off-by Keith Owens <[email protected]>

---

 arch/i386/kernel/traps.c     |   14 +++++++-------
 arch/powerpc/kernel/traps.c  |   14 +++++++-------
 arch/sparc64/kernel/traps.c  |   15 ++++++++-------
 arch/x86_64/kernel/traps.c   |   15 ++++++++-------
 include/asm-i386/kdebug.h    |    6 ++----
 include/asm-powerpc/kdebug.h |    8 ++------
 include/asm-sparc64/kdebug.h |    7 ++-----
 include/asm-x86_64/kdebug.h  |   26 ++++++++++++--------------
 8 files changed, 48 insertions(+), 57 deletions(-)

arch/i386/kernel/traps.c +7 -7

Index: linux/arch/i386/kernel/traps.c
===================================================================
@@ -93,19 +93,19 @@ asmlinkage void machine_check(void);
 
 static int kstack_depth_to_print = 24;
 struct notifier_block *i386die_chain;
-static DEFINE_SPINLOCK(die_notifier_lock);
 
 int register_die_notifier(struct notifier_block *nb)
 {
-	int err = 0;
-	unsigned long flags;
-	spin_lock_irqsave(&die_notifier_lock, flags);
-	err = notifier_chain_register(&i386die_chain, nb);
-	spin_unlock_irqrestore(&die_notifier_lock, flags);
-	return err;
+	return notifier_chain_register(&i386die_chain, nb);
 }
 EXPORT_SYMBOL(register_die_notifier);
 
+int unregister_die_notifier(struct notifier_block *nb)
+{
+	return notifier_chain_unregister(&i386die_chain, nb);
+}
+EXPORT_SYMBOL(unregister_die_notifier);
+
 static inline int valid_stack_ptr(struct thread_info *tinfo, void *p)
 {
 	return	p > (void *)tinfo &&

arch/powerpc/kernel/traps.c +7 -7

Index: linux/arch/powerpc/kernel/traps.c
===================================================================
@@ -74,18 +74,18 @@ EXPORT_SYMBOL(__debugger_fault_handler);
 #endif
 
 struct notifier_block *powerpc_die_chain;
-static DEFINE_SPINLOCK(die_notifier_lock);
 
 int register_die_notifier(struct notifier_block *nb)
 {
-	int err = 0;
-	unsigned long flags;
+	return notifier_chain_register(&powerpc_die_chain, nb);
+}
+EXPORT_SYMBOL(register_die_notifier);
 
-	spin_lock_irqsave(&die_notifier_lock, flags);
-	err = notifier_chain_register(&powerpc_die_chain, nb);
-	spin_unlock_irqrestore(&die_notifier_lock, flags);
-	return err;
+int unregister_die_notifier(struct notifier_block *nb)
+{
+	return notifier_chain_unregister(&powerpc_die_chain, nb);
 }
+EXPORT_SYMBOL(unregister_die_notifier);
 
 /*
  * Trap & Exception support

arch/sparc64/kernel/traps.c +8 -7

Index: linux/arch/sparc64/kernel/traps.c
===================================================================
@@ -43,17 +43,18 @@
 #endif
 
 struct notifier_block *sparc64die_chain;
-static DEFINE_SPINLOCK(die_notifier_lock);
 
 int register_die_notifier(struct notifier_block *nb)
 {
-	int err = 0;
-	unsigned long flags;
-	spin_lock_irqsave(&die_notifier_lock, flags);
-	err = notifier_chain_register(&sparc64die_chain, nb);
-	spin_unlock_irqrestore(&die_notifier_lock, flags);
-	return err;
+	return notifier_chain_register(&sparc64die_chain, nb);
 }
+EXPORT_SYMBOL(register_die_notifier);
+
+int unregister_die_notifier(struct notifier_block *nb)
+{
+	return notifier_chain_unregister(&sparc64die_chain, nb);
+}
+EXPORT_SYMBOL(unregister_die_notifier);
 
 /* When an irrecoverable trap occurs at tl > 0, the trap entry
  * code logs the trap state registers at every level in the trap

arch/x86_64/kernel/traps.c +8 -7

Index: linux/arch/x86_64/kernel/traps.c
===================================================================
@@ -73,17 +73,18 @@ asmlinkage void spurious_interrupt_bug(v
 asmlinkage void call_debug(void);
 
 struct notifier_block *die_chain;
-static DEFINE_SPINLOCK(die_notifier_lock);
 
 int register_die_notifier(struct notifier_block *nb)
 {
-	int err = 0;
-	unsigned long flags;
-	spin_lock_irqsave(&die_notifier_lock, flags);
-	err = notifier_chain_register(&die_chain, nb);
-	spin_unlock_irqrestore(&die_notifier_lock, flags);
-	return err;
+	return notifier_chain_register(&die_chain, nb);
+}
+EXPORT_SYMBOL(register_die_notifier);
+
+int unregister_die_notifier(struct notifier_block *nb)
+{
+	return notifier_chain_unregister(&die_chain, nb);
 }
+EXPORT_SYMBOL(unregister_die_notifier);
 
 static inline void conditional_sti(struct pt_regs *regs)
 {

include/asm-i386/kdebug.h +2 -4

Index: linux/include/asm-i386/kdebug.h
===================================================================
@@ -17,10 +17,8 @@ struct die_args {
 	int signr;
 };
 
-/* Note - you should never unregister because that can race with NMIs.
-   If you really want to do it first unregister - then synchronize_sched - then free.
-  */
-int register_die_notifier(struct notifier_block *nb);
+extern int register_die_notifier(struct notifier_block *);
+extern int unregister_die_notifier(struct notifier_block *);
 extern struct notifier_block *i386die_chain;

include/asm-powerpc/kdebug.h +2 -6

Index: linux/include/asm-powerpc/kdebug.h
===================================================================
@@ -15,12 +15,8 @@ struct die_args {
 	int signr;
 };
 
-/*
-   Note - you should never unregister because that can race with NMIs.
-   If you really want to do it first unregister - then synchronize_sched -
-   then free.
- */
-int register_die_notifier(struct notifier_block *nb);
+extern int register_die_notifier(struct notifier_block *);
+extern int unregister_die_notifier(struct notifier_block *);
 extern struct notifier_block *powerpc_die_chain;
 
 /* Grossly misnamed. */

include/asm-sparc64/kdebug.h +2 -5

Index: linux/include/asm-sparc64/kdebug.h
===================================================================
@@ -15,11 +15,8 @@ struct die_args {
 	int signr;
 };
 
-/* Note - you should never unregister because that can race with NMIs.
- * If you really want to do it first unregister - then synchronize_sched
- * - then free.
- */
-int register_die_notifier(struct notifier_block *nb);
+extern int register_die_notifier(struct notifier_block *);
+extern int unregister_die_notifier(struct notifier_block *);
 extern struct notifier_block *sparc64die_chain;
 
 extern void bad_trap(struct pt_regs *, long);

include/asm-x86_64/kdebug.h +12 ~~-14~~

Index: linux/include/asm-x86_64/kdebug.h
===================================================================
@@ -5,21 +5,19 @@
 
 struct pt_regs;
 
-struct die_args { 
+struct die_args {
 	struct pt_regs *regs;
 	const char *str;
-	long err; 
+	long err;
 	int trapnr;
 	int signr;
-}; 
+};
 
-/* Note - you should never unregister because that can race with NMIs.
-   If you really want to do it first unregister - then synchronize_sched - then free.
-  */
-int register_die_notifier(struct notifier_block *nb);
+extern int register_die_notifier(struct notifier_block *);
+extern int unregister_die_notifier(struct notifier_block *);
 extern struct notifier_block *die_chain;
 /* Grossly misnamed. */
-enum die_val { 
+enum die_val {
 	DIE_OOPS = 1,
 	DIE_INT3,
 	DIE_DEBUG,

@@ -33,13 +31,13 @@ enum die_val { 
 	DIE_CALL,
 	DIE_NMI_IPI,
 	DIE_PAGE_FAULT,
-}; 
-	
+};
+
 static inline int notify_die(enum die_val val,char *str,struct pt_regs *regs,long err,int trap, int sig)
-{ 
-	struct die_args args = { .regs=regs, .str=str, .err=err, .trapnr=trap,.signr=sig }; 
-	return notifier_call_chain(&die_chain, val, &args); 
-} 
+{
+	struct die_args args = { .regs=regs, .str=str, .err=err, .trapnr=trap,.signr=sig };
+	return notifier_call_chain(&die_chain, val, &args);
+}
 
 extern int printk_address(unsigned long address);
 extern void die(const char *,struct pt_regs *,long);

Alan Stern

2005-11-15 15:09 UTC #1 lore raw

Quoted 11 lines · Keith Owens

Remove the extraneous die_notifier_lock, notifier_chain_register() and notifier_chain_unregister already take a lock. Also there is some work being done to make the generic notify code race safe and the die_notifier_lock would get in that way of that rework. See
http://marc.theaimsgroup.com/?l=linux-kernel&m=113175279131346&w=2
Add unregister_die_notifier() and export it on all architectures.
Export register_die_notifier() on some architectures, others already had an export.

Quoted 7 lines

Correct include/asm-*/kdebug.h to define the functions as extern.
Remove trailing whitespace in include/asm-x86_64/kdebug.h.
Compile tested on i386 and x86_64, powerpc and sparc64 have only been desk checked. The ia64 version of this code has already been changed in 2.6.15-rc1.

Keith, can you delay this patch until the general notifier fixups have been merged? Most of what you did duplicates changes that we are making and some of it will clash.

Alan Stern

Keith Owens

author

2005-11-15 21:03 UTC #2 lore raw

Quoted 11 lines · Alan Stern

Quoted 5 lines · Keith Owens
Remove the extraneous die_notifier_lock, notifier_chain_register() and notifier_chain_unregister already take a lock. Also there is some work being done to make the generic notify code race safe and the die_notifier_lock would get in that way of that rework. See
http://marc.theaimsgroup.com/?l=linux-kernel&m=113175279131346&w=2
Keith, can you delay this patch until the general notifier fixups have been merged? Most of what you did duplicates changes that we are making and some of it will clash.

No problem. SInce you are doing notifier cleanups as well, feel free to fold my patch into yours.