Nuke db_is_active in favor of db_active

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Nuke db_is_active in favor of db_active

Christian Ludwig-3
We have two variables with the same meaning. db_active is used in way
more places, so let's nuke db_is_active.

Now that db_active is in <sys/systm.h> for a while already and not
guarded by DDB anymore, take the opportunity to clean up some places
that use it.
---
 sys/arch/macppc/dev/zs.c               |  8 ++------
 sys/arch/sparc64/sparc64/ipifuncs.c    |  2 --
 sys/ddb/db_trap.c                      |  2 --
 sys/ddb/db_var.h                       |  1 -
 sys/dev/pci/drm/include/linux/kernel.h |  4 +---
 sys/dev/pci/drm/include/linux/kgdb.h   |  4 ++--
 sys/kern/subr_prf.c                    | 14 ++------------
 sys/kern/subr_witness.c                |  2 +-
 8 files changed, 8 insertions(+), 29 deletions(-)

diff --git a/sys/arch/macppc/dev/zs.c b/sys/arch/macppc/dev/zs.c
index ba4453683ca..f6f1901a3e1 100644
--- a/sys/arch/macppc/dev/zs.c
+++ b/sys/arch/macppc/dev/zs.c
@@ -1091,12 +1091,8 @@ zs_abort(struct zs_chanstate *channel)
  } while (rr0 & ZSRR0_BREAK);
 
 #if defined(DDB)
- {
- extern int db_active;
-
- if (!db_active)
- db_enter();
- }
+ if (!db_active)
+ db_enter();
 #endif
 }
 
diff --git a/sys/arch/sparc64/sparc64/ipifuncs.c b/sys/arch/sparc64/sparc64/ipifuncs.c
index 1ffeb666bf4..54603319a1e 100644
--- a/sys/arch/sparc64/sparc64/ipifuncs.c
+++ b/sys/arch/sparc64/sparc64/ipifuncs.c
@@ -39,8 +39,6 @@
 #include <machine/pmap.h>
 #include <machine/sparc64.h>
 
-extern int db_active;
-
 #define SPARC64_IPI_RETRIES 10000
 
 #define sparc64_ipi_sleep() delay(1000)
diff --git a/sys/ddb/db_trap.c b/sys/ddb/db_trap.c
index 85467256e61..f1c6317a715 100644
--- a/sys/ddb/db_trap.c
+++ b/sys/ddb/db_trap.c
@@ -52,7 +52,6 @@ db_trap(int type, int code)
  boolean_t bkpt;
  boolean_t watchpt;
 
- db_is_active = 1;
  bkpt = IS_BREAKPOINT_TRAP(type, code);
  watchpt = IS_WATCHPOINT_TRAP(type, code);
 
@@ -94,5 +93,4 @@ db_trap(int type, int code)
  }
 
  db_restart_at_pc(&ddb_regs, watchpt);
- db_is_active = 0;
 }
diff --git a/sys/ddb/db_var.h b/sys/ddb/db_var.h
index 3bb02b5d34d..f264aaa6c7f 100644
--- a/sys/ddb/db_var.h
+++ b/sys/ddb/db_var.h
@@ -67,7 +67,6 @@ extern int db_max_line;
 extern int db_panic;
 extern int db_console;
 extern int db_log;
-extern int db_is_active;
 extern int db_profile;
 
 int ddb_sysctl(int *, u_int, void *, size_t *, void *, size_t,
diff --git a/sys/dev/pci/drm/include/linux/kernel.h b/sys/dev/pci/drm/include/linux/kernel.h
index 188efad2f4f..d0b274c88c8 100644
--- a/sys/dev/pci/drm/include/linux/kernel.h
+++ b/sys/dev/pci/drm/include/linux/kernel.h
@@ -9,8 +9,6 @@
 #include <sys/stdarg.h>
 #include <sys/malloc.h>
 
-#include <ddb/db_var.h>
-
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <linux/bitops.h>
@@ -119,7 +117,7 @@ static inline int
 _in_dbg_master(void)
 {
 #ifdef DDB
- return (db_is_active);
+ return (db_active);
 #endif
  return (0);
 }
diff --git a/sys/dev/pci/drm/include/linux/kgdb.h b/sys/dev/pci/drm/include/linux/kgdb.h
index 73759b3be75..874a0ebe0be 100644
--- a/sys/dev/pci/drm/include/linux/kgdb.h
+++ b/sys/dev/pci/drm/include/linux/kgdb.h
@@ -3,13 +3,13 @@
 #ifndef _LINUX_KGDB_H
 #define _LINUX_KGDB_H
 
-#include <ddb/db_var.h>
+#include <sys/systm.h>
 
 static inline int
 in_dbg_master(void)
 {
 #ifdef DDB
- return (db_is_active);
+ return (db_active);
 #endif
  return (0);
 }
diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c
index 6abac53452a..87bb23d1ded 100644
--- a/sys/kern/subr_prf.c
+++ b/sys/kern/subr_prf.c
@@ -118,11 +118,6 @@ int db_console = 1;
 #else
 int db_console = 0;
 #endif
-
-/*
- * flag to indicate if we are currently in ddb (on some processor)
- */
-int db_is_active;
 #endif
 
 /*
@@ -330,16 +325,11 @@ void
 kputchar(int c, int flags, struct tty *tp)
 {
  extern int msgbufmapped;
- int ddb_active = 0;
-
-#ifdef DDB
- ddb_active = db_is_active;
-#endif
 
  if (panicstr)
  constty = NULL;
 
- if ((flags & TOCONS) && tp == NULL && constty && !ddb_active) {
+ if ((flags & TOCONS) && tp == NULL && constty && !db_active) {
  tp = constty;
  flags |= TOTTY;
  }
@@ -349,7 +339,7 @@ kputchar(int c, int flags, struct tty *tp)
  if ((flags & TOLOG) &&
     c != '\0' && c != '\r' && c != 0177 && msgbufmapped)
  msgbuf_putchar(msgbufp, c);
- if ((flags & TOCONS) && (constty == NULL || ddb_active) && c != '\0')
+ if ((flags & TOCONS) && (constty == NULL || db_active) && c != '\0')
  (*v_putc)(c);
 #ifdef DDB
  if (flags & TODDB)
diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
index 6a45ccd3735..5c6ecfe62de 100644
--- a/sys/kern/subr_witness.c
+++ b/sys/kern/subr_witness.c
@@ -2051,7 +2051,7 @@ witness_ddb_list(struct proc *p)
  struct witness_cpu *wc = &witness_cpu[cpu_number()];
 
  KASSERTMSG(witness_cold == 0, "%s: witness_cold", __func__);
- KASSERTMSG(db_is_active, "%s: not in the debugger", __func__);
+ KASSERTMSG(db_active, "%s: not in the debugger", __func__);
 
  if (witness_watch < 1)
  return;
--
2.22.0

Reply | Threaded
Open this post in threaded view
|

Re: Nuke db_is_active in favor of db_active

Martin Pieuchot
On 26/06/19(Wed) 19:13, Christian Ludwig wrote:
> We have two variables with the same meaning. db_active is used in way
> more places, so let's nuke db_is_active.

Thanks for the cleanup!

> Now that db_active is in <sys/systm.h> for a while already and not
> guarded by DDB anymore, take the opportunity to clean up some places
> that use it.

At least m88k and powerpc do not set db_active before calling db_trap().

This needs to be fixed first :)

> ---
>  sys/arch/macppc/dev/zs.c               |  8 ++------
>  sys/arch/sparc64/sparc64/ipifuncs.c    |  2 --
>  sys/ddb/db_trap.c                      |  2 --
>  sys/ddb/db_var.h                       |  1 -
>  sys/dev/pci/drm/include/linux/kernel.h |  4 +---
>  sys/dev/pci/drm/include/linux/kgdb.h   |  4 ++--
>  sys/kern/subr_prf.c                    | 14 ++------------
>  sys/kern/subr_witness.c                |  2 +-
>  8 files changed, 8 insertions(+), 29 deletions(-)
>
> diff --git a/sys/arch/macppc/dev/zs.c b/sys/arch/macppc/dev/zs.c
> index ba4453683ca..f6f1901a3e1 100644
> --- a/sys/arch/macppc/dev/zs.c
> +++ b/sys/arch/macppc/dev/zs.c
> @@ -1091,12 +1091,8 @@ zs_abort(struct zs_chanstate *channel)
>   } while (rr0 & ZSRR0_BREAK);
>  
>  #if defined(DDB)
> - {
> - extern int db_active;
> -
> - if (!db_active)
> - db_enter();
> - }
> + if (!db_active)
> + db_enter();
>  #endif
>  }
>  
> diff --git a/sys/arch/sparc64/sparc64/ipifuncs.c b/sys/arch/sparc64/sparc64/ipifuncs.c
> index 1ffeb666bf4..54603319a1e 100644
> --- a/sys/arch/sparc64/sparc64/ipifuncs.c
> +++ b/sys/arch/sparc64/sparc64/ipifuncs.c
> @@ -39,8 +39,6 @@
>  #include <machine/pmap.h>
>  #include <machine/sparc64.h>
>  
> -extern int db_active;
> -
>  #define SPARC64_IPI_RETRIES 10000
>  
>  #define sparc64_ipi_sleep() delay(1000)
> diff --git a/sys/ddb/db_trap.c b/sys/ddb/db_trap.c
> index 85467256e61..f1c6317a715 100644
> --- a/sys/ddb/db_trap.c
> +++ b/sys/ddb/db_trap.c
> @@ -52,7 +52,6 @@ db_trap(int type, int code)
>   boolean_t bkpt;
>   boolean_t watchpt;
>  
> - db_is_active = 1;
>   bkpt = IS_BREAKPOINT_TRAP(type, code);
>   watchpt = IS_WATCHPOINT_TRAP(type, code);
>  
> @@ -94,5 +93,4 @@ db_trap(int type, int code)
>   }
>  
>   db_restart_at_pc(&ddb_regs, watchpt);
> - db_is_active = 0;
>  }
> diff --git a/sys/ddb/db_var.h b/sys/ddb/db_var.h
> index 3bb02b5d34d..f264aaa6c7f 100644
> --- a/sys/ddb/db_var.h
> +++ b/sys/ddb/db_var.h
> @@ -67,7 +67,6 @@ extern int db_max_line;
>  extern int db_panic;
>  extern int db_console;
>  extern int db_log;
> -extern int db_is_active;
>  extern int db_profile;
>  
>  int ddb_sysctl(int *, u_int, void *, size_t *, void *, size_t,
> diff --git a/sys/dev/pci/drm/include/linux/kernel.h b/sys/dev/pci/drm/include/linux/kernel.h
> index 188efad2f4f..d0b274c88c8 100644
> --- a/sys/dev/pci/drm/include/linux/kernel.h
> +++ b/sys/dev/pci/drm/include/linux/kernel.h
> @@ -9,8 +9,6 @@
>  #include <sys/stdarg.h>
>  #include <sys/malloc.h>
>  
> -#include <ddb/db_var.h>
> -
>  #include <linux/types.h>
>  #include <linux/compiler.h>
>  #include <linux/bitops.h>
> @@ -119,7 +117,7 @@ static inline int
>  _in_dbg_master(void)
>  {
>  #ifdef DDB
> - return (db_is_active);
> + return (db_active);
>  #endif
>   return (0);
>  }
> diff --git a/sys/dev/pci/drm/include/linux/kgdb.h b/sys/dev/pci/drm/include/linux/kgdb.h
> index 73759b3be75..874a0ebe0be 100644
> --- a/sys/dev/pci/drm/include/linux/kgdb.h
> +++ b/sys/dev/pci/drm/include/linux/kgdb.h
> @@ -3,13 +3,13 @@
>  #ifndef _LINUX_KGDB_H
>  #define _LINUX_KGDB_H
>  
> -#include <ddb/db_var.h>
> +#include <sys/systm.h>
>  
>  static inline int
>  in_dbg_master(void)
>  {
>  #ifdef DDB
> - return (db_is_active);
> + return (db_active);
>  #endif
>   return (0);
>  }
> diff --git a/sys/kern/subr_prf.c b/sys/kern/subr_prf.c
> index 6abac53452a..87bb23d1ded 100644
> --- a/sys/kern/subr_prf.c
> +++ b/sys/kern/subr_prf.c
> @@ -118,11 +118,6 @@ int db_console = 1;
>  #else
>  int db_console = 0;
>  #endif
> -
> -/*
> - * flag to indicate if we are currently in ddb (on some processor)
> - */
> -int db_is_active;
>  #endif
>  
>  /*
> @@ -330,16 +325,11 @@ void
>  kputchar(int c, int flags, struct tty *tp)
>  {
>   extern int msgbufmapped;
> - int ddb_active = 0;
> -
> -#ifdef DDB
> - ddb_active = db_is_active;
> -#endif
>  
>   if (panicstr)
>   constty = NULL;
>  
> - if ((flags & TOCONS) && tp == NULL && constty && !ddb_active) {
> + if ((flags & TOCONS) && tp == NULL && constty && !db_active) {
>   tp = constty;
>   flags |= TOTTY;
>   }
> @@ -349,7 +339,7 @@ kputchar(int c, int flags, struct tty *tp)
>   if ((flags & TOLOG) &&
>      c != '\0' && c != '\r' && c != 0177 && msgbufmapped)
>   msgbuf_putchar(msgbufp, c);
> - if ((flags & TOCONS) && (constty == NULL || ddb_active) && c != '\0')
> + if ((flags & TOCONS) && (constty == NULL || db_active) && c != '\0')
>   (*v_putc)(c);
>  #ifdef DDB
>   if (flags & TODDB)
> diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
> index 6a45ccd3735..5c6ecfe62de 100644
> --- a/sys/kern/subr_witness.c
> +++ b/sys/kern/subr_witness.c
> @@ -2051,7 +2051,7 @@ witness_ddb_list(struct proc *p)
>   struct witness_cpu *wc = &witness_cpu[cpu_number()];
>  
>   KASSERTMSG(witness_cold == 0, "%s: witness_cold", __func__);
> - KASSERTMSG(db_is_active, "%s: not in the debugger", __func__);
> + KASSERTMSG(db_active, "%s: not in the debugger", __func__);
>  
>   if (witness_watch < 1)
>   return;
> --
> 2.22.0
>

Reply | Threaded
Open this post in threaded view
|

Re: Nuke db_is_active in favor of db_active

Martin Pieuchot
On 08/07/19(Mon) 15:00, Martin Pieuchot wrote:

> On 26/06/19(Wed) 19:13, Christian Ludwig wrote:
> > We have two variables with the same meaning. db_active is used in way
> > more places, so let's nuke db_is_active.
>
> Thanks for the cleanup!
>
> > Now that db_active is in <sys/systm.h> for a while already and not
> > guarded by DDB anymore, take the opportunity to clean up some places
> > that use it.
>
> At least m88k and powerpc do not set db_active before calling db_trap().
>
> This needs to be fixed first :)

Updated diff below includes that, any ok?

Index: arch/m88k/m88k/db_interface.c
===================================================================
RCS file: /cvs/src/sys/arch/m88k/m88k/db_interface.c,v
retrieving revision 1.22
diff -u -p -r1.22 db_interface.c
--- arch/m88k/m88k/db_interface.c 30 Apr 2017 16:45:45 -0000 1.22
+++ arch/m88k/m88k/db_interface.c 17 Jul 2019 00:07:16 -0000
@@ -405,9 +405,11 @@ m88k_db_trap(type, frame)
 
  ddb_regs = frame->tf_regs;
 
+ db_active++;
  cnpollc(TRUE);
  db_trap(type, 0);
  cnpollc(FALSE);
+ db_active--;
 
  frame->tf_regs = ddb_regs;
 
Index: arch/macppc/dev/zs.c
===================================================================
RCS file: /cvs/src/sys/arch/macppc/dev/zs.c,v
retrieving revision 1.28
diff -u -p -r1.28 zs.c
--- arch/macppc/dev/zs.c 30 Dec 2017 20:46:59 -0000 1.28
+++ arch/macppc/dev/zs.c 17 Jul 2019 00:04:40 -0000
@@ -1091,12 +1091,8 @@ zs_abort(struct zs_chanstate *channel)
  } while (rr0 & ZSRR0_BREAK);
 
 #if defined(DDB)
- {
- extern int db_active;
-
- if (!db_active)
- db_enter();
- }
+ if (!db_active)
+ db_enter();
 #endif
 }
 
Index: arch/powerpc/powerpc/trap.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc/powerpc/trap.c,v
retrieving revision 1.109
diff -u -p -r1.109 trap.c
--- arch/powerpc/powerpc/trap.c 9 Jul 2019 23:48:08 -0000 1.109
+++ arch/powerpc/powerpc/trap.c 17 Jul 2019 00:06:52 -0000
@@ -569,9 +569,11 @@ for (i = 0; i < errnum; i++) {
  /* should check for correct byte here or panic */
 #ifdef DDB
  db_save_regs(frame);
+ db_active++;
  cnpollc(TRUE);
  db_trap(T_BREAKPOINT, 0);
  cnpollc(FALSE);
+ db_active--;
 #else
  panic("trap EXC_PGM");
 #endif
Index: arch/sparc64/sparc64/ipifuncs.c
===================================================================
RCS file: /cvs/src/sys/arch/sparc64/sparc64/ipifuncs.c,v
retrieving revision 1.17
diff -u -p -r1.17 ipifuncs.c
--- arch/sparc64/sparc64/ipifuncs.c 3 Dec 2017 10:55:50 -0000 1.17
+++ arch/sparc64/sparc64/ipifuncs.c 17 Jul 2019 00:04:40 -0000
@@ -39,8 +39,6 @@
 #include <machine/pmap.h>
 #include <machine/sparc64.h>
 
-extern int db_active;
-
 #define SPARC64_IPI_RETRIES 10000
 
 #define sparc64_ipi_sleep() delay(1000)
Index: ddb/db_trap.c
===================================================================
RCS file: /cvs/src/sys/ddb/db_trap.c,v
retrieving revision 1.28
diff -u -p -r1.28 db_trap.c
--- ddb/db_trap.c 9 Jan 2017 17:58:44 -0000 1.28
+++ ddb/db_trap.c 17 Jul 2019 00:04:40 -0000
@@ -52,7 +52,6 @@ db_trap(int type, int code)
  boolean_t bkpt;
  boolean_t watchpt;
 
- db_is_active = 1;
  bkpt = IS_BREAKPOINT_TRAP(type, code);
  watchpt = IS_WATCHPOINT_TRAP(type, code);
 
@@ -94,5 +93,4 @@ db_trap(int type, int code)
  }
 
  db_restart_at_pc(&ddb_regs, watchpt);
- db_is_active = 0;
 }
Index: ddb/db_var.h
===================================================================
RCS file: /cvs/src/sys/ddb/db_var.h,v
retrieving revision 1.12
diff -u -p -r1.12 db_var.h
--- ddb/db_var.h 4 Sep 2016 09:22:29 -0000 1.12
+++ ddb/db_var.h 17 Jul 2019 00:04:40 -0000
@@ -67,7 +67,6 @@ extern int db_max_line;
 extern int db_panic;
 extern int db_console;
 extern int db_log;
-extern int db_is_active;
 extern int db_profile;
 
 int ddb_sysctl(int *, u_int, void *, size_t *, void *, size_t,
Index: dev/pci/drm/include/linux/kernel.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/include/linux/kernel.h,v
retrieving revision 1.1
diff -u -p -r1.1 kernel.h
--- dev/pci/drm/include/linux/kernel.h 14 Apr 2019 10:14:53 -0000 1.1
+++ dev/pci/drm/include/linux/kernel.h 17 Jul 2019 00:04:40 -0000
@@ -9,8 +9,6 @@
 #include <sys/stdarg.h>
 #include <sys/malloc.h>
 
-#include <ddb/db_var.h>
-
 #include <linux/types.h>
 #include <linux/compiler.h>
 #include <linux/bitops.h>
@@ -119,7 +117,7 @@ static inline int
 _in_dbg_master(void)
 {
 #ifdef DDB
- return (db_is_active);
+ return (db_active);
 #endif
  return (0);
 }
Index: dev/pci/drm/include/linux/kgdb.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/drm/include/linux/kgdb.h,v
retrieving revision 1.1
diff -u -p -r1.1 kgdb.h
--- dev/pci/drm/include/linux/kgdb.h 14 Apr 2019 10:14:53 -0000 1.1
+++ dev/pci/drm/include/linux/kgdb.h 17 Jul 2019 00:04:40 -0000
@@ -3,13 +3,13 @@
 #ifndef _LINUX_KGDB_H
 #define _LINUX_KGDB_H
 
-#include <ddb/db_var.h>
+#include <sys/systm.h>
 
 static inline int
 in_dbg_master(void)
 {
 #ifdef DDB
- return (db_is_active);
+ return (db_active);
 #endif
  return (0);
 }
Index: kern/subr_prf.c
===================================================================
RCS file: /cvs/src/sys/kern/subr_prf.c,v
retrieving revision 1.98
diff -u -p -r1.98 subr_prf.c
--- kern/subr_prf.c 8 May 2019 16:22:42 -0000 1.98
+++ kern/subr_prf.c 17 Jul 2019 00:11:15 -0000
@@ -118,11 +118,6 @@ int db_console = 1;
 #else
 int db_console = 0;
 #endif
-
-/*
- * flag to indicate if we are currently in ddb (on some processor)
- */
-int db_is_active;
 #endif
 
 /*
@@ -330,16 +325,11 @@ void
 kputchar(int c, int flags, struct tty *tp)
 {
  extern int msgbufmapped;
- int ddb_active = 0;
-
-#ifdef DDB
- ddb_active = db_is_active;
-#endif
 
  if (panicstr)
  constty = NULL;
 
- if ((flags & TOCONS) && tp == NULL && constty && !ddb_active) {
+ if ((flags & TOCONS) && tp == NULL && constty != NULL && !db_active) {
  tp = constty;
  flags |= TOTTY;
  }
@@ -349,7 +339,7 @@ kputchar(int c, int flags, struct tty *t
  if ((flags & TOLOG) &&
     c != '\0' && c != '\r' && c != 0177 && msgbufmapped)
  msgbuf_putchar(msgbufp, c);
- if ((flags & TOCONS) && (constty == NULL || ddb_active) && c != '\0')
+ if ((flags & TOCONS) && (constty == NULL || db_active) && c != '\0')
  (*v_putc)(c);
 #ifdef DDB
  if (flags & TODDB)
Index: kern/subr_witness.c
===================================================================
RCS file: /cvs/src/sys/kern/subr_witness.c,v
retrieving revision 1.33
diff -u -p -r1.33 subr_witness.c
--- kern/subr_witness.c 6 Jun 2019 16:45:10 -0000 1.33
+++ kern/subr_witness.c 17 Jul 2019 00:04:40 -0000
@@ -2051,7 +2051,7 @@ witness_ddb_list(struct proc *p)
  struct witness_cpu *wc = &witness_cpu[cpu_number()];
 
  KASSERTMSG(witness_cold == 0, "%s: witness_cold", __func__);
- KASSERTMSG(db_is_active, "%s: not in the debugger", __func__);
+ KASSERTMSG(db_active, "%s: not in the debugger", __func__);
 
  if (witness_watch < 1)
  return;