Re: [PATCH] kgdb: fix gcc-11 warning on indentation

[Jason Wessel]

On 3/22/21 2:22 PM, Doug Anderson wrote:

Hi,

On Mon, Mar 22, 2021 at 11:19 AM Arnd Bergmann  wrote:


On Mon, Mar 22, 2021 at 6:07 PM Doug Anderson  wrote:

On Mon, Mar 22, 2021 at 9:43 AM Arnd Bergmann  wrote:


-#define v1printk(a...) do { \
-   if (verbose) \
-   printk(KERN_INFO a); \
-   } while (0)
-#define v2printk(a...) do { \
-   if (verbose > 1) \
-   printk(KERN_INFO a); \
-   touch_nmi_watchdog();   \
-   } while (0)
-#define eprintk(a...) do { \
-   printk(KERN_ERR a); \
-   WARN_ON(1); \
-   } while (0)
+#define v1printk(a...) do {\


nit: In addition to the indentation change you're also lining up the
backslashes. Is that just personal preference, or is there some
official recommendation in the kernel? I don't really have a strong
opinion either way (IMO each style has its advantages).


I don't think there is an official recommendation, I just think the
style is more common, and it helped my figure out what the
indentation should look like in this case.


OK, makes sense. I just wasn't sure if there was some standard that I
wasn't aware of. Given that you have to touch all these lines anyway
then making them all pretty like this seems fine to me.



+   if (verbose)\
+   printk(KERN_INFO a);\
+} while (0)
+#define v2printk(a...) do {\
+   if (verbose > 1)\
+   printk(KERN_INFO a);\
+   touch_nmi_watchdog();   \


This touch_nmi_watchdog() is pretty wonky. I guess maybe the
assumption is that the "verbose level 2" prints are so chatty that the
printing might prevent us from touching the NMI watchdog in the way
that we normally do and thus we need an extra one here?

...but, in that case, I think the old code was _wrong_ and that the
intention was that the touch_nmi_watchdog() should only be if "verose

1" as the indentation implied. There doesn't feel like a reason to

touch the watchdog if we're not doing anything slow.


No idea. It was like this in Jason's original version from 2008.


Yeah, I noticed the same. I'd be curious what Daneil (or Jason if he's
reading) says. I suppose i could always wait until your patch lands
and then send a new patch that puts it inside the "if" statement and
we can debate it then.




The original board this was developed with was a 32bit eeepc.

The intent was that when v2printk() was called for a verbose > 1
condition the touch_nmi_watchdog() was called.   The test case
where a whole lot of single steps are executed sequentially was not
letting the watchdog get reset by the normal kernel routine.
The serial port was so slow it was pretty easy to hit this problem
and it would just power cycle itself.

The original intent would have bee:

#define v2printk(a...) do { \
if (verbose > 1) {   \
printk(KERN_INFO a);\
touch_nmi_watchdog();   \
}   \
} while (0)


I'd guess this probably not the first time gcc-11 is finding brace
imbalances.

Cheers,
Jason.

Re: [PATCH] kgdb: Remove kgdb_schedule_breakpoint()

[Jason Wessel]

On 2/10/21 8:25 AM, Daniel Thompson wrote:

To the very best of my knowledge there has never been any in-tree
code that calls this function. It exists largely to support an
out-of-tree driver that provides kgdb-over-ethernet using the
netpoll API.



There was another out of tree user of this, but I don't know if
this is still applicable today.  The scenario is around the
ability to use a character sequence when kgdboc is active
on the console such as , to cause a break point, vs
using a hardware break over a tty (because not all hardware supported this).

I could send the original patch that implements this along, but
I question if it is needed given the devices out there.  The reason
the original patch existed at all was to deal with some pick serial
hardware.

 original out of tree patch header 

Subject: [PATCH] kgdboc, tty: Add the rx polling call back capability

The idea is to allow kgdboc to intercept a  or any other
character of preference to cause breakpoint interrupt which will start
the kgdb interface running on the controlling terminal where the
character was typed.

The default behavior of kgdboc changes such that the control-c will
always generate an entry to kgdb unless the "n" option is used in the
kgdb configuration line. IE: kgdboc=ttyS0,n,115200

In order to make use of the new API, a low level serial driver must
check to see if it should execute the callback function for each
character that it processes.  This is similar to the approach used
with the NET_POLL API's rx_hook.

The only changes to the tty layer introduced by this patch are:
  * Add poll_rx_cb() call back for the low level driver
  * Move the poll_init() into kgdboc and out of tty_find_polling_driv()
  * change poll_init() to accept the rx callback parameter
---
 Documentation/DocBook/kgdb.tmpl  |   46 ++---
 drivers/tty/serial/kgdboc.c  |   70 ++-
 drivers/tty/serial/serial_core.c |   23 
 drivers/tty/tty_io.c |9 +
 include/linux/serial_core.h  |3 +
 include/linux/tty_driver.h   |3 +
 6 files changed, 139 insertions(+), 15 deletions(-)


---





kgdboe has been out-of-tree for more than 10 years and I don't
recall any serious attempt to upstream it at any point in the last
five. At this stage it looks better to stop carrying this code in
the kernel and integrate the code into the out-of-tree driver
instead.


Because it has no in tree users, it absolutely makes the most
sense to purge this function.

Acked-by: Jason Wessel 



The long term trajectory for the kernel looks likely to include
effort to remove or reduce the use of tasklets (something that has
also been true for the last 10 years). Thus the main real reason
for this patch is to make explicit that the in-tree kgdb features
do not require tasklets.

Signed-off-by: Daniel Thompson 
---

Notes:
 During this cycle two developers have proposed tidying up the
 DECLARE_TASKLET_OLD() in the debug core. Both threads ended with a
 suggestion to remove kgdb_schedule_breakpoint() but I don't recall
 seeing a follow up patch for either thread... so I wrote it myself.

  include/linux/kgdb.h  |  1 -
  kernel/debug/debug_core.c | 26 --
  2 files changed, 27 deletions(-)

diff --git a/include/linux/kgdb.h b/include/linux/kgdb.h
index 0d6cf64c8bb12..0444b44bd156d 100644
--- a/include/linux/kgdb.h
+++ b/include/linux/kgdb.h
@@ -325,7 +325,6 @@ extern char *kgdb_mem2hex(char *mem, char *buf, int count);
  extern int kgdb_hex2mem(char *buf, char *mem, int count);

  extern int kgdb_isremovedbreak(unsigned long addr);
-extern void kgdb_schedule_breakpoint(void);
  extern int kgdb_has_hit_break(unsigned long addr);

  extern int
diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 7f22c1c0ffe80..b636d517c02c4 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -119,7 +119,6 @@ static DEFINE_RAW_SPINLOCK(dbg_slave_lock);
   */
  static atomic_t   masters_in_kgdb;
  static atomic_t   slaves_in_kgdb;
-static atomic_tkgdb_break_tasklet_var;
  atomic_t  kgdb_setting_breakpoint;

  struct task_struct*kgdb_usethread;
@@ -1084,31 +1083,6 @@ static void kgdb_unregister_callbacks(void)
}
  }

-/*
- * There are times a tasklet needs to be used vs a compiled in
- * break point so as to cause an exception outside a kgdb I/O module,
- * such as is the case with kgdboe, where calling a breakpoint in the
- * I/O driver itself would be fatal.
- */
-static void kgdb_tasklet_bpt(unsigned long ing)
-{
-   kgdb_breakpoint();
-   atomic_set(_break_tasklet_var, 0);
-}
-
-static DECLARE_TASKLET_OLD(kgdb_tasklet_breakpoint, kgdb_tasklet_bpt);
-
-void kgdb_schedule_breakpoint(void)
-{
-   if (atomic_read(_break_t

Re: [PATCH] gdbstub: mark expected switch fall-throughs

[Jason Wessel]

We'll have to fix them at some point.

Acked-by: Jason Wessel 


Cheers,
Jason.

On 2/26/19 1:16 PM, Gustavo A. R. Silva wrote:

In preparation to enabling -Wimplicit-fallthrough, mark switch
cases where we are expecting to fall through.

This patch fixes the following warnings:

kernel/debug/gdbstub.c: In function ‘gdb_serial_stub’:
kernel/debug/gdbstub.c:1031:7: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 if (remcom_in_buffer[1] == '\0') {
^
kernel/debug/gdbstub.c:1036:3: note: here
case 'C': /* Exception passing */
^~~~
kernel/debug/gdbstub.c:1040:7: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 if (tmp == 0)
^
kernel/debug/gdbstub.c:1043:3: note: here
case 'c': /* Continue packet */
^~~~
kernel/debug/gdbstub.c:1050:4: warning: this statement may fall through 
[-Wimplicit-fallthrough=]
 dbg_activate_sw_breakpoints();
 ^
kernel/debug/gdbstub.c:1052:3: note: here
default:
^~~

Warning level 3 was used: -Wimplicit-fallthrough=3

Notice that, in this particular case, the code comment is modified
in accordance with what GCC is expecting to find.

This patch is part of the ongoing efforts to enable
-Wimplicit-fallthrough.

Signed-off-by: Gustavo A. R. Silva 
---
  kernel/debug/gdbstub.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index 7510dc687c0d..9f267b8905b4 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -1033,13 +1033,14 @@ int gdb_serial_stub(struct kgdb_state *ks)
return DBG_PASS_EVENT;
}
  #endif
+   /* Fall through */
case 'C': /* Exception passing */
tmp = gdb_cmd_exception_pass(ks);
if (tmp > 0)
goto default_handle;
if (tmp == 0)
break;
-   /* Fall through on tmp < 0 */
+   /* Fall through - on tmp < 0 */
case 'c': /* Continue packet */
case 's': /* Single step packet */
if (kgdb_contthread && kgdb_contthread != current) {
@@ -1048,7 +1049,7 @@ int gdb_serial_stub(struct kgdb_state *ks)
break;
}
dbg_activate_sw_breakpoints();
-   /* Fall through to default processing */
+   /* Fall through - to default processing */
default:
  default_handle:
error = kgdb_arch_handle_exception(ks->ex_vector,

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Jason Wessel]

On 09/28/2018 07:57 AM, Michael Ellerman wrote:

Christophe LEROY  writes:

Le 27/09/2018 à 13:09, Michael Ellerman a écrit :

Christophe LEROY  writes:

Le 26/09/2018 à 13:11, Daniel Thompson a écrit :

The Fixes: and now your Reviewed-by: appear automatically in patchwork
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
so I believe they'll be automatically included when Jason or someone
else takes the patch, no ?


patchwork won't add the Fixes tag from the reply, it needs to be in the
original mail.

See:
https://github.com/getpatchwork/patchwork/issues/151



Ok, so it accounts it and adds a '1' in the F column in the patches
list, but won't take it into account.


Yes. The logic that populates the columns is separate from the logic
that scrapes the tags, which is a bug :)


Then I'll send a v2 with revised commit text.





No need.  
https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next

Since it is a regression fix, we'll try and get it merged as soon as we can.

Cheers,
Jason.

Re: [PATCH] kdb: use correct pointer when 'btc' calls 'btt'

[Jason Wessel]

On 09/28/2018 07:57 AM, Michael Ellerman wrote:

Christophe LEROY  writes:

Le 27/09/2018 à 13:09, Michael Ellerman a écrit :

Christophe LEROY  writes:

Le 26/09/2018 à 13:11, Daniel Thompson a écrit :

The Fixes: and now your Reviewed-by: appear automatically in patchwork
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=65715),
so I believe they'll be automatically included when Jason or someone
else takes the patch, no ?


patchwork won't add the Fixes tag from the reply, it needs to be in the
original mail.

See:
https://github.com/getpatchwork/patchwork/issues/151



Ok, so it accounts it and adds a '1' in the F column in the patches
list, but won't take it into account.


Yes. The logic that populates the columns is separate from the logic
that scrapes the tags, which is a bug :)


Then I'll send a v2 with revised commit text.





No need.  
https://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git/commit/?h=kgdb-next

Since it is a regression fix, we'll try and get it merged as soon as we can.

Cheers,
Jason.

[GIT PULL] KGDB/KDB fixes for 4.16

[Jason Wessel]

Linus,

Please pull the kgdb tree.

Summary of changes:
   * Fix 2032 time access issues and new compiler warnings
   * minor regression test cleanup
   * formatting fixes for end user use of kdb

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.16

Thanks,
Jason.

---
The following changes since commit 5b7d27967dabfb17c21b0d98b29153b9e3ee71e5:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-01-24 
17:24:30 -0800)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.16

for you to fetch changes up to 2cf2f0d5b91fd1b06a6ae260462fc7945ea84add:

  kdb: use memmove instead of overlapping memcpy (2018-02-04 21:29:53 -0600)


   * Fix 2032 time access issues and new compiler warnings
   * minor regression test cleanup
   * formatting fixes for end user use of kdb


Arnd Bergmann (2):
  kdb: use __ktime_get_real_seconds instead of __current_kernel_time
  kdb: use memmove instead of overlapping memcpy

Baolin Wang (1):
  kdb: use ktime_get_mono_fast_ns() instead of ktime_get_ts()

Daniel Thompson (1):
  misc: kgdbts: Display progress of asynchronous tests

Randy Dunlap (3):
  kdb: make "mdr" command repeat
  kdb: drop newline in unknown command output
  kdb: bl: don't use tab character in output

 drivers/misc/kgdbts.c  |  8 +++-
 include/linux/timekeeping.h|  1 +
 kernel/debug/kdb/kdb_bp.c  |  4 +-
 kernel/debug/kdb/kdb_main.c| 89 +-
 kernel/debug/kdb/kdb_support.c |  4 +-
 kernel/time/timekeeping_internal.h |  2 -
 6 files changed, 51 insertions(+), 57 deletions(-)

[GIT PULL] KGDB/KDB fixes for 4.16

[Jason Wessel]

Linus,

Please pull the kgdb tree.

Summary of changes:
   * Fix 2032 time access issues and new compiler warnings
   * minor regression test cleanup
   * formatting fixes for end user use of kdb

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.16

Thanks,
Jason.

---
The following changes since commit 5b7d27967dabfb17c21b0d98b29153b9e3ee71e5:

  Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net (2018-01-24 
17:24:30 -0800)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.16

for you to fetch changes up to 2cf2f0d5b91fd1b06a6ae260462fc7945ea84add:

  kdb: use memmove instead of overlapping memcpy (2018-02-04 21:29:53 -0600)


   * Fix 2032 time access issues and new compiler warnings
   * minor regression test cleanup
   * formatting fixes for end user use of kdb


Arnd Bergmann (2):
  kdb: use __ktime_get_real_seconds instead of __current_kernel_time
  kdb: use memmove instead of overlapping memcpy

Baolin Wang (1):
  kdb: use ktime_get_mono_fast_ns() instead of ktime_get_ts()

Daniel Thompson (1):
  misc: kgdbts: Display progress of asynchronous tests

Randy Dunlap (3):
  kdb: make "mdr" command repeat
  kdb: drop newline in unknown command output
  kdb: bl: don't use tab character in output

 drivers/misc/kgdbts.c  |  8 +++-
 include/linux/timekeeping.h|  1 +
 kernel/debug/kdb/kdb_bp.c  |  4 +-
 kernel/debug/kdb/kdb_main.c| 89 +-
 kernel/debug/kdb/kdb_support.c |  4 +-
 kernel/time/timekeeping_internal.h |  2 -
 6 files changed, 51 insertions(+), 57 deletions(-)

Re: [PATCH] kdb: use memmove instead of overlapping memcpy

[Jason Wessel]

On 02/02/2018 08:59 AM, Arnd Bergmann wrote:

gcc discovered that the memcpy() arguments in kdbnearsym() overlap, so
we should really use memmove(), which is defined to handle that correctly:

In function 'memcpy',
 inlined from 'kdbnearsym' at 
/git/arm-soc/kernel/debug/kdb/kdb_support.c:132:4:
/git/arm-soc/include/linux/string.h:353:9: error: '__builtin_memcpy' accessing 
792 bytes at offsets 0 and 8 overlaps 784 bytes at offset 8 [-Werror=restrict]
   return __builtin_memcpy(p, q, size);

Signed-off-by: Arnd Bergmann <a...@arndb.de>
---
  kernel/debug/kdb/kdb_support.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 5d8ef3a07ecd..1ad4370ccbf0 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -129,13 +129,13 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
}
if (i >= ARRAY_SIZE(kdb_name_table)) {
debug_kfree(kdb_name_table[0]);
-   memcpy(kdb_name_table, kdb_name_table+1,
+   memmove(kdb_name_table, kdb_name_table+1,
   sizeof(kdb_name_table[0]) *
   (ARRAY_SIZE(kdb_name_table)-1));
} else {
debug_kfree(knt1);
knt1 = kdb_name_table[i];
-   memcpy(kdb_name_table+i, kdb_name_table+i+1,
+   memmove(kdb_name_table+i, kdb_name_table+i+1,
   sizeof(kdb_name_table[0]) *
   (ARRAY_SIZE(kdb_name_table)-i-1));
}




That is good by me.   Many thanks!  Added to queue.

Reviewed-by: Jason Wessel <jason.wes...@windriver.com>

Jason.

Re: [PATCH] kdb: use memmove instead of overlapping memcpy

[Jason Wessel]

On 02/02/2018 08:59 AM, Arnd Bergmann wrote:

gcc discovered that the memcpy() arguments in kdbnearsym() overlap, so
we should really use memmove(), which is defined to handle that correctly:

In function 'memcpy',
 inlined from 'kdbnearsym' at 
/git/arm-soc/kernel/debug/kdb/kdb_support.c:132:4:
/git/arm-soc/include/linux/string.h:353:9: error: '__builtin_memcpy' accessing 
792 bytes at offsets 0 and 8 overlaps 784 bytes at offset 8 [-Werror=restrict]
   return __builtin_memcpy(p, q, size);

Signed-off-by: Arnd Bergmann 
---
  kernel/debug/kdb/kdb_support.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/debug/kdb/kdb_support.c b/kernel/debug/kdb/kdb_support.c
index 5d8ef3a07ecd..1ad4370ccbf0 100644
--- a/kernel/debug/kdb/kdb_support.c
+++ b/kernel/debug/kdb/kdb_support.c
@@ -129,13 +129,13 @@ int kdbnearsym(unsigned long addr, kdb_symtab_t *symtab)
}
if (i >= ARRAY_SIZE(kdb_name_table)) {
debug_kfree(kdb_name_table[0]);
-   memcpy(kdb_name_table, kdb_name_table+1,
+   memmove(kdb_name_table, kdb_name_table+1,
   sizeof(kdb_name_table[0]) *
   (ARRAY_SIZE(kdb_name_table)-1));
} else {
debug_kfree(knt1);
knt1 = kdb_name_table[i];
-   memcpy(kdb_name_table+i, kdb_name_table+i+1,
+   memmove(kdb_name_table+i, kdb_name_table+i+1,
   sizeof(kdb_name_table[0]) *
   (ARRAY_SIZE(kdb_name_table)-i-1));
}




That is good by me.   Many thanks!  Added to queue.

Reviewed-by: Jason Wessel 

Jason.

Re: [PATCH v2] kdb: use ktime_get_mono_fast_ns() instead of ktime_get_ts()

[Jason Wessel]

On 01/30/2018 07:35 AM, Arnd Bergmann wrote:

On Mon, Jan 29, 2018 at 3:22 AM, Baolin Wang  wrote:

The kdb code will print the monotonic time by ktime_get_ts(), but
the ktime_get_ts() will be protected by a sequence lock, that will
introduce one deadlock risk if the lock was already held in the
context from which we entered the debugger.

Thus we can use the ktime_get_mono_fast_ns() to get the monotonic
time, which is NMI safe access to clock monotonic. Moreover we can
remove the 'struct timespec', which is not y2038 safe.

Signed-off-by: Baolin Wang 


Reviewed-by: Arnd Bergmann 




I'll add this in today, run some tests and it should be in the merge window.

Thanks,
Jason

Re: [PATCH v2] kdb: use ktime_get_mono_fast_ns() instead of ktime_get_ts()

[Jason Wessel]

On 01/30/2018 07:35 AM, Arnd Bergmann wrote:

On Mon, Jan 29, 2018 at 3:22 AM, Baolin Wang  wrote:

The kdb code will print the monotonic time by ktime_get_ts(), but
the ktime_get_ts() will be protected by a sequence lock, that will
introduce one deadlock risk if the lock was already held in the
context from which we entered the debugger.

Thus we can use the ktime_get_mono_fast_ns() to get the monotonic
time, which is NMI safe access to clock monotonic. Moreover we can
remove the 'struct timespec', which is not y2038 safe.

Signed-off-by: Baolin Wang 


Reviewed-by: Arnd Bergmann 




I'll add this in today, run some tests and it should be in the merge window.

Thanks,
Jason

Re: [PATCH] kdb: use ktime_get_seconds() instead of ktime_get_ts()

[Jason Wessel]

On 01/25/2018 09:03 PM, Baolin Wang wrote:

The kdb code will print the monotonic time by ktime_get_ts(), but
the ktime_get_ts() will be protected by a sequence lock, that will
introduce one deadlock risk if the lock was already held in the
context from which we entered the debugger.

Since kdb is only interested in the second field, we can use the
ktime_get_seconds() to get the monotonic time without a lock,
moreover we can remove the 'struct timespec', which is not y2038
safe.

Signed-off-by: Baolin Wang <baolin.w...@linaro.org>


Acked-by: Jason Wessel <jason.wes...@windriver.com>


Thanks.   Added to the kgdb-next branch for the next merge cycle.

Jason.


---
  kernel/debug/kdb/kdb_main.c |4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 69e70f4..f0fc6f7 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2486,10 +2486,8 @@ static int kdb_kill(int argc, const char **argv)
   */
  static void kdb_sysinfo(struct sysinfo *val)
  {
-   struct timespec uptime;
-   ktime_get_ts();
memset(val, 0, sizeof(*val));
-   val->uptime = uptime.tv_sec;
+   val->uptime = ktime_get_seconds();
val->loads[0] = avenrun[0];
val->loads[1] = avenrun[1];
val->loads[2] = avenrun[2];

Re: [PATCH] kdb: use ktime_get_seconds() instead of ktime_get_ts()

[Jason Wessel]

On 01/25/2018 09:03 PM, Baolin Wang wrote:

The kdb code will print the monotonic time by ktime_get_ts(), but
the ktime_get_ts() will be protected by a sequence lock, that will
introduce one deadlock risk if the lock was already held in the
context from which we entered the debugger.

Since kdb is only interested in the second field, we can use the
ktime_get_seconds() to get the monotonic time without a lock,
moreover we can remove the 'struct timespec', which is not y2038
safe.

Signed-off-by: Baolin Wang 


Acked-by: Jason Wessel 


Thanks.   Added to the kgdb-next branch for the next merge cycle.

Jason.


---
  kernel/debug/kdb/kdb_main.c |4 +---
  1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 69e70f4..f0fc6f7 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2486,10 +2486,8 @@ static int kdb_kill(int argc, const char **argv)
   */
  static void kdb_sysinfo(struct sysinfo *val)
  {
-   struct timespec uptime;
-   ktime_get_ts();
memset(val, 0, sizeof(*val));
-   val->uptime = uptime.tv_sec;
+   val->uptime = ktime_get_seconds();
val->loads[0] = avenrun[0];
val->loads[1] = avenrun[1];
val->loads[2] = avenrun[2];

Re: [PATCH] kdb: Change timespec to use timespec64

[Jason Wessel]

On 01/25/2018 05:38 AM, Daniel Thompson wrote:

On Thu, Jan 25, 2018 at 05:18:54PM +0800, Baolin Wang wrote:

On 25 January 2018 at 16:55, Arnd Bergmann  wrote:

On Thu, Jan 25, 2018 at 9:05 AM, Baolin Wang  wrote:

@@ -2554,7 +2554,7 @@ static int kdb_summary(int argc, const char **argv)
 kdb_printf("domainname %s\n", init_uts_ns.name.domainname);
 kdb_printf("ccversion  %s\n", __stringify(CCVERSION));

-   now = __current_kernel_time();
+   now = current_kernel_time64();
 kdb_gmtime(, );
 kdb_printf("date   %04d-%02d-%02d %02d:%02d:%02d "
"tz_minuteswest %d\n",


Thanks for picking this one up again, we should find a permanent solution here.
Unfortunately you patch is incorrect, as we cannot safely call
current_kernel_time64()
from NMI context.


Ah, thanks for pointing out the issue, since I do not know what
context the function will be called in kdb.



The __ prefix on __current_kernel_time() indicates that this is a special call
that intentionally doesn't read the hardware time to avoid taking locks that
might already be held in the context from which we entered the debugger.

See https://patchwork.kernel.org/patch/10002097/ for my earlier patch.


This patch had not been merged into mainline?


Not yet (and I'm afraid it's not in kgdb-next either) but the ack from Jason is 
from
this kernel cycle so we'll see what can be done!




I thought for what ever reason this was going through the time keeper subtree.  
 I added it immediately to kgdb-next so it will be evaluated in the linux-next 
tree in the next day or so, and we can get this merged in the merge window.

Thanks,
Jason.

Re: [PATCH] kdb: Change timespec to use timespec64

[Jason Wessel]

On 01/25/2018 05:38 AM, Daniel Thompson wrote:

On Thu, Jan 25, 2018 at 05:18:54PM +0800, Baolin Wang wrote:

On 25 January 2018 at 16:55, Arnd Bergmann  wrote:

On Thu, Jan 25, 2018 at 9:05 AM, Baolin Wang  wrote:

@@ -2554,7 +2554,7 @@ static int kdb_summary(int argc, const char **argv)
 kdb_printf("domainname %s\n", init_uts_ns.name.domainname);
 kdb_printf("ccversion  %s\n", __stringify(CCVERSION));

-   now = __current_kernel_time();
+   now = current_kernel_time64();
 kdb_gmtime(, );
 kdb_printf("date   %04d-%02d-%02d %02d:%02d:%02d "
"tz_minuteswest %d\n",


Thanks for picking this one up again, we should find a permanent solution here.
Unfortunately you patch is incorrect, as we cannot safely call
current_kernel_time64()
from NMI context.


Ah, thanks for pointing out the issue, since I do not know what
context the function will be called in kdb.



The __ prefix on __current_kernel_time() indicates that this is a special call
that intentionally doesn't read the hardware time to avoid taking locks that
might already be held in the context from which we entered the debugger.

See https://patchwork.kernel.org/patch/10002097/ for my earlier patch.


This patch had not been merged into mainline?


Not yet (and I'm afraid it's not in kgdb-next either) but the ack from Jason is 
from
this kernel cycle so we'll see what can be done!




I thought for what ever reason this was going through the time keeper subtree.  
 I added it immediately to kgdb-next so it will be evaluated in the linux-next 
tree in the next day or so, and we can get this merged in the merge window.

Thanks,
Jason.

Re: [PATCH] misc: kgdbts: Display progress of asynchronous tests

[Jason Wessel]

On 12/12/2017 06:10 AM, Daniel Thompson wrote:

kgdbts includes a couple of different "thrashing" style tests that
may have long runtimes (especially on simulated platforms) and which
run asynchronously. This is uncomfortable for interactive use and
makes setting timeouts tricky for automatic use.



Do you know which test was specifically causing a problem?  It might not be 
documented anywhere but I had usually started a user space process which 
quickly created and deleted user space processes in order to make the kgdbts 
tests complete quickly.

I don't really see any issue with emitting a printk to indicate progress as it 
is debug only and test specific.  As you know printk's change timing.  If I had 
a dime for each time I had seen a problem go away when I started adding 
printk's I'd have at least a 50 cents.  :-)


Jason.

PS.  Added this to kgdb-next and I'll put in a request to get kgdb-next added 
back to the linux-next builder.



Fix by providing a optional means to show progress during these tests.
Selecting 100 is somewhat arbitrary but it matches the step used on
the synchronous tests, is large enough to keep the call to printk
from invalidating the testing and is human enough to "feel about
right".

Signed-off-by: Daniel Thompson 
---
  drivers/misc/kgdbts.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 24108bfad889..6193270e7b3d 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -400,10 +400,14 @@ static void skip_back_repeat_test(char *arg)
int go_back = simple_strtol(arg, NULL, 10);

repeat_test--;
-   if (repeat_test <= 0)
+   if (repeat_test <= 0) {
ts.idx++;
-   else
+   } else {
+   if (repeat_test % 100 == 0)
+   v1printk("kgdbts:RUN ... %d remaining\n", repeat_test);
+
ts.idx -= go_back;
+   }
fill_get_buf(ts.tst[ts.idx].get);
  }

--
2.14.3

Re: [PATCH] misc: kgdbts: Display progress of asynchronous tests

[Jason Wessel]

On 12/12/2017 06:10 AM, Daniel Thompson wrote:

kgdbts includes a couple of different "thrashing" style tests that
may have long runtimes (especially on simulated platforms) and which
run asynchronously. This is uncomfortable for interactive use and
makes setting timeouts tricky for automatic use.



Do you know which test was specifically causing a problem?  It might not be 
documented anywhere but I had usually started a user space process which 
quickly created and deleted user space processes in order to make the kgdbts 
tests complete quickly.

I don't really see any issue with emitting a printk to indicate progress as it 
is debug only and test specific.  As you know printk's change timing.  If I had 
a dime for each time I had seen a problem go away when I started adding 
printk's I'd have at least a 50 cents.  :-)


Jason.

PS.  Added this to kgdb-next and I'll put in a request to get kgdb-next added 
back to the linux-next builder.



Fix by providing a optional means to show progress during these tests.
Selecting 100 is somewhat arbitrary but it matches the step used on
the synchronous tests, is large enough to keep the call to printk
from invalidating the testing and is human enough to "feel about
right".

Signed-off-by: Daniel Thompson 
---
  drivers/misc/kgdbts.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/kgdbts.c b/drivers/misc/kgdbts.c
index 24108bfad889..6193270e7b3d 100644
--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -400,10 +400,14 @@ static void skip_back_repeat_test(char *arg)
int go_back = simple_strtol(arg, NULL, 10);

repeat_test--;
-   if (repeat_test <= 0)
+   if (repeat_test <= 0) {
ts.idx++;
-   else
+   } else {
+   if (repeat_test % 100 == 0)
+   v1printk("kgdbts:RUN ... %d remaining\n", repeat_test);
+
ts.idx -= go_back;
+   }
fill_get_buf(ts.tst[ts.idx].get);
  }

--
2.14.3

Re: [PATCH 2/3] kdb: drop newline in unknown command output

[Jason Wessel]

On 12/08/2017 12:19 PM, Randy Dunlap wrote:

From: Randy Dunlap <rdun...@infradead.org>





Thanks for the series Randy.   I'll get these applied, but I will take
a look at changing this patch slightly.



When an unknown command is entered, kdb prints "Unknown kdb command:"
and then the unknown text, including the newline character. This
causes the ending single-quote mark to be printed on the next line
by itself, so just change the ending newline character to a null
character (end of string) so that it won't be "printed."

Signed-off-by: Randy Dunlap <rdun...@infradead.org>
Cc: Daniel Thompson <daniel.thomp...@linaro.org>
Cc: Jason Wessel <jason.wes...@windriver.com>
Cc: kgdb-bugrep...@lists.sourceforge.net
---
  kernel/debug/kdb/kdb_main.c |   11 +++
  1 file changed, 11 insertions(+)

--- lnx-415-rc1.orig/kernel/debug/kdb/kdb_main.c
+++ lnx-415-rc1/kernel/debug/kdb/kdb_main.c
@@ -1150,6 +1150,16 @@ void kdb_set_current_task(struct task_st
kdb_current_regs = NULL;
  }
  
+static void drop_newline(char *buf)

+{
+   size_t len = strlen(buf);
+
+   if (len == 0)
+   return;
+   if (*(buf + len - 1) == '\n')
+   *(buf + len - 1) = '\0';
+}
+
  /*
   * kdb_local - The main code for kdb.  This routine is invoked on a
   *specific processor, it is not global.  The main kdb() routine
@@ -1327,6 +1337,7 @@ do_full_getstr:
cmdptr = cmd_head;
diag = kdb_parse(cmdbuf);
if (diag == KDB_NOTFOUND) {
+   drop_newline(cmdbuf);



We might be able to get away with just adjusting the pointer in the kdb_parse 
instead of adding the drop_newline because we are returning error anyway and 
the parse needs to be called again in the future for a new command.


Thanks,
Jason.


kdb_printf("Unknown kdb command: '%s'\n", cmdbuf);
diag = 0;
}

Re: [PATCH 2/3] kdb: drop newline in unknown command output

[Jason Wessel]

On 12/08/2017 12:19 PM, Randy Dunlap wrote:

From: Randy Dunlap 





Thanks for the series Randy.   I'll get these applied, but I will take
a look at changing this patch slightly.



When an unknown command is entered, kdb prints "Unknown kdb command:"
and then the unknown text, including the newline character. This
causes the ending single-quote mark to be printed on the next line
by itself, so just change the ending newline character to a null
character (end of string) so that it won't be "printed."

Signed-off-by: Randy Dunlap 
Cc: Daniel Thompson 
Cc: Jason Wessel 
Cc: kgdb-bugrep...@lists.sourceforge.net
---
  kernel/debug/kdb/kdb_main.c |   11 +++
  1 file changed, 11 insertions(+)

--- lnx-415-rc1.orig/kernel/debug/kdb/kdb_main.c
+++ lnx-415-rc1/kernel/debug/kdb/kdb_main.c
@@ -1150,6 +1150,16 @@ void kdb_set_current_task(struct task_st
kdb_current_regs = NULL;
  }
  
+static void drop_newline(char *buf)

+{
+   size_t len = strlen(buf);
+
+   if (len == 0)
+   return;
+   if (*(buf + len - 1) == '\n')
+   *(buf + len - 1) = '\0';
+}
+
  /*
   * kdb_local - The main code for kdb.  This routine is invoked on a
   *specific processor, it is not global.  The main kdb() routine
@@ -1327,6 +1337,7 @@ do_full_getstr:
cmdptr = cmd_head;
diag = kdb_parse(cmdbuf);
if (diag == KDB_NOTFOUND) {
+   drop_newline(cmdbuf);



We might be able to get away with just adjusting the pointer in the kdb_parse 
instead of adding the drop_newline because we are returning error anyway and 
the parse needs to be called again in the future for a new command.


Thanks,
Jason.


kdb_printf("Unknown kdb command: '%s'\n", cmdbuf);
diag = 0;
}

Re: [PATCH 2/2] kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson

[Jason Wessel]

On 12/06/2017 06:52 PM, Randy Dunlap wrote:

On 12/06/2017 02:33 PM, Jason Wessel wrote:

  L:kgdb-bugrep...@lists.sourceforge.net
  T:git git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git



I would urge you to move to a different mailing list server.


No argument here, considering some of the past problems with the mail list.  I 
put in a request to get a list created on vger, and it can be migrated after it 
gets created.

Thanks,
Jason.

Re: [PATCH 2/2] kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson

[Jason Wessel]

On 12/06/2017 06:52 PM, Randy Dunlap wrote:

On 12/06/2017 02:33 PM, Jason Wessel wrote:

  L:kgdb-bugrep...@lists.sourceforge.net
  T:git git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git



I would urge you to move to a different mailing list server.


No argument here, considering some of the past problems with the mail list.  I 
put in a request to get a list created on vger, and it can be migrated after it 
gets created.

Thanks,
Jason.

Re: [Kgdb-bugreport] [PATCH] MAINTAINERS: kgdb: Replace Jason with Daniel

[Jason Wessel]

On 12/05/2017 10:42 AM, Randy Dunlap wrote:

On 12/05/2017 06:55 AM, Daniel Thompson wrote:

On 05/12/17 14:37, Jason Wessel wrote:

I have a series of 50+ patches for kgdb/kdb/usb which have never been 
published.  I am not saying that we actually need any of those patches, but it 
would be nice to let the community decide, and we can see if there is anything 
worth merging into the next cycle or future work with other maintainers.   My 
kernel.org tree stopped working a long time ago, probably from inactivity.  
I'll see if that can get restored in the next few days, or I'll use my github 
tree and send the unpublished work to the mailing list as an RFC.


I, for one, would be interested to see these.


Me also.  I have 3 kdb patches that I just made.




If you have some patches please do send them along to the list.  I have added 
Daniel as an additional maintainer for when I am not around.

We are open for business again now that my kernel.org tree accepts my tag 
signing again.  It will take some time to go through these unpublished patches 
to see what is actually relevant, but I'll posting some of them to the mailing 
reasonably list soon.

Cheers,
Jason.

ps

While on the topic of debuggers...  I was thinking it might be interesting to 
have a gdb-serial stub in an FPGA for debugging the kernel not unlike what was 
done with the firewire debugger that Andi Kleen worked on long ago.  I am not 
exactly sure what kind of run control options exist there but in terms of 
accessing memory it would certainly be plausible to access it.  One option I 
know that is plausible for run control is a small kernel interrupt handler 
perhaps for the run control interface based on the fact you can some FPGAs show 
up like a PCI device.

While I haven't been directly working in upstream linux in last year or two, I 
still do plenty of debugging of full systems with simulators, hardware, and now 
FPGAs too.  :-)

Re: [Kgdb-bugreport] [PATCH] MAINTAINERS: kgdb: Replace Jason with Daniel

[Jason Wessel]

On 12/05/2017 10:42 AM, Randy Dunlap wrote:

On 12/05/2017 06:55 AM, Daniel Thompson wrote:

On 05/12/17 14:37, Jason Wessel wrote:

I have a series of 50+ patches for kgdb/kdb/usb which have never been 
published.  I am not saying that we actually need any of those patches, but it 
would be nice to let the community decide, and we can see if there is anything 
worth merging into the next cycle or future work with other maintainers.   My 
kernel.org tree stopped working a long time ago, probably from inactivity.  
I'll see if that can get restored in the next few days, or I'll use my github 
tree and send the unpublished work to the mailing list as an RFC.


I, for one, would be interested to see these.


Me also.  I have 3 kdb patches that I just made.




If you have some patches please do send them along to the list.  I have added 
Daniel as an additional maintainer for when I am not around.

We are open for business again now that my kernel.org tree accepts my tag 
signing again.  It will take some time to go through these unpublished patches 
to see what is actually relevant, but I'll posting some of them to the mailing 
reasonably list soon.

Cheers,
Jason.

ps

While on the topic of debuggers...  I was thinking it might be interesting to 
have a gdb-serial stub in an FPGA for debugging the kernel not unlike what was 
done with the firewire debugger that Andi Kleen worked on long ago.  I am not 
exactly sure what kind of run control options exist there but in terms of 
accessing memory it would certainly be plausible to access it.  One option I 
know that is plausible for run control is a small kernel interrupt handler 
perhaps for the run control interface based on the fact you can some FPGAs show 
up like a PCI device.

While I haven't been directly working in upstream linux in last year or two, I 
still do plenty of debugging of full systems with simulators, hardware, and now 
FPGAs too.  :-)

[GIT PULL] KGDB/KDB fixes for 4.15-rc2

[Jason Wessel]

Linus,

Please pull the kgdb tree.

Summary of changes:
   * Fix long standing problem with kdb kallsyms_symbol_next() return value
   * Add new co-maintainer Daniel Thompson

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.15-rc2

Thanks,
Jason.

The following changes since commit bebc6082da0a9f5d47a1ea2edc099bf671058bd4:

  Linux 4.14 (2017-11-12 10:46:13 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.15-rc2

for you to fetch changes up to 4e23f78c74934e8ea624b59df58e646e0657608a:

  kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson (2017-12-06 16:12:43 
-0600)


KGDB:
   * Fix long standing problem with kdb kallsyms_symbol_next() return value
   * Add new co-maintainer Daniel Thompson


Daniel Thompson (1):
  kdb: Fix handling of kallsyms_symbol_next() return value

Jason Wessel (1):
  kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson

 MAINTAINERS   | 1 +
 kernel/debug/kdb/kdb_io.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

[GIT PULL] KGDB/KDB fixes for 4.15-rc2

[Jason Wessel]

Linus,

Please pull the kgdb tree.

Summary of changes:
   * Fix long standing problem with kdb kallsyms_symbol_next() return value
   * Add new co-maintainer Daniel Thompson

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.15-rc2

Thanks,
Jason.

The following changes since commit bebc6082da0a9f5d47a1ea2edc099bf671058bd4:

  Linux 4.14 (2017-11-12 10:46:13 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-4.15-rc2

for you to fetch changes up to 4e23f78c74934e8ea624b59df58e646e0657608a:

  kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson (2017-12-06 16:12:43 
-0600)


KGDB:
   * Fix long standing problem with kdb kallsyms_symbol_next() return value
   * Add new co-maintainer Daniel Thompson


Daniel Thompson (1):
  kdb: Fix handling of kallsyms_symbol_next() return value

Jason Wessel (1):
  kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson

 MAINTAINERS   | 1 +
 kernel/debug/kdb/kdb_io.c | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

[PATCH 1/2] kdb: Fix handling of kallsyms_symbol_next() return value

[Jason Wessel]

From: Daniel Thompson <daniel.thomp...@linaro.org>

kallsyms_symbol_next() returns a boolean (true on success). Currently
kdb_read() tests the return value with an inequality that
unconditionally evaluates to true.

This is fixed in the obvious way and, since the conditional branch is
supposed to be unreachable, we also add a WARN_ON().

Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org>
Cc: linux-stable <sta...@vger.kernel.org>
Signed-off-by: Jason Wessel <jason.wes...@windriver.com>
---
 kernel/debug/kdb/kdb_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index e74be38..ed5d349 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -350,7 +350,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
}
kdb_printf("\n");
for (i = 0; i < count; i++) {
-   if (kallsyms_symbol_next(p_tmp, i) < 0)
+   if (WARN_ON(!kallsyms_symbol_next(p_tmp, i)))
break;
kdb_printf("%s ", p_tmp);
*(p_tmp + len) = '\0';
-- 
1.9.1

[PATCH 1/2] kdb: Fix handling of kallsyms_symbol_next() return value

[Jason Wessel]

From: Daniel Thompson 

kallsyms_symbol_next() returns a boolean (true on success). Currently
kdb_read() tests the return value with an inequality that
unconditionally evaluates to true.

This is fixed in the obvious way and, since the conditional branch is
supposed to be unreachable, we also add a WARN_ON().

Reported-by: Dan Carpenter 
Signed-off-by: Daniel Thompson 
Cc: linux-stable 
Signed-off-by: Jason Wessel 
---
 kernel/debug/kdb/kdb_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c
index e74be38..ed5d349 100644
--- a/kernel/debug/kdb/kdb_io.c
+++ b/kernel/debug/kdb/kdb_io.c
@@ -350,7 +350,7 @@ static char *kdb_read(char *buffer, size_t bufsize)
}
kdb_printf("\n");
for (i = 0; i < count; i++) {
-   if (kallsyms_symbol_next(p_tmp, i) < 0)
+   if (WARN_ON(!kallsyms_symbol_next(p_tmp, i)))
break;
kdb_printf("%s ", p_tmp);
*(p_tmp + len) = '\0';
-- 
1.9.1

[PATCH 2/2] kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson

[Jason Wessel]

Signed-off-by: Jason Wessel <jason.wes...@windriver.com>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2811a21..74be63b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7659,6 +7659,7 @@ F:security/keys/
 
 KGDB / KDB /debug_core
 M: Jason Wessel <jason.wes...@windriver.com>
+M: Daniel Thompson <daniel.thomp...@linaro.org>
 W: http://kgdb.wiki.kernel.org/
 L: kgdb-bugrep...@lists.sourceforge.net
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git
-- 
1.9.1

[PATCH 2/2] kgdb/kdb/debug_core: Add co-maintainer Daniel Thompson

[Jason Wessel]

Signed-off-by: Jason Wessel 
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2811a21..74be63b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7659,6 +7659,7 @@ F:security/keys/
 
 KGDB / KDB /debug_core
 M: Jason Wessel 
+M: Daniel Thompson 
 W: http://kgdb.wiki.kernel.org/
 L: kgdb-bugrep...@lists.sourceforge.net
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git
-- 
1.9.1

Re: [PATCH] kdb: use __ktime_get_real_seconds instead of __current_kernel_time

[Jason Wessel]

On 10/12/2017 09:06 AM, Arnd Bergmann wrote:

kdb is the only user of the __current_kernel_time() interface, which is
not y2038 safe and should be removed at some point.

The kdb code also goes to great lengths to print the time in a
human-readable format from 'struct timespec', again using a non-y2038-safe
re-implementation of the generic time_to_tm() code.

Using __current_kernel_time() here is necessary since the regular
accessors that require a sequence lock might hang when called during the
xtime update. However, this is safe in the particular case since kdb is
only interested in the tv_sec field that is updated atomically.

In order to make this y2038-safe, I'm converting the code to the generic
time64_to_tm helper, but that introduces the problem that we have no
interface like __current_kernel_time() that provides a 64-bit timestamp
in a lockless, safe and architecture-independent way. I have multiple
ideas for how to solve that:

- __ktime_get_real_seconds() is lockless, but can return
   incorrect results on 32-bit architectures in the special case that
   we are in the process of changing the time across the epoch, either
   during the timer tick that overflows the seconds in 2038, or while
   calling settimeofday.

- ktime_get_real_fast_ns() would work in this context, but does
   require a call into the clocksource driver to return a high-resolution
   timestamp. This may have undesired side-effects in the debugger,
   since we want to limit the interactions with the rest of the kernel.

- Adding a ktime_get_real_fast_seconds() based on tk_fast_mono
   plus tkr->base_real without the tk_clock_read() delta. Not sure about
   the value of adding yet another interface here.

- Changing the existing ktime_get_real_seconds() to use
   tk_fast_mono on 32-bit architectures rather than xtime_sec.  I think
   this could work, but am not entirely sure if this is an improvement.

I picked the first of those for simplicity here. It's technically
not correct but probably good enough as the time is only used for the
debugging output and the race will likely never be hit in practice.
Another downside is having to move the declaration into a public header
file.

Let me know if anyone has a different preference.



It all seems reasonable to me.  Separately I created the same patch because I 
didn't see this mail first.  The only difference was that I added a comment 
about the __ktime_get_real_seconds() not taking the lock because it was done 
that way in other places in the header file.

===
 extern time64_t ktime_get_real_seconds(void);
+/* does not take xtime_lock */
+extern time64_t __ktime_get_real_seconds(void);
===

Acked-by: Jason Wessel <jason.wes...@windriver.com>

Thanks for your work on the 2038 problems.  :-)

Cheers,
Jason.

Re: [PATCH] kdb: use __ktime_get_real_seconds instead of __current_kernel_time

[Jason Wessel]

On 10/12/2017 09:06 AM, Arnd Bergmann wrote:

kdb is the only user of the __current_kernel_time() interface, which is
not y2038 safe and should be removed at some point.

The kdb code also goes to great lengths to print the time in a
human-readable format from 'struct timespec', again using a non-y2038-safe
re-implementation of the generic time_to_tm() code.

Using __current_kernel_time() here is necessary since the regular
accessors that require a sequence lock might hang when called during the
xtime update. However, this is safe in the particular case since kdb is
only interested in the tv_sec field that is updated atomically.

In order to make this y2038-safe, I'm converting the code to the generic
time64_to_tm helper, but that introduces the problem that we have no
interface like __current_kernel_time() that provides a 64-bit timestamp
in a lockless, safe and architecture-independent way. I have multiple
ideas for how to solve that:

- __ktime_get_real_seconds() is lockless, but can return
   incorrect results on 32-bit architectures in the special case that
   we are in the process of changing the time across the epoch, either
   during the timer tick that overflows the seconds in 2038, or while
   calling settimeofday.

- ktime_get_real_fast_ns() would work in this context, but does
   require a call into the clocksource driver to return a high-resolution
   timestamp. This may have undesired side-effects in the debugger,
   since we want to limit the interactions with the rest of the kernel.

- Adding a ktime_get_real_fast_seconds() based on tk_fast_mono
   plus tkr->base_real without the tk_clock_read() delta. Not sure about
   the value of adding yet another interface here.

- Changing the existing ktime_get_real_seconds() to use
   tk_fast_mono on 32-bit architectures rather than xtime_sec.  I think
   this could work, but am not entirely sure if this is an improvement.

I picked the first of those for simplicity here. It's technically
not correct but probably good enough as the time is only used for the
debugging output and the race will likely never be hit in practice.
Another downside is having to move the declaration into a public header
file.

Let me know if anyone has a different preference.



It all seems reasonable to me.  Separately I created the same patch because I 
didn't see this mail first.  The only difference was that I added a comment 
about the __ktime_get_real_seconds() not taking the lock because it was done 
that way in other places in the header file.

===
 extern time64_t ktime_get_real_seconds(void);
+/* does not take xtime_lock */
+extern time64_t __ktime_get_real_seconds(void);
===

Acked-by: Jason Wessel 

Thanks for your work on the 2038 problems.  :-)

Cheers,
Jason.

Re: [PATCH] kdb: use __ktime_get_real_seconds instead of __current_kernel_time

[Jason Wessel]

On 10/13/2017 03:26 AM, Daniel Thompson wrote:

On 12/10/17 23:40, Andrew Morton wrote:

On Thu, 12 Oct 2017 16:06:11 +0200 Arnd Bergmann  wrote:


kdb is the only user of the __current_kernel_time() interface, which is
not y2038 safe and should be removed at some point.

The kdb code also goes to great lengths to print the time in a
human-readable format from 'struct timespec', again using a non-y2038-safe
re-implementation of the generic time_to_tm() code.


Is it really necessary for the kdb `summary' command to print the
time/date?  Which puppies would die if we just removed it all?


kdb may enter spontaneously (BUG(), etc) so it can be useful if one
returns from an overnight test run to know how long things survived.

It would almost certainly be possible for a skilled user to reconstruct
the time of death. Having said that, one of the things you can do with
kdb (although I admit *I* have never done it) is leave a macro command
in the hands of an unskilled user.

Short summary: no puppies would die, but perhaps some might go hungry
for a little while when their owner is late home?



Daniel is correct.  This is information that was just a nice to have for 
postmortem analysis it can also be called via gdb macros.

If kdb is really the last remaining user, it seems like the interface should 
get removed and kdb can print time another way that is compatible with the 2038 
fixes.

After having taken a quick look it would seem __ktime_get_real_seconds()  (because we 
need the non-lock protected version) and time64_to_tm() should be the proper replacement. 
 There is certainly no reason to duplicate code in kdb for the "summary" 
functions.

I am assuming no one has fixed this yet, so I should be able to provide a patch 
to the list along the lines of what is below.  And I will follow it with a 
second patch to remove the __current_kernel_time() to lkml and the timekeeper 
maintainer.

Cheers,
Jason.

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 09168c52ab64..2529cc470a45 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -53,6 +53,8 @@ extern void getrawmonotonic64(struct timespec64 *ts);
 extern void ktime_get_ts64(struct timespec64 *ts);
 extern time64_t ktime_get_seconds(void);
 extern time64_t ktime_get_real_seconds(void);
+/* does not take xtime_lock */
+extern time64_t __ktime_get_real_seconds(void);
 
 extern int __getnstimeofday64(struct timespec64 *tv);

 extern void getnstimeofday64(struct timespec64 *tv);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 2a20c0dfdafc..c7a02710d884 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2477,41 +2477,6 @@ static int kdb_kill(int argc, const char **argv)
return 0;
 }
 
-struct kdb_tm {

-   int tm_sec; /* seconds */
-   int tm_min; /* minutes */
-   int tm_hour;/* hours */
-   int tm_mday;/* day of the month */
-   int tm_mon; /* month */
-   int tm_year;/* year */
-};
-
-static void kdb_gmtime(struct timespec *tv, struct kdb_tm *tm)
-{
-   /* This will work from 1970-2099, 2100 is not a leap year */
-   static int mon_day[] = { 31, 29, 31, 30, 31, 30, 31,
-31, 30, 31, 30, 31 };
-   memset(tm, 0, sizeof(*tm));
-   tm->tm_sec  = tv->tv_sec % (24 * 60 * 60);
-   tm->tm_mday = tv->tv_sec / (24 * 60 * 60) +
-   (2 * 365 + 1); /* shift base from 1970 to 1968 */
-   tm->tm_min =  tm->tm_sec / 60 % 60;
-   tm->tm_hour = tm->tm_sec / 60 / 60;
-   tm->tm_sec =  tm->tm_sec % 60;
-   tm->tm_year = 68 + 4*(tm->tm_mday / (4*365+1));
-   tm->tm_mday %= (4*365+1);
-   mon_day[1] = 29;
-   while (tm->tm_mday >= mon_day[tm->tm_mon]) {
-   tm->tm_mday -= mon_day[tm->tm_mon];
-   if (++tm->tm_mon == 12) {
-   tm->tm_mon = 0;
-   ++tm->tm_year;
-   mon_day[1] = 28;
-   }
-   }
-   ++tm->tm_mday;
-}
-
 /*
  * Most of this code has been lifted from kernel/timer.c::sys_sysinfo().
  * I cannot call that code directly from kdb, it has an unconditional
@@ -2537,8 +2502,8 @@ static void kdb_sysinfo(struct sysinfo *val)
  */
 static int kdb_summary(int argc, const char **argv)
 {
-   struct timespec now;
-   struct kdb_tm tm;
+   time64_t now_seconds;
+   struct tm tm;
struct sysinfo val;
 
 	if (argc)

@@ -2552,9 +2517,9 @@ static int kdb_summary(int argc, const char **argv)
kdb_printf("domainname %s\n", init_uts_ns.name.domainname);
kdb_printf("ccversion  %s\n", __stringify(CCVERSION));
 
-	now = __current_kernel_time();

-   kdb_gmtime(, );
-   kdb_printf("date   %04d-%02d-%02d %02d:%02d:%02d "
+   now_seconds = __ktime_get_real_seconds();
+   time64_to_tm(now_seconds, sys_tz.tz_minuteswest * 60, );
+   kdb_printf("date   

Re: [PATCH] kdb: use __ktime_get_real_seconds instead of __current_kernel_time

[Jason Wessel]

On 10/13/2017 03:26 AM, Daniel Thompson wrote:

On 12/10/17 23:40, Andrew Morton wrote:

On Thu, 12 Oct 2017 16:06:11 +0200 Arnd Bergmann  wrote:


kdb is the only user of the __current_kernel_time() interface, which is
not y2038 safe and should be removed at some point.

The kdb code also goes to great lengths to print the time in a
human-readable format from 'struct timespec', again using a non-y2038-safe
re-implementation of the generic time_to_tm() code.


Is it really necessary for the kdb `summary' command to print the
time/date?  Which puppies would die if we just removed it all?


kdb may enter spontaneously (BUG(), etc) so it can be useful if one
returns from an overnight test run to know how long things survived.

It would almost certainly be possible for a skilled user to reconstruct
the time of death. Having said that, one of the things you can do with
kdb (although I admit *I* have never done it) is leave a macro command
in the hands of an unskilled user.

Short summary: no puppies would die, but perhaps some might go hungry
for a little while when their owner is late home?



Daniel is correct.  This is information that was just a nice to have for 
postmortem analysis it can also be called via gdb macros.

If kdb is really the last remaining user, it seems like the interface should 
get removed and kdb can print time another way that is compatible with the 2038 
fixes.

After having taken a quick look it would seem __ktime_get_real_seconds()  (because we 
need the non-lock protected version) and time64_to_tm() should be the proper replacement. 
 There is certainly no reason to duplicate code in kdb for the "summary" 
functions.

I am assuming no one has fixed this yet, so I should be able to provide a patch 
to the list along the lines of what is below.  And I will follow it with a 
second patch to remove the __current_kernel_time() to lkml and the timekeeper 
maintainer.

Cheers,
Jason.

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 09168c52ab64..2529cc470a45 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -53,6 +53,8 @@ extern void getrawmonotonic64(struct timespec64 *ts);
 extern void ktime_get_ts64(struct timespec64 *ts);
 extern time64_t ktime_get_seconds(void);
 extern time64_t ktime_get_real_seconds(void);
+/* does not take xtime_lock */
+extern time64_t __ktime_get_real_seconds(void);
 
 extern int __getnstimeofday64(struct timespec64 *tv);

 extern void getnstimeofday64(struct timespec64 *tv);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 2a20c0dfdafc..c7a02710d884 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2477,41 +2477,6 @@ static int kdb_kill(int argc, const char **argv)
return 0;
 }
 
-struct kdb_tm {

-   int tm_sec; /* seconds */
-   int tm_min; /* minutes */
-   int tm_hour;/* hours */
-   int tm_mday;/* day of the month */
-   int tm_mon; /* month */
-   int tm_year;/* year */
-};
-
-static void kdb_gmtime(struct timespec *tv, struct kdb_tm *tm)
-{
-   /* This will work from 1970-2099, 2100 is not a leap year */
-   static int mon_day[] = { 31, 29, 31, 30, 31, 30, 31,
-31, 30, 31, 30, 31 };
-   memset(tm, 0, sizeof(*tm));
-   tm->tm_sec  = tv->tv_sec % (24 * 60 * 60);
-   tm->tm_mday = tv->tv_sec / (24 * 60 * 60) +
-   (2 * 365 + 1); /* shift base from 1970 to 1968 */
-   tm->tm_min =  tm->tm_sec / 60 % 60;
-   tm->tm_hour = tm->tm_sec / 60 / 60;
-   tm->tm_sec =  tm->tm_sec % 60;
-   tm->tm_year = 68 + 4*(tm->tm_mday / (4*365+1));
-   tm->tm_mday %= (4*365+1);
-   mon_day[1] = 29;
-   while (tm->tm_mday >= mon_day[tm->tm_mon]) {
-   tm->tm_mday -= mon_day[tm->tm_mon];
-   if (++tm->tm_mon == 12) {
-   tm->tm_mon = 0;
-   ++tm->tm_year;
-   mon_day[1] = 28;
-   }
-   }
-   ++tm->tm_mday;
-}
-
 /*
  * Most of this code has been lifted from kernel/timer.c::sys_sysinfo().
  * I cannot call that code directly from kdb, it has an unconditional
@@ -2537,8 +2502,8 @@ static void kdb_sysinfo(struct sysinfo *val)
  */
 static int kdb_summary(int argc, const char **argv)
 {
-   struct timespec now;
-   struct kdb_tm tm;
+   time64_t now_seconds;
+   struct tm tm;
struct sysinfo val;
 
 	if (argc)

@@ -2552,9 +2517,9 @@ static int kdb_summary(int argc, const char **argv)
kdb_printf("domainname %s\n", init_uts_ns.name.domainname);
kdb_printf("ccversion  %s\n", __stringify(CCVERSION));
 
-	now = __current_kernel_time();

-   kdb_gmtime(, );
-   kdb_printf("date   %04d-%02d-%02d %02d:%02d:%02d "
+   now_seconds = __ktime_get_real_seconds();
+   time64_to_tm(now_seconds, sys_tz.tz_minuteswest * 60, );
+   kdb_printf("date   %04ld-%02d-%02d 

Re: [Kgdb-bugreport] [PATCH] MAINTAINERS: kgdb: Replace Jason with Daniel

[Jason Wessel]

On 12/05/2017 08:09 AM, Lee Jones wrote:

On Tue, 05 Dec 2017, Daniel Thompson wrote:


... with many, many thanks for Jason for all his hard work.

Cc: Jason Wessel <jason.wes...@windriver.com>
Signed-off-by: Daniel Thompson <daniel.thomp...@linaro.org>
---

Notes:
 Over the years Jason has become increasingly hard to get hold off
 and I think he must now be regarded as inactive.
 
 Patches in kgdb-next (mine as it happens) have been there for over a

 year without a corresponding pull request and a couple of architecture
 specific kgdb fixes have ended up missing a release cycle (or two) as
 the architecture maintainer waits for an Acked-by from Jason.
 
 In the past I've had to rely on Andrew M. to land my own changes to

 kgdb and in the v4.14 cycle you'll find my Acked-by on b8347c219649
 ("x86/debug: Handle warnings before the notifier chain, to fix KGDB
 crash"). That I was sharing surrogate acks convinced me we need a
 change here and I've offered Jason help via private e-mail without
 reply.
 
 So, I really would prefer it it if this patch listed me as a

 co-maintainer or, failing that, as least had Jason's blessing... but
 it doesn't. I certainly suggest this patch takes a long time in
 review, and if it doesn't attract Jason's attention then I can only
 reiterate what is says in the commit log: Thanks Jason!

  MAINTAINERS | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)


It looks like Jason has been inactive in all aspects of upstream
maintainership and as a contributor for well over a year now.


I have not been working directly on upstream kernel contributions for quite 
some time.  It doesn't mean I haven't been involved with kernel development.  
Patches that I have reviewed or suggested to other developers generally don't 
bare my name.  I wouldn't mind trying to take a slightly more gradual passing 
of the baton and add Daniel as co-maintainer for a while before I retire from 
kernel work and merge myself away in the coming years. :-)

I have a series of 50+ patches for kgdb/kdb/usb which have never been 
published.  I am not saying that we actually need any of those patches, but it 
would be nice to let the community decide, and we can see if there is anything 
worth merging into the next cycle or future work with other maintainers.   My 
kernel.org tree stopped working a long time ago, probably from inactivity.  
I'll see if that can get restored in the next few days, or I'll use my github 
tree and send the unpublished work to the mailing list as an RFC.  And for what 
it is worth if none of this happens by the end of 4.16, by all means Daniel has 
my blessing to be the sole maintainer.

Many thanks to Daniel for his contributions!

Cheers,
Jason.

Re: [Kgdb-bugreport] [PATCH] MAINTAINERS: kgdb: Replace Jason with Daniel

[Jason Wessel]

On 12/05/2017 08:09 AM, Lee Jones wrote:

On Tue, 05 Dec 2017, Daniel Thompson wrote:


... with many, many thanks for Jason for all his hard work.

Cc: Jason Wessel 
Signed-off-by: Daniel Thompson 
---

Notes:
 Over the years Jason has become increasingly hard to get hold off
 and I think he must now be regarded as inactive.
 
 Patches in kgdb-next (mine as it happens) have been there for over a

 year without a corresponding pull request and a couple of architecture
 specific kgdb fixes have ended up missing a release cycle (or two) as
 the architecture maintainer waits for an Acked-by from Jason.
 
 In the past I've had to rely on Andrew M. to land my own changes to

 kgdb and in the v4.14 cycle you'll find my Acked-by on b8347c219649
 ("x86/debug: Handle warnings before the notifier chain, to fix KGDB
 crash"). That I was sharing surrogate acks convinced me we need a
 change here and I've offered Jason help via private e-mail without
 reply.
 
 So, I really would prefer it it if this patch listed me as a

 co-maintainer or, failing that, as least had Jason's blessing... but
 it doesn't. I certainly suggest this patch takes a long time in
 review, and if it doesn't attract Jason's attention then I can only
 reiterate what is says in the commit log: Thanks Jason!

  MAINTAINERS | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)


It looks like Jason has been inactive in all aspects of upstream
maintainership and as a contributor for well over a year now.


I have not been working directly on upstream kernel contributions for quite 
some time.  It doesn't mean I haven't been involved with kernel development.  
Patches that I have reviewed or suggested to other developers generally don't 
bare my name.  I wouldn't mind trying to take a slightly more gradual passing 
of the baton and add Daniel as co-maintainer for a while before I retire from 
kernel work and merge myself away in the coming years. :-)

I have a series of 50+ patches for kgdb/kdb/usb which have never been 
published.  I am not saying that we actually need any of those patches, but it 
would be nice to let the community decide, and we can see if there is anything 
worth merging into the next cycle or future work with other maintainers.   My 
kernel.org tree stopped working a long time ago, probably from inactivity.  
I'll see if that can get restored in the next few days, or I'll use my github 
tree and send the unpublished work to the mailing list as an RFC.  And for what 
it is worth if none of this happens by the end of 4.16, by all means Daniel has 
my blessing to be the sole maintainer.

Many thanks to Daniel for his contributions!

Cheers,
Jason.

Re: [BUG] kgdb/ftrace - sleeping in invalid context

[Jason Wessel]

On 11/17/2016 01:16 PM, Brian Norris wrote:

Hi,

I've been poking around KGDB, and I noticed that the KDB 'ftdump'
command (to dump ftrace logs) produces warnings like this:

(gdb) monitor ftdump
Dumping ftrace buffer:
BUG: sleeping function called from invalid context at mm/slab.h:359
in_atomic(): 1, irqs_disabled(): 128, pid: 116, name: irq/65-chromeos
CPU: 4 PID: 116 Comm: irq/65-chromeos Not tainted 4.4.21 #575
Call trace:
[] dump_backtrace+0x0/0x160
[] show_stack+0x20/0x28
[] dump_stack+0x90/0xb0
[] ___might_sleep+0x140/0x14c
[] __might_sleep+0x80/0x90
[] kmem_cache_alloc_trace+0x5c/0x238
[] ring_buffer_read_prepare+0x4c/0xa4
[] kdb_ftdump+0x200/0x3e4
[] kdb_parse+0x548/0x628
[] gdb_serial_stub+0x89c/0xaac
[] kgdb_cpu_enter+0x1e0/0x5b0
[] kgdb_handle_exception+0x1a0/0x1e4
[] kgdb_compiled_brk_fn+0x30/0x3c
[] brk_handler+0x9c/0xb0
[] do_debug_exception+0x60/0xe0
Exception stack(0xffc0ecffb820 to 0xffc0ecffb950)
b820: ffc0010a6000 0080 ffc0ecffba10 ffc0002bf020
b840: 61c5  ffc0ecffb870 00018160
b860: 0003 00c3 ffc000be15b9 ffc000be7053
b880: 0005 ffc0010a6c48 ffc0ecffb930 ffc00026f8e8
b8a0: ffc000c362ca ffc00108c000 ffc00026f880 0001
b8c0: 0007 ffc0ecfb6600 0001 cb88537fdc8ba615
b8e0: ffc0011b6430 0001  ffc0011b6438
b900:   0006 ffc0010cc1c0
b920: ffc00043eed8 7f7f7f7f 681f39616369ff46 7f7f7f7f7f7f7f7f
b940: 0101010101010101 0008
[] el1_dbg+0x18/0x74
[] sysrq_handle_dbg+0x54/0x5c
[] __handle_sysrq+0xa4/0x15c
[] sysrq_filter+0x11c/0x348
[] input_to_handler+0x60/0x100
[] input_pass_values.part.2+0x78/0x144
[] input_handle_event+0x280/0x4d4
[] input_event+0x70/0x8c
[...]

It looks like (almost?) all KDB code gets run in an exception context,
so I don't see how sleeping allocations (such as those in
ring_buffer_read_prepare()) are supposed to be able to work if they
don't immediately find enough memory.

I can't think of a great simple fix for this, other than borrowing the
hack from kdb_private.h:

#define GFP_KDB (in_interrupt() ? GFP_ATOMIC : GFP_KERNEL)

AFAICT, the necessary allocations are all pretty small actually, and so
GFP_ATOMIC may not be a problem, even in the non-KDB ring buffer case.

Thoughts? I figured I'd ask questions before blindly sending patches, as
I'm not very familiar with this code.


In the past some buffers had be pre-allocated outside of the exception context 
in order to properly dump the ftrace code. Perhaps these patches were lost or 
the interface changed slightly over time.  Certainly we should never perform 
allocations while in the exception context.

Cheers,
Jason.

Re: [BUG] kgdb/ftrace - sleeping in invalid context

[Jason Wessel]

On 11/17/2016 01:16 PM, Brian Norris wrote:

Hi,

I've been poking around KGDB, and I noticed that the KDB 'ftdump'
command (to dump ftrace logs) produces warnings like this:

(gdb) monitor ftdump
Dumping ftrace buffer:
BUG: sleeping function called from invalid context at mm/slab.h:359
in_atomic(): 1, irqs_disabled(): 128, pid: 116, name: irq/65-chromeos
CPU: 4 PID: 116 Comm: irq/65-chromeos Not tainted 4.4.21 #575
Call trace:
[] dump_backtrace+0x0/0x160
[] show_stack+0x20/0x28
[] dump_stack+0x90/0xb0
[] ___might_sleep+0x140/0x14c
[] __might_sleep+0x80/0x90
[] kmem_cache_alloc_trace+0x5c/0x238
[] ring_buffer_read_prepare+0x4c/0xa4
[] kdb_ftdump+0x200/0x3e4
[] kdb_parse+0x548/0x628
[] gdb_serial_stub+0x89c/0xaac
[] kgdb_cpu_enter+0x1e0/0x5b0
[] kgdb_handle_exception+0x1a0/0x1e4
[] kgdb_compiled_brk_fn+0x30/0x3c
[] brk_handler+0x9c/0xb0
[] do_debug_exception+0x60/0xe0
Exception stack(0xffc0ecffb820 to 0xffc0ecffb950)
b820: ffc0010a6000 0080 ffc0ecffba10 ffc0002bf020
b840: 61c5  ffc0ecffb870 00018160
b860: 0003 00c3 ffc000be15b9 ffc000be7053
b880: 0005 ffc0010a6c48 ffc0ecffb930 ffc00026f8e8
b8a0: ffc000c362ca ffc00108c000 ffc00026f880 0001
b8c0: 0007 ffc0ecfb6600 0001 cb88537fdc8ba615
b8e0: ffc0011b6430 0001  ffc0011b6438
b900:   0006 ffc0010cc1c0
b920: ffc00043eed8 7f7f7f7f 681f39616369ff46 7f7f7f7f7f7f7f7f
b940: 0101010101010101 0008
[] el1_dbg+0x18/0x74
[] sysrq_handle_dbg+0x54/0x5c
[] __handle_sysrq+0xa4/0x15c
[] sysrq_filter+0x11c/0x348
[] input_to_handler+0x60/0x100
[] input_pass_values.part.2+0x78/0x144
[] input_handle_event+0x280/0x4d4
[] input_event+0x70/0x8c
[...]

It looks like (almost?) all KDB code gets run in an exception context,
so I don't see how sleeping allocations (such as those in
ring_buffer_read_prepare()) are supposed to be able to work if they
don't immediately find enough memory.

I can't think of a great simple fix for this, other than borrowing the
hack from kdb_private.h:

#define GFP_KDB (in_interrupt() ? GFP_ATOMIC : GFP_KERNEL)

AFAICT, the necessary allocations are all pretty small actually, and so
GFP_ATOMIC may not be a problem, even in the non-KDB ring buffer case.

Thoughts? I figured I'd ask questions before blindly sending patches, as
I'm not very familiar with this code.


In the past some buffers had be pre-allocated outside of the exception context 
in order to properly dump the ftrace code. Perhaps these patches were lost or 
the interface changed slightly over time.  Certainly we should never perform 
allocations while in the exception context.

Cheers,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 09/15/2016 11:32 PM, AKASHI Takahiro wrote:

@@ -176,18 +183,14 @@ int kgdb_arch_handle_exception(int exception_vector, int 
signo,

 * over and over again.
 */
kgdb_arch_update_addr(linux_regs, remcom_in_buffer);
-   atomic_set(_cpu_doing_single_step, -1);
-   kgdb_single_step =  0;


This is a subtle change, but I assume it is what you intended?  All the CPUs 
will get released into the run state when exiting the kgdb exception handler.

You are talking about "- kgdb_single_step = 0." Right?



Correct.


Do you think that there is any (negative) side effect of this change?



Not at all.   The kernel debugger always skids to a stop, and it is more reliable from a 
locking perspective if the other CPU threads are released while a single CPU is asked to 
single step until the next "skid" for all the other CPUs.

When you do not release the other CPUs you can end up single stepping a CPU 
which dead locks or never exits a lock elsewhere due to what ever it was 
blocking on never getting freed from another CPU.

Cheers,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 09/15/2016 11:32 PM, AKASHI Takahiro wrote:

@@ -176,18 +183,14 @@ int kgdb_arch_handle_exception(int exception_vector, int 
signo,

 * over and over again.
 */
kgdb_arch_update_addr(linux_regs, remcom_in_buffer);
-   atomic_set(_cpu_doing_single_step, -1);
-   kgdb_single_step =  0;


This is a subtle change, but I assume it is what you intended?  All the CPUs 
will get released into the run state when exiting the kgdb exception handler.

You are talking about "- kgdb_single_step = 0." Right?



Correct.


Do you think that there is any (negative) side effect of this change?



Not at all.   The kernel debugger always skids to a stop, and it is more reliable from a 
locking perspective if the other CPU threads are released while a single CPU is asked to 
single step until the next "skid" for all the other CPUs.

When you do not release the other CPUs you can end up single stepping a CPU 
which dead locks or never exits a lock elsewhere due to what ever it was 
blocking on never getting freed from another CPU.

Cheers,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 09/16/2016 02:45 AM, Will Deacon wrote:

On Fri, Sep 16, 2016 at 01:32:19PM +0900, AKASHI Takahiro wrote:

On Thu, Sep 15, 2016 at 08:04:57AM -0500, Jason Wessel wrote:

I added the patch to kgdb-next after fixing up the context since it no
longer applied to the mainline (
https://git.kernel.org/cgit/linux/kernel/git/jwessel/kgdb.git/log/?h=kgdb-next).
If there is further discussion on the point above, another patch can be
added, but it I am assuming this is the way you desire it to work as
there are some other architectures that use the same behaviour.  I do
not presently have any ARM64 hardware to validate this particular
change.

I also added to the patch a "Cc: linux-stable <sta...@vger.kernel.org>"
so we can have this appear on some of the older kernels.

Since Will asked me to split this patch into a few, I need some reworks
to clarify which hunks in the patch are necessary for which version of kernel.

Yes, splitting the patch would be much better for sorting out the stable
backports too. Jason, please can you drop the patch for now? I don't mind
whether the end result goes via arm64 or kgdb, but we should at least both
agree on it first :)


Splitting it is a very wise idea so that we can have all the -stable kernels 
patched up with a working single step function.

The separated patches can easily be tagged with the CC line examples as shown 
below:

Cc: <sta...@vger.kernel.org> # 3.15.x-
Cc: <sta...@vger.kernel.org> # 4.4
Cc: <sta...@vger.kernel.org> # 4.4-4.5

I had dropped the original patch.

Cheers,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 09/16/2016 02:45 AM, Will Deacon wrote:

On Fri, Sep 16, 2016 at 01:32:19PM +0900, AKASHI Takahiro wrote:

On Thu, Sep 15, 2016 at 08:04:57AM -0500, Jason Wessel wrote:

I added the patch to kgdb-next after fixing up the context since it no
longer applied to the mainline (
https://git.kernel.org/cgit/linux/kernel/git/jwessel/kgdb.git/log/?h=kgdb-next).
If there is further discussion on the point above, another patch can be
added, but it I am assuming this is the way you desire it to work as
there are some other architectures that use the same behaviour.  I do
not presently have any ARM64 hardware to validate this particular
change.

I also added to the patch a "Cc: linux-stable "
so we can have this appear on some of the older kernels.

Since Will asked me to split this patch into a few, I need some reworks
to clarify which hunks in the patch are necessary for which version of kernel.

Yes, splitting the patch would be much better for sorting out the stable
backports too. Jason, please can you drop the patch for now? I don't mind
whether the end result goes via arm64 or kgdb, but we should at least both
agree on it first :)


Splitting it is a very wise idea so that we can have all the -stable kernels 
patched up with a working single step function.

The separated patches can easily be tagged with the CC line examples as shown 
below:

Cc:  # 3.15.x-
Cc:  # 4.4
Cc:  # 4.4-4.5

I had dropped the original patch.

Cheers,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 04/20/2015 08:13 PM, AKASHI Takahiro wrote:

Jason,

Could you please review my patch below?
See also arm64 maintainer's comment:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html

Thanks,
-Takahiro AKASHI

I tried to verify kgdb in vanilla kernel on fast model, but it seems that
the single stepping with kgdb doesn't work correctly since its first
appearance at v3.15.

On v3.15, 'stepi' command after breaking the kernel at some breakpoint
steps forward to the next instruction, but the succeeding 'stepi' never
goes beyond that.
On v3.16, 'stepi' moves forward and stops at the next instruction just
after enable_dbg in el1_dbg, and never goes beyond that. This variance of
behavior seems to come in with the following patch in v3.16:

commit 2a2830703a23 ("arm64: debug: avoid accessing mdscr_el1 on fault
paths where possible")

This patch
(1) moves kgdb_disable_single_step() from 'c' command handling to single
step handler.
This makes sure that single stepping gets effective at every 's' command.
Please note that, under the current implementation, single step bit in
spsr, which is cleared by the first single stepping, will not be set
again for the consecutive 's' commands because single step bit in mdscr
is still kept on (that is, kernel_active_single_step() in
kgdb_arch_handle_exception() is true).
(2) re-implements kgdb_roundup_cpus() because the current implementation
enabled interrupts naively. See below.
(3) removes 'enable_dbg' in el1_dbg.
Single step bit in mdscr is turned on in do_handle_exception()->
kgdb_handle_expection() before returning to debugged context, and if
debug exception is enabled in el1_dbg, we will see unexpected single-
stepping in el1_dbg.
Since v3.18, the following patch does the same:
  commit 1059c6bf8534 ("arm64: debug: don't re-enable debug exceptions
  on return from el1_dbg)
(4) masks interrupts while single-stepping one instruction.
If an interrupt is caught during processing a single-stepping, debug
exception is unintentionally enabled by el1_irq's 'enable_dbg' before
returning to debugged context.
Thus, like in (2), we will see unexpected single-stepping in el1_irq.

Basically (1) and (2) are for v3.15, (3) and (4) for v3.1[67].

@@ -176,18 +183,14 @@ int kgdb_arch_handle_exception(int exception_vector, int 
signo,
 * over and over again.
 */
kgdb_arch_update_addr(linux_regs, remcom_in_buffer);
-   atomic_set(_cpu_doing_single_step, -1);
-   kgdb_single_step =  0;



This is a subtle change, but I assume it is what you intended?  All the CPUs 
will get released into the run state when exiting the kgdb exception handler.



-
-   /*
-* Received continue command, disable single step
-*/
-   if (kernel_active_single_step())
-   kernel_disable_single_step();




I see why this is not needed above any more given that you have a function that 
cleans up the state on entry to the kgdb exception handler.

The rest of the patch is fine.

I added the patch to kgdb-next after fixing up the context since it no longer 
applied to the mainline ( 
https://git.kernel.org/cgit/linux/kernel/git/jwessel/kgdb.git/log/?h=kgdb-next).
  If there is further discussion on the point above, another patch can be 
added, but it I am assuming this is the way you desire it to work as there are 
some other architectures that use the same behaviour.  I do not presently have 
any ARM64 hardware to validate this particular change.

I also added to the patch a "Cc: linux-stable " so we 
can have this appear on some of the older kernels.


Thanks,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 04/20/2015 08:13 PM, AKASHI Takahiro wrote:

Jason,

Could you please review my patch below?
See also arm64 maintainer's comment:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html

Thanks,
-Takahiro AKASHI

I tried to verify kgdb in vanilla kernel on fast model, but it seems that
the single stepping with kgdb doesn't work correctly since its first
appearance at v3.15.

On v3.15, 'stepi' command after breaking the kernel at some breakpoint
steps forward to the next instruction, but the succeeding 'stepi' never
goes beyond that.
On v3.16, 'stepi' moves forward and stops at the next instruction just
after enable_dbg in el1_dbg, and never goes beyond that. This variance of
behavior seems to come in with the following patch in v3.16:

commit 2a2830703a23 ("arm64: debug: avoid accessing mdscr_el1 on fault
paths where possible")

This patch
(1) moves kgdb_disable_single_step() from 'c' command handling to single
step handler.
This makes sure that single stepping gets effective at every 's' command.
Please note that, under the current implementation, single step bit in
spsr, which is cleared by the first single stepping, will not be set
again for the consecutive 's' commands because single step bit in mdscr
is still kept on (that is, kernel_active_single_step() in
kgdb_arch_handle_exception() is true).
(2) re-implements kgdb_roundup_cpus() because the current implementation
enabled interrupts naively. See below.
(3) removes 'enable_dbg' in el1_dbg.
Single step bit in mdscr is turned on in do_handle_exception()->
kgdb_handle_expection() before returning to debugged context, and if
debug exception is enabled in el1_dbg, we will see unexpected single-
stepping in el1_dbg.
Since v3.18, the following patch does the same:
  commit 1059c6bf8534 ("arm64: debug: don't re-enable debug exceptions
  on return from el1_dbg)
(4) masks interrupts while single-stepping one instruction.
If an interrupt is caught during processing a single-stepping, debug
exception is unintentionally enabled by el1_irq's 'enable_dbg' before
returning to debugged context.
Thus, like in (2), we will see unexpected single-stepping in el1_irq.

Basically (1) and (2) are for v3.15, (3) and (4) for v3.1[67].

@@ -176,18 +183,14 @@ int kgdb_arch_handle_exception(int exception_vector, int 
signo,
 * over and over again.
 */
kgdb_arch_update_addr(linux_regs, remcom_in_buffer);
-   atomic_set(_cpu_doing_single_step, -1);
-   kgdb_single_step =  0;



This is a subtle change, but I assume it is what you intended?  All the CPUs 
will get released into the run state when exiting the kgdb exception handler.



-
-   /*
-* Received continue command, disable single step
-*/
-   if (kernel_active_single_step())
-   kernel_disable_single_step();




I see why this is not needed above any more given that you have a function that 
cleans up the state on entry to the kgdb exception handler.

The rest of the patch is fine.

I added the patch to kgdb-next after fixing up the context since it no longer 
applied to the mainline ( 
https://git.kernel.org/cgit/linux/kernel/git/jwessel/kgdb.git/log/?h=kgdb-next).
  If there is further discussion on the point above, another patch can be 
added, but it I am assuming this is the way you desire it to work as there are 
some other architectures that use the same behaviour.  I do not presently have 
any ARM64 hardware to validate this particular change.

I also added to the patch a "Cc: linux-stable " so we 
can have this appear on some of the older kernels.


Thanks,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 09/15/2016 05:41 AM, Daniel Thompson wrote:

On 15/09/16 08:56, AKASHI Takahiro wrote:

On Wed, Sep 14, 2016 at 03:58:51PM +0100, Will Deacon wrote:

Hi Akashi,

On Tue, Apr 21, 2015 at 02:13:13AM +0100, AKASHI Takahiro wrote:

Could you please review my patch below?
See also arm64 maintainer's comment:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html


-ETIMEDOUT waiting for the kdgb folk to comment. Ppeople have reported
that this patch is required for kgdb to work correctly on arm64, so I'm
happy to merge it.


I'm happy, too.


I'll keep an eye out and FWIW see if I can throw in a review. I'm not
really one of "kgdb folk" but did examine it fairly deeply in the early
stages of the FIQ/NMI work (and which has since stopped focussing on kgdb).

I have some equally elderly, albeit rather less critical, kdb patches
that I should have pushed harder for so I'm sympathetic here ;-)



Hey, if you do happen to have something, why not send it along.  I just asked 
the linux-next folks to re-activate the kgdb-next branch so that merges can 
start taking place again.  It appears that the final merge request never even 
went through from linux-next as I just rebased it and there was a patch in 
there from Daniel dating back to over a year ago.

If there is work out there that needs merging and reviews lets get it done.  I 
had been kind of stuck in time on what seems like the stone age 3.14 kernel 
projects until recently, but jumped back into mainline development about a 
month ago.

I have no objection to merging the ARM64 single step patch and have comments in 
a separate mail that follows with respect to the patch.


Cheers,
Jason.

Re: [RESEND PATCH] arm64: kgdb: fix single stepping

[Jason Wessel]

On 09/15/2016 05:41 AM, Daniel Thompson wrote:

On 15/09/16 08:56, AKASHI Takahiro wrote:

On Wed, Sep 14, 2016 at 03:58:51PM +0100, Will Deacon wrote:

Hi Akashi,

On Tue, Apr 21, 2015 at 02:13:13AM +0100, AKASHI Takahiro wrote:

Could you please review my patch below?
See also arm64 maintainer's comment:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/313712.html


-ETIMEDOUT waiting for the kdgb folk to comment. Ppeople have reported
that this patch is required for kgdb to work correctly on arm64, so I'm
happy to merge it.


I'm happy, too.


I'll keep an eye out and FWIW see if I can throw in a review. I'm not
really one of "kgdb folk" but did examine it fairly deeply in the early
stages of the FIQ/NMI work (and which has since stopped focussing on kgdb).

I have some equally elderly, albeit rather less critical, kdb patches
that I should have pushed harder for so I'm sympathetic here ;-)



Hey, if you do happen to have something, why not send it along.  I just asked 
the linux-next folks to re-activate the kgdb-next branch so that merges can 
start taking place again.  It appears that the final merge request never even 
went through from linux-next as I just rebased it and there was a patch in 
there from Daniel dating back to over a year ago.

If there is work out there that needs merging and reviews lets get it done.  I 
had been kind of stuck in time on what seems like the stone age 3.14 kernel 
projects until recently, but jumped back into mainline development about a 
month ago.

I have no objection to merging the ARM64 single step patch and have comments in 
a separate mail that follows with respect to the patch.


Cheers,
Jason.

Re: linux-next: removal of the kgdb tree

[Jason Wessel]

On 03/08/2016 11:16 PM, Stephen Rothwell wrote:

Hi Jason,

I noticed that the kgdb tree

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git branch 
kgdb-next

has not been updated since March 2015.  I am going to remove it from
linux-next tomorrow unless I hear that it may be useful.  It can always
be easily added back if it proves useful in the future.



The time has finally arrived.  :-)

Could you please add the kgdb-next branch back to linux-next.

Thanks,
Jason.

Re: linux-next: removal of the kgdb tree

[Jason Wessel]

On 03/08/2016 11:16 PM, Stephen Rothwell wrote:

Hi Jason,

I noticed that the kgdb tree

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git branch 
kgdb-next

has not been updated since March 2015.  I am going to remove it from
linux-next tomorrow unless I hear that it may be useful.  It can always
be easily added back if it proves useful in the future.



The time has finally arrived.  :-)

Could you please add the kgdb-next branch back to linux-next.

Thanks,
Jason.

Re: [PATCH -repost] kgdb: depends on VT

[Jason Wessel]

On 03/31/2016 03:29 AM, Jiri Slaby wrote:

With VT=n, the kernel build fails with:
drivers/built-in.o: In function `kgdboc_pre_exp_handler':
kgdboc.c:(.text+0x7b5aa): undefined reference to `fg_console'
kgdboc.c:(.text+0x7b5ce): undefined reference to `vc_cons'
kgdboc.c:(.text+0x7b5d5): undefined reference to `vc_cons'

kgdboc.o is built when KGDB_SERIAL_CONSOLE is set. So make
KGDB_SERIAL_CONSOLE depend on HW_CONSOLE which includes those symbols.


Acked-by: Jason Wessel <jason.wes...@windriver.com>


I'll put this in the kgdb-next branch and submit it to the upstream, unless it 
gets merged in another queue.

Thanks!
Jason.

Re: [PATCH -repost] kgdb: depends on VT

[Jason Wessel]

On 03/31/2016 03:29 AM, Jiri Slaby wrote:

With VT=n, the kernel build fails with:
drivers/built-in.o: In function `kgdboc_pre_exp_handler':
kgdboc.c:(.text+0x7b5aa): undefined reference to `fg_console'
kgdboc.c:(.text+0x7b5ce): undefined reference to `vc_cons'
kgdboc.c:(.text+0x7b5d5): undefined reference to `vc_cons'

kgdboc.o is built when KGDB_SERIAL_CONSOLE is set. So make
KGDB_SERIAL_CONSOLE depend on HW_CONSOLE which includes those symbols.


Acked-by: Jason Wessel 


I'll put this in the kgdb-next branch and submit it to the upstream, unless it 
gets merged in another queue.

Thanks!
Jason.

Re: [PATCH 4/4] drivers/misc: make kgdbts.c slightly more explicitly non-modular

[Jason Wessel]

On 08/08/2015 03:35 PM, Paul Gortmaker wrote:

The Kconfig currently controlling compilation of this code is:

lib/Kconfig.kgdb:config KGDB_TESTS
lib/Kconfig.kgdb:   bool "KGDB: internal test suite"

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

We can't remove the module.h include since we've kept the use of
module_param in this file for now.



This is correct, if you remove that there is no way to invoke the test suite 
later on at run time.

I tried out the patch and it works fine with no regressions.

Unrelated to this it seems there is a problem with the read/write of the break points 
when crossing the point where the kernel write protects the read-only data.  Basically, 
the break point is implanted into the code page, which is made read-only, and then it can 
no longer be removed.  What we typically do in that case "after read-only pages are 
established", is to use COW pages for the break points.  I'll have to look further 
into what if anything we might have to do about it. At least the emergency printk logic 
worked to show there is a problem. :-)
    
Acked-by: Jason Wessel 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 4/4] drivers/misc: make kgdbts.c slightly more explicitly non-modular

[Jason Wessel]

On 08/08/2015 03:35 PM, Paul Gortmaker wrote:

The Kconfig currently controlling compilation of this code is:

lib/Kconfig.kgdb:config KGDB_TESTS
lib/Kconfig.kgdb:   bool KGDB: internal test suite

...meaning that it currently is not being built as a module by anyone.

Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.

Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit.

We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.

We can't remove the module.h include since we've kept the use of
module_param in this file for now.



This is correct, if you remove that there is no way to invoke the test suite 
later on at run time.

I tried out the patch and it works fine with no regressions.

Unrelated to this it seems there is a problem with the read/write of the break points 
when crossing the point where the kernel write protects the read-only data.  Basically, 
the break point is implanted into the code page, which is made read-only, and then it can 
no longer be removed.  What we typically do in that case after read-only pages are 
established, is to use COW pages for the break points.  I'll have to look further 
into what if anything we might have to do about it. At least the emergency printk logic 
worked to show there is a problem. :-)

Acked-by: Jason Wessel jason.wes...@windriver.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kdb: Fix handling of kallsyms_symbol_next() return value

[Jason Wessel]

On 03/02/2015 08:13 AM, Daniel Thompson wrote:
> kallsyms_symbol_next() returns a boolean (true on success). Currently
> kdb_read() tests the return value with an inequality that
> unconditionally evaluates to true.
> 
> This is fixed in the obvious way and, since the conditional branch is
> supposed to be unreachable, we also add a WARN_ON().
> 


Thanks.  Applied to kgdb-next.  I'll send it along to the upstream
before the end of the v4.0 cycle, we'll see if we pickup any other
fixes along the way. :-)

Cheers,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] kdb: Fix handling of kallsyms_symbol_next() return value

[Jason Wessel]

On 03/02/2015 08:13 AM, Daniel Thompson wrote:
 kallsyms_symbol_next() returns a boolean (true on success). Currently
 kdb_read() tests the return value with an inequality that
 unconditionally evaluates to true.
 
 This is fixed in the obvious way and, since the conditional branch is
 supposed to be unreachable, we also add a WARN_ON().
 


Thanks.  Applied to kgdb-next.  I'll send it along to the upstream
before the end of the v4.0 cycle, we'll see if we pickup any other
fixes along the way. :-)

Cheers,
Jason.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.20-rc1

[Jason Wessel]

Linus,

Please pull the kgdb tree.

Summary of changes:

New:
   * KDB: improved searching
   * No longer enter debug core on panic if panic timeout is set

KGDB/KDB regressions / cleanups
   * fix pdf doc build errors
   * prevent junk characters on kdb console from printk levels

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.20-rc1

Thanks,
Jason.

The following changes since commit bfa76d49576599a4b9f9b7a71f23d73d6dcff735:

  Linux 3.19 (2015-02-08 18:54:22 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linux-3.20-rc1

for you to fetch changes up to dd8f30cc0550f36861ddfa42c27cc5c580b0bf8c:

  kgdb, docs: Fix  pdfdocs build errors (2015-02-19 12:39:04 -0600)


KGDB/KDB New:
   * KDB: improved searching
   * No longer enter debug core on panic if panic timeout is set

KGDB/KDB regressions / cleanups
   * fix pdf doc build errors
   * prevent junk characters on kdb console from printk levels


Colin Cross (1):
  debug: prevent entering debug mode on panic/exception.

Daniel Thompson (5):
  kdb: Avoid printing KERN_ levels to consoles
  kdb: Remove stack dump when entering kgdb due to NMI
  kdb: Fix a prompt management bug when using | grep
  kdb: Provide forward search at more prompt
  kdb: Const qualifier for kdb_getstr's prompt argument

Jason Wessel (1):
  kdb: Fix off by one error in kdb_cpu()

Jay Lan (1):
  kdb: fix incorrect counts in KDB summary command output

Rajaneesh Acharya (1):
  kgdb, docs: Fix  pdfdocs build errors

 Documentation/DocBook/kgdb.tmpl |  6 +++---
 include/linux/kdb.h |  8 +++-
 kernel/debug/debug_core.c   | 19 ++-
 kernel/debug/kdb/kdb_io.c   | 46 
++
 kernel/debug/kdb/kdb_main.c | 16 
 kernel/debug/kdb/kdb_private.h  |  4 +++-
 kernel/printk/printk.c  |  2 +-
 7 files changed, 74 insertions(+), 27 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.20-rc1

[Jason Wessel]

Linus,

Please pull the kgdb tree.

Summary of changes:

New:
   * KDB: improved searching
   * No longer enter debug core on panic if panic timeout is set

KGDB/KDB regressions / cleanups
   * fix pdf doc build errors
   * prevent junk characters on kdb console from printk levels

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.20-rc1

Thanks,
Jason.

The following changes since commit bfa76d49576599a4b9f9b7a71f23d73d6dcff735:

  Linux 3.19 (2015-02-08 18:54:22 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linux-3.20-rc1

for you to fetch changes up to dd8f30cc0550f36861ddfa42c27cc5c580b0bf8c:

  kgdb, docs: Fix para pdfdocs build errors (2015-02-19 12:39:04 -0600)


KGDB/KDB New:
   * KDB: improved searching
   * No longer enter debug core on panic if panic timeout is set

KGDB/KDB regressions / cleanups
   * fix pdf doc build errors
   * prevent junk characters on kdb console from printk levels


Colin Cross (1):
  debug: prevent entering debug mode on panic/exception.

Daniel Thompson (5):
  kdb: Avoid printing KERN_ levels to consoles
  kdb: Remove stack dump when entering kgdb due to NMI
  kdb: Fix a prompt management bug when using | grep
  kdb: Provide forward search at more prompt
  kdb: Const qualifier for kdb_getstr's prompt argument

Jason Wessel (1):
  kdb: Fix off by one error in kdb_cpu()

Jay Lan (1):
  kdb: fix incorrect counts in KDB summary command output

Rajaneesh Acharya (1):
  kgdb, docs: Fix para pdfdocs build errors

 Documentation/DocBook/kgdb.tmpl |  6 +++---
 include/linux/kdb.h |  8 +++-
 kernel/debug/debug_core.c   | 19 ++-
 kernel/debug/kdb/kdb_io.c   | 46 
++
 kernel/debug/kdb/kdb_main.c | 16 
 kernel/debug/kdb/kdb_private.h  |  4 +++-
 kernel/printk/printk.c  |  2 +-
 7 files changed, 74 insertions(+), 27 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.19-rc4

[Jason Wessel]

Linus,

Please pull the kgdb tree bug fixes.

These have been around since 3.17 and in kgdb-next for the last 9
weeks and some will go back to -stable.

Summary of changes:

KGDB/KDB fixes and cleanups
 Cleanups
   kdb: Remove unused command flags, repeat flags and KDB_REPEAT_NONE

 Fixes
   kgdb/kdb: Allow access on a single core, if a CPU round up is deemed
  impossible, which will allow inspection of the now "trashed" kernel
   kdb: Add enable mask for the command groups
   kdb: access controls to restrict sensitive commands 


git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.19-rc4

Thanks,
Jason.

The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:

  Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)

are available in the git repository at:

  . kgdb-next

for you to fetch changes up to 0f16996cf2ed7c368dd95b4c517ce572b96a10f5:

  kernel/debug/debug_core.c: Logging clean-up (2014-11-11 09:31:53 -0600)


Anton Vorontsov (6):
  kdb: Remove currently unused kdbtab_t->cmd_flags
  kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags
  kdb: Rename kdb_register_repeat() to kdb_register_flags()
  kdb: Use KDB_REPEAT_* values as flags
  kdb: Remove KDB_REPEAT_NONE flag
  kdb: Add enable mask for groups of commands

Daniel Thompson (3):
  kdb: Categorize kdb commands (similar to SysRq categorization)
  kdb: Allow access to sensitive commands to be restricted by default
  kgdb: timeout if secondary CPUs ignore the roundup

Fabian Frederick (1):
  kernel/debug/debug_core.c: Logging clean-up

 include/linux/kdb.h |  62 --
 kernel/debug/debug_core.c   |  52 
 kernel/debug/kdb/kdb_bp.c   |  37 +++---
 kernel/debug/kdb/kdb_debugger.c |   4 +
 kernel/debug/kdb/kdb_main.c | 267 +---
 kernel/debug/kdb/kdb_private.h  |   3 +-
 kernel/trace/trace_kdb.c|   4 +-
 lib/Kconfig.kgdb|  25 
 8 files changed, 306 insertions(+), 148 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.19-rc4

[Jason Wessel]

Linus,

Please pull the kgdb tree bug fixes.

These have been around since 3.17 and in kgdb-next for the last 9
weeks and some will go back to -stable.

Summary of changes:

KGDB/KDB fixes and cleanups
 Cleanups
   kdb: Remove unused command flags, repeat flags and KDB_REPEAT_NONE

 Fixes
   kgdb/kdb: Allow access on a single core, if a CPU round up is deemed
  impossible, which will allow inspection of the now trashed kernel
   kdb: Add enable mask for the command groups
   kdb: access controls to restrict sensitive commands 


git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.19-rc4

Thanks,
Jason.

The following changes since commit 206c5f60a3d902bc4b56dab2de3e88de5eb06108:

  Linux 3.18-rc4 (2014-11-09 14:55:29 -0800)

are available in the git repository at:

  . kgdb-next

for you to fetch changes up to 0f16996cf2ed7c368dd95b4c517ce572b96a10f5:

  kernel/debug/debug_core.c: Logging clean-up (2014-11-11 09:31:53 -0600)


Anton Vorontsov (6):
  kdb: Remove currently unused kdbtab_t-cmd_flags
  kdb: Rename kdb_repeat_t to kdb_cmdflags_t, cmd_repeat to cmd_flags
  kdb: Rename kdb_register_repeat() to kdb_register_flags()
  kdb: Use KDB_REPEAT_* values as flags
  kdb: Remove KDB_REPEAT_NONE flag
  kdb: Add enable mask for groups of commands

Daniel Thompson (3):
  kdb: Categorize kdb commands (similar to SysRq categorization)
  kdb: Allow access to sensitive commands to be restricted by default
  kgdb: timeout if secondary CPUs ignore the roundup

Fabian Frederick (1):
  kernel/debug/debug_core.c: Logging clean-up

 include/linux/kdb.h |  62 --
 kernel/debug/debug_core.c   |  52 
 kernel/debug/kdb/kdb_bp.c   |  37 +++---
 kernel/debug/kdb/kdb_debugger.c |   4 +
 kernel/debug/kdb/kdb_main.c | 267 +---
 kernel/debug/kdb/kdb_private.h  |   3 +-
 kernel/trace/trace_kdb.c|   4 +-
 lib/Kconfig.kgdb|  25 
 8 files changed, 306 insertions(+), 148 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kgdb-next not pushed to Linus for 3.19

[Jason Wessel]

On 01/06/2015 03:21 PM, Andrew Morton wrote:
> On Tue, 06 Jan 2015 10:57:42 -0800 Joe Perches  wrote:
>
>> On Tue, 2015-01-06 at 18:54 +, Daniel Thompson wrote:
>>> Hi Jason
>>>
>>> I'm trying to figure out what to do with my long-outstanding kgdb/kdb
>>> patches in preparation for the 3.20 merge window.
>>>
>>> As of now I have five pending patch sets some of which are well over six
>>> months old (and none have nay outstanding review comments).
>>>
>>> When I raised this with you a couple of months ago, two of the patch
>>> sets did land in your kgdb-next tree. However nothing seems to have
>>> happened since then and I couldn't find any messages from you during the
>>> 3.19 merge window.
>>>
>>> I'm afraid I don't know what else I need to do to progress things.
>>>
>>> I do plan to do routine rebasing and resending of my patchsets but,
>>> based on the past experience, that seems unlikely to be enough to get
>>> the code delivered in 3.20.
>>>
>>> Do you think I would be better sending these patches via someone else?
>>> In any case, advice would be very welcome.
>> Andrew?
>>
>> I think Daniel's kgdb patches are bug fixes.
>>
>> Can you please pick them up?
> yup.  Merging patches which are already in -next is a bit of a pain,
> but I'll cope.
>
> Daniel, can you please resend everything in a nice clean coherent
> stream?
>

I did not mean to miss the merge window, but I ended up being out the majority 
of December -> yesterday.

Now that I am back, I don't think you have to burden Andrew here.  I'll send a 
pull request for what is in kgdb-next since it is cleanups and fixes, and 
regression test anything else Daniel has left.

Cheers,
Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kgdb-next not pushed to Linus for 3.19

[Jason Wessel]

On 01/06/2015 03:21 PM, Andrew Morton wrote:
 On Tue, 06 Jan 2015 10:57:42 -0800 Joe Perches j...@perches.com wrote:

 On Tue, 2015-01-06 at 18:54 +, Daniel Thompson wrote:
 Hi Jason

 I'm trying to figure out what to do with my long-outstanding kgdb/kdb
 patches in preparation for the 3.20 merge window.

 As of now I have five pending patch sets some of which are well over six
 months old (and none have nay outstanding review comments).

 When I raised this with you a couple of months ago, two of the patch
 sets did land in your kgdb-next tree. However nothing seems to have
 happened since then and I couldn't find any messages from you during the
 3.19 merge window.

 I'm afraid I don't know what else I need to do to progress things.

 I do plan to do routine rebasing and resending of my patchsets but,
 based on the past experience, that seems unlikely to be enough to get
 the code delivered in 3.20.

 Do you think I would be better sending these patches via someone else?
 In any case, advice would be very welcome.
 Andrew?

 I think Daniel's kgdb patches are bug fixes.

 Can you please pick them up?
 yup.  Merging patches which are already in -next is a bit of a pain,
 but I'll cope.

 Daniel, can you please resend everything in a nice clean coherent
 stream?


I did not mean to miss the merge window, but I ended up being out the majority 
of December - yesterday.

Now that I am back, I don't think you have to burden Andrew here.  I'll send a 
pull request for what is in kgdb-next since it is cleanups and fixes, and 
regression test anything else Daniel has left.

Cheers,
Jason.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RESEND PATCH v5 3.16-rc4 0/8] kdb: Allow selective reduction in capabilities

[Jason Wessel]

On 07/11/2014 06:33 AM, Daniel Thompson wrote:
> This patchset implements restricted modes for the KDB debugger. It is a
> continuation of previous work of Anton Vorontsov. There are no
> outstanding review comments for this patchset.
>
> It provides a means for the root user to choose the set of kdb commands
> that are available on the kdb console. It is implemented similarly to
> the existing code to mask the available magic SysRq commands with modes
> for disable-all (0), enable-all(1) and a bitmask to enable/disable
> groups of functionality.
>
> The implementation of the mask check includes a feature to allow a
> command to change which group it belongs to based on whether or not the
> command has arguments (for example, go without arguments is a very safe
> command whilst go with an argument allows arbitrary changes to the
> program counter).
>
> There are a few patches, some are just cleanups, some are churn-ish
> cleanups, but inevitable. And the rest implements the mode -- after all
> the preparations, everything is pretty straightforward.

I plan to take one more walk through the code and aim to get this merged for 
the upcoming merge window.  :-)

Cheers,
Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RESEND PATCH v5 3.16-rc4 0/8] kdb: Allow selective reduction in capabilities

[Jason Wessel]

On 07/11/2014 06:33 AM, Daniel Thompson wrote:
 This patchset implements restricted modes for the KDB debugger. It is a
 continuation of previous work of Anton Vorontsov. There are no
 outstanding review comments for this patchset.

 It provides a means for the root user to choose the set of kdb commands
 that are available on the kdb console. It is implemented similarly to
 the existing code to mask the available magic SysRq commands with modes
 for disable-all (0), enable-all(1) and a bitmask to enable/disable
 groups of functionality.

 The implementation of the mask check includes a feature to allow a
 command to change which group it belongs to based on whether or not the
 command has arguments (for example, go without arguments is a very safe
 command whilst go with an argument allows arbitrary changes to the
 program counter).

 There are a few patches, some are just cleanups, some are churn-ish
 cleanups, but inevitable. And the rest implements the mode -- after all
 the preparations, everything is pretty straightforward.

I plan to take one more walk through the code and aim to get this merged for 
the upcoming merge window.  :-)

Cheers,
Jason.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] kgdb: Timeout if secondary CPUs ignore the roundup

[Jason Wessel]

On 07/01/2014 09:16 AM, Daniel Thompson wrote:
> Currently if an active CPU fails to respond to a roundup request the
> CPU that requested the roundup will become stuck. This needlessly
> reduces the robustness of the debugger.
>
> This patch introduces a timeout allowing the system state to be examined
> even when the system contains unresponsive processors. It also modifies
> kdb's cpu command to make it censor attempts to switch to unresponsive
> processors and to report their state as (D)ead.


It seems reasonable to allow entry on the master core because there certainly 
could be useful information to be had with respect to how you got there in the 
first place, but I wonder about the case for resuming the system.   In general 
if you couldn't sync in the the first place, the system is dead.  My opinion is 
that we probably should explicitly disallow a resume or single step at that 
point.

Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC PATCH] kgdb: Timeout if secondary CPUs ignore the roundup

[Jason Wessel]

On 07/01/2014 09:16 AM, Daniel Thompson wrote:
 Currently if an active CPU fails to respond to a roundup request the
 CPU that requested the roundup will become stuck. This needlessly
 reduces the robustness of the debugger.

 This patch introduces a timeout allowing the system state to be examined
 even when the system contains unresponsive processors. It also modifies
 kdb's cpu command to make it censor attempts to switch to unresponsive
 processors and to report their state as (D)ead.


It seems reasonable to allow entry on the master core because there certainly 
could be useful information to be had with respect to how you got there in the 
first place, but I wonder about the case for resuming the system.   In general 
if you couldn't sync in the the first place, the system is dead.  My opinion is 
that we probably should explicitly disallow a resume or single step at that 
point.

Jason.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] kernel/debug/debug_core.c: Logging clean-up

[Jason Wessel]

On 06/12/2014 01:48 PM, Fabian Frederick wrote:
> -Convert printk( to pr_foo()
> -Add pr_fmt
> -Coalesce formats
>
> Cc: Jason Wessel 
> Cc: Andrew Morton 
> Signed-off-by: Fabian Frederick 


Thanks.

I looked through all the bits to make sure there were no cases were we
relied on a printk out to the external debug client and everything is
all good.  This even prefixes the KGDB: in one place it was missed as
well as implicitly fixing all the kgdb: -> KGDB: inconsistencies.

The core functionality of kgdb is not changed of course.

I'll put this into the merge queue.

Reviewed-by: Jason Wessel 

Cheers,
Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] kernel/debug/debug_core.c: Logging clean-up

[Jason Wessel]

On 06/12/2014 01:48 PM, Fabian Frederick wrote:
 -Convert printk( to pr_foo()
 -Add pr_fmt
 -Coalesce formats

 Cc: Jason Wessel jason.wes...@windriver.com
 Cc: Andrew Morton a...@linux-foundation.org
 Signed-off-by: Fabian Frederick f...@skynet.be


Thanks.

I looked through all the bits to make sure there were no cases were we
relied on a printk out to the external debug client and everything is
all good.  This even prefixes the KGDB: in one place it was missed as
well as implicitly fixing all the kgdb: - KGDB: inconsistencies.

The core functionality of kgdb is not changed of course.

I'll put this into the merge queue.

Reviewed-by: Jason Wessel jason.wes...@windriver.com

Cheers,
Jason.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] serial: core: Consistent LF handling for poll_put_char

[Jason Wessel]

On 05/14/2014 09:55 AM, Daniel Thompson wrote:
> The behaviour of the UART poll_put_char infrastructure is inconsistent
> with respect to linefeed conversions. This in turn leads to difficulty
> using kdb on serial ports that are not also consoles
> (e.g. console=ttyAMA0,115200 kgdboc=ttyAMA1,115200).
>
> The following drivers automatically convert '\n' to '\r\n' inside their
> own poll functions but the remaining seventeen do not:
>
> serial8250, cpm, pch_uart, serial_pxa, serial_txx9,
>
> This can be made fully consistent but performing the conversion in
> uart_poll_put_char(). A similar conversion is already made inside
> uart_console_write() but it is optional for drivers to use this
> function. Fortunately we can be confident the translation is safe
> because the (very common) 8250 already does this translation.


I'll have to take a look at some of the other drivers.  If all the instances of 
the function calls are going to coded per driver, it might make more sense to 
add variable to struct uart_port, vs changing the number of arguments to 
uart_poll_put_char.  And then the default can simply be coded in the struct 
initialization to the most common need.

Cheers,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/4] serial: core: Consistent LF handling for poll_put_char

[Jason Wessel]

On 05/14/2014 09:55 AM, Daniel Thompson wrote:
 The behaviour of the UART poll_put_char infrastructure is inconsistent
 with respect to linefeed conversions. This in turn leads to difficulty
 using kdb on serial ports that are not also consoles
 (e.g. console=ttyAMA0,115200 kgdboc=ttyAMA1,115200).

 The following drivers automatically convert '\n' to '\r\n' inside their
 own poll functions but the remaining seventeen do not:

 serial8250, cpm, pch_uart, serial_pxa, serial_txx9,

 This can be made fully consistent but performing the conversion in
 uart_poll_put_char(). A similar conversion is already made inside
 uart_console_write() but it is optional for drivers to use this
 function. Fortunately we can be confident the translation is safe
 because the (very common) 8250 already does this translation.


I'll have to take a look at some of the other drivers.  If all the instances of 
the function calls are going to coded per driver, it might make more sense to 
add variable to struct uart_port, vs changing the number of arguments to 
uart_poll_put_char.  And then the default can simply be coded in the struct 
initialization to the most common need.

Cheers,
Jason.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/7] KGDB/KDB: add support for external NMI handler to call KGDB/KDB.

[Jason Wessel]

On 09/23/2013 04:25 PM, Mike Travis wrote:
> This patch adds a kgdb_nmicallin() interface that can be used by
> external NMI handlers to call the KGDB/KDB handler.  The primary need
> for this is for those types of NMI interrupts where all the CPUs
> have already received the NMI signal.  Therefore no send_IPI(NMI)
> is required, and in fact it will cause a 2nd unhandled NMI to occur.
> This generates the "Dazed and Confuzed" messages.
> 
> Since all the CPUs are getting the NMI at roughly the same time, it's not
> guaranteed that the first CPU that hits the NMI handler will manage to
> enter KGDB and set the dbg_master_lock before the slaves start entering.
> The new argument "send_ready" was added for KGDB to signal the NMI handler
> to release the slave CPUs for entry into KGDB.
> 
> Signed-off-by: Mike Travis 
> Reviewed-by: Dimitri Sivanich 
> Reviewed-by: Hedi Berriche 


One problem that I pointed out before and then you can add 

Acked-by: Jason Wessel 


> ---
>  include/linux/kdb.h |1 +
>  include/linux/kgdb.h|1 +
>  kernel/debug/debug_core.c   |   30 +-
>  kernel/debug/debug_core.h   |1 +
>  kernel/debug/kdb/kdb_debugger.c |5 -
>  kernel/debug/kdb/kdb_main.c |3 +++
>  6 files changed, 39 insertions(+), 2 deletions(-)
> 
> --- linux.orig/include/linux/kdb.h
> +++ linux/include/linux/kdb.h
> @@ -109,6 +109,7 @@ typedef enum {
>   KDB_REASON_RECURSE, /* Recursive entry to kdb;
>* regs probably valid */
>   KDB_REASON_SSTEP,   /* Single Step trap. - regs valid */
> + KDB_REASON_SYSTEM_NMI,  /* In NMI due to SYSTEM cmd; regs valid */
>  } kdb_reason_t;
>  
>  extern int kdb_trap_printk;
> --- linux.orig/include/linux/kgdb.h
> +++ linux/include/linux/kgdb.h
> @@ -310,6 +310,7 @@ extern int
>  kgdb_handle_exception(int ex_vector, int signo, int err_code,
> struct pt_regs *regs);
>  extern int kgdb_nmicallback(int cpu, void *regs);
> +extern int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t 
> *snd_rdy);
>  extern void gdbstub_exit(int status);
>  
>  extern int   kgdb_single_step;
> --- linux.orig/kernel/debug/debug_core.c
> +++ linux/kernel/debug/debug_core.c
> @@ -575,8 +575,12 @@ return_normal:
>   raw_spin_lock(_slave_lock);
>  
>  #ifdef CONFIG_SMP
> + /* If SYSTEM_NMI, slaves are already waiting */
> + if (ks->err_code == KDB_REASON_SYSTEM_NMI)
> + atomic_set(ks->send_ready, 1);
> +
>   /* Signal the other CPUs to enter kgdb_wait() */
> - if ((!kgdb_single_step) && kgdb_do_roundup)
> + else if ((!kgdb_single_step) && kgdb_do_roundup)
>   kgdb_roundup_cpus(flags);
>  #endif
>  
> @@ -729,6 +733,30 @@ int kgdb_nmicallback(int cpu, void *regs
>   return 0;
>   }
>  #endif
> + return 1;
> +}
> +
> +int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t *send_ready)
> +{
> +#ifdef CONFIG_SMP
> + if (!kgdb_io_ready(0) || !send_ready)
> + return 1;
> +
> + if (kgdb_info[cpu].enter_kgdb == 0) {
> + struct kgdb_state kgdb_var;
> + struct kgdb_state *ks = _var;
> +
> + memset(ks, 0, sizeof(struct kgdb_state));
> + ks->cpu = cpu;
> + ks->ex_vector   = trapnr;
> + ks->signo   = SIGTRAP;
> + ks->err_code= KDB_REASON_SYSTEM_NMI;
> + ks->linux_regs  = regs;
> + ks->send_ready  = send_ready;
> + kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> + return 0;
> + }
> +#endif
>   return 1;
>  }
>  
> --- linux.orig/kernel/debug/debug_core.h
> +++ linux/kernel/debug/debug_core.h
> @@ -26,6 +26,7 @@ struct kgdb_state {
>   unsigned long   threadid;
>   longkgdb_usethreadid;
>   struct pt_regs  *linux_regs;
> + atomic_t*send_ready;
>  };
>  
>  /* Exception state values */
> --- linux.orig/kernel/debug/kdb/kdb_debugger.c
> +++ linux/kernel/debug/kdb/kdb_debugger.c
> @@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
>   if (atomic_read(_setting_breakpoint))
>   reason = KDB_REASON_KEYBOARD;
>  
> - if (in_nmi())
> + if (ks->err_code == KDB_REASON_SYSTEM_NMI)


This should be:

(ks->err == KDB_REASON_SYSNMI && ks->signo == SIGTRAP)

Other arch's in their arch specific handling code may set ks->err differently.  
The err code is signal spe

Re: [PATCH 5/7] KGDB/KDB: add support for external NMI handler to call KGDB/KDB.

[Jason Wessel]

On 09/23/2013 04:25 PM, Mike Travis wrote:
 This patch adds a kgdb_nmicallin() interface that can be used by
 external NMI handlers to call the KGDB/KDB handler.  The primary need
 for this is for those types of NMI interrupts where all the CPUs
 have already received the NMI signal.  Therefore no send_IPI(NMI)
 is required, and in fact it will cause a 2nd unhandled NMI to occur.
 This generates the Dazed and Confuzed messages.
 
 Since all the CPUs are getting the NMI at roughly the same time, it's not
 guaranteed that the first CPU that hits the NMI handler will manage to
 enter KGDB and set the dbg_master_lock before the slaves start entering.
 The new argument send_ready was added for KGDB to signal the NMI handler
 to release the slave CPUs for entry into KGDB.
 
 Signed-off-by: Mike Travis tra...@sgi.com
 Reviewed-by: Dimitri Sivanich sivan...@sgi.com
 Reviewed-by: Hedi Berriche h...@sgi.com


One problem that I pointed out before and then you can add 

Acked-by: Jason Wessel jason.wes...@windriver.com


 ---
  include/linux/kdb.h |1 +
  include/linux/kgdb.h|1 +
  kernel/debug/debug_core.c   |   30 +-
  kernel/debug/debug_core.h   |1 +
  kernel/debug/kdb/kdb_debugger.c |5 -
  kernel/debug/kdb/kdb_main.c |3 +++
  6 files changed, 39 insertions(+), 2 deletions(-)
 
 --- linux.orig/include/linux/kdb.h
 +++ linux/include/linux/kdb.h
 @@ -109,6 +109,7 @@ typedef enum {
   KDB_REASON_RECURSE, /* Recursive entry to kdb;
* regs probably valid */
   KDB_REASON_SSTEP,   /* Single Step trap. - regs valid */
 + KDB_REASON_SYSTEM_NMI,  /* In NMI due to SYSTEM cmd; regs valid */
  } kdb_reason_t;
  
  extern int kdb_trap_printk;
 --- linux.orig/include/linux/kgdb.h
 +++ linux/include/linux/kgdb.h
 @@ -310,6 +310,7 @@ extern int
  kgdb_handle_exception(int ex_vector, int signo, int err_code,
 struct pt_regs *regs);
  extern int kgdb_nmicallback(int cpu, void *regs);
 +extern int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t 
 *snd_rdy);
  extern void gdbstub_exit(int status);
  
  extern int   kgdb_single_step;
 --- linux.orig/kernel/debug/debug_core.c
 +++ linux/kernel/debug/debug_core.c
 @@ -575,8 +575,12 @@ return_normal:
   raw_spin_lock(dbg_slave_lock);
  
  #ifdef CONFIG_SMP
 + /* If SYSTEM_NMI, slaves are already waiting */
 + if (ks-err_code == KDB_REASON_SYSTEM_NMI)
 + atomic_set(ks-send_ready, 1);
 +
   /* Signal the other CPUs to enter kgdb_wait() */
 - if ((!kgdb_single_step)  kgdb_do_roundup)
 + else if ((!kgdb_single_step)  kgdb_do_roundup)
   kgdb_roundup_cpus(flags);
  #endif
  
 @@ -729,6 +733,30 @@ int kgdb_nmicallback(int cpu, void *regs
   return 0;
   }
  #endif
 + return 1;
 +}
 +
 +int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t *send_ready)
 +{
 +#ifdef CONFIG_SMP
 + if (!kgdb_io_ready(0) || !send_ready)
 + return 1;
 +
 + if (kgdb_info[cpu].enter_kgdb == 0) {
 + struct kgdb_state kgdb_var;
 + struct kgdb_state *ks = kgdb_var;
 +
 + memset(ks, 0, sizeof(struct kgdb_state));
 + ks-cpu = cpu;
 + ks-ex_vector   = trapnr;
 + ks-signo   = SIGTRAP;
 + ks-err_code= KDB_REASON_SYSTEM_NMI;
 + ks-linux_regs  = regs;
 + ks-send_ready  = send_ready;
 + kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
 + return 0;
 + }
 +#endif
   return 1;
  }
  
 --- linux.orig/kernel/debug/debug_core.h
 +++ linux/kernel/debug/debug_core.h
 @@ -26,6 +26,7 @@ struct kgdb_state {
   unsigned long   threadid;
   longkgdb_usethreadid;
   struct pt_regs  *linux_regs;
 + atomic_t*send_ready;
  };
  
  /* Exception state values */
 --- linux.orig/kernel/debug/kdb/kdb_debugger.c
 +++ linux/kernel/debug/kdb/kdb_debugger.c
 @@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
   if (atomic_read(kgdb_setting_breakpoint))
   reason = KDB_REASON_KEYBOARD;
  
 - if (in_nmi())
 + if (ks-err_code == KDB_REASON_SYSTEM_NMI)


This should be:

(ks-err == KDB_REASON_SYSNMI  ks-signo == SIGTRAP)

Other arch's in their arch specific handling code may set ks-err differently.  
The err code is signal specific.  For example:

arch/hexagon/kernel/kgdb.c:  if (kgdb_handle_exception(args-trapnr  0xff, 
args-signr, args-err,

The error passed in will be different for other signals and we do not want it 
to collide.

Cheers,
Jason.


 + reason = KDB_REASON_SYSTEM_NMI;
 +
 + else if (in_nmi())
   reason = KDB_REASON_NMI;
  
   for (i = 0, bp = kdb_breakpoints; i  KDB_MAXBPT; i++, bp++) {
 --- linux.orig/kernel/debug/kdb/kdb_main.c

Re: [PATCH 7/9] KGDB/KDB: add new system NMI entry code to KDB

[Jason Wessel]

On 09/05/2013 05:50 PM, Mike Travis wrote:
> This patch adds a new "KDB_REASON" code (KDB_REASON_SYSTEM_NMI).  This
> is purely cosmetic to distinguish it from the other various reasons that
> NMI may occur and are usually after an error occurred.  Also the dumping
> of registers is not done to more closely match what is displayed when KDB
> is entered manually via the sysreq 'g' key.


This patch is not quite right.   See below.


> 
> Signed-off-by: Mike Travis 
> Reviewed-by: Dimitri Sivanich 
> Reviewed-by: Hedi Berriche 
> ---
>  include/linux/kdb.h |1 +
>  include/linux/kgdb.h|1 +
>  kernel/debug/debug_core.c   |5 +
>  kernel/debug/kdb/kdb_debugger.c |5 -
>  kernel/debug/kdb/kdb_main.c |3 +++
>  5 files changed, 14 insertions(+), 1 deletion(-)
> 
> --- linux.orig/include/linux/kdb.h
> +++ linux/include/linux/kdb.h
> @@ -109,6 +109,7 @@ typedef enum {
>   KDB_REASON_RECURSE, /* Recursive entry to kdb;
>* regs probably valid */
>   KDB_REASON_SSTEP,   /* Single Step trap. - regs valid */
> + KDB_REASON_SYSTEM_NMI,  /* In NMI due to SYSTEM cmd; regs valid */
>  } kdb_reason_t;
>  
>  extern int kdb_trap_printk;
> --- linux.orig/include/linux/kgdb.h
> +++ linux/include/linux/kgdb.h
> @@ -52,6 +52,7 @@ extern int kgdb_connected;
>  extern int kgdb_io_module_registered;
>  
>  extern atomic_t  kgdb_setting_breakpoint;
> +extern atomic_t  kgdb_system_nmi;


We don't need extra atomics.  You should add another variable to the kgdb_state 
which is processor specific in this case.

Better yet, just set the ks->err_code properly in your kgdb_nmicallin() or in 
the origination call to kgdb_nmicallback() from your nmi handler (remember I 
still have the question pending if we actually need kgdb_nmicallin() in the 
first place.  You already did the work of adding another NMI type to the enum.  
We just need to use the ks->err_code variable as well.


>  extern atomic_t  kgdb_cpu_doing_single_step;
>  
>  extern struct task_struct*kgdb_usethread;
> --- linux.orig/kernel/debug/debug_core.c
> +++ linux/kernel/debug/debug_core.c
> @@ -125,6 +125,7 @@ static atomic_t   masters_in_kgdb;
>  static atomic_t  slaves_in_kgdb;
>  static atomic_t  kgdb_break_tasklet_var;
>  atomic_t kgdb_setting_breakpoint;
> +atomic_t kgdb_system_nmi;
>  
>  struct task_struct   *kgdb_usethread;
>  struct task_struct   *kgdb_contthread;
> @@ -760,7 +761,11 @@ int kgdb_nmicallin(int cpu, int trapnr,
>  
>   /* Indicate there are slaves waiting */
>   kgdb_info[cpu].send_ready = send_ready;
> +
> + /* Use new reason code "SYSTEM_NMI" */
> + atomic_inc(_system_nmi);
>   kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
> + atomic_dec(_system_nmi);
>   kgdb_do_roundup = save_kgdb_do_roundup;
>   kgdb_info[cpu].send_ready = NULL;
>  
> --- linux.orig/kernel/debug/kdb/kdb_debugger.c
> +++ linux/kernel/debug/kdb/kdb_debugger.c
> @@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
>   if (atomic_read(_setting_breakpoint))
>   reason = KDB_REASON_KEYBOARD;
>  
> - if (in_nmi())
> + if (atomic_read(_system_nmi))
> + reason = KDB_REASON_SYSTEM_NMI;


This would get changed to if (ks->err == KDB_REASON_SYSNMI && ks->signo == 
SIGTRAP) 

Cheers,
Jason.

> +
> + else if (in_nmi())
>   reason = KDB_REASON_NMI;
>  
>   for (i = 0, bp = kdb_breakpoints; i < KDB_MAXBPT; i++, bp++) {
> --- linux.orig/kernel/debug/kdb/kdb_main.c
> +++ linux/kernel/debug/kdb/kdb_main.c
> @@ -1200,6 +1200,9 @@ static int kdb_local(kdb_reason_t reason
>  instruction_pointer(regs));
>   kdb_dumpregs(regs);
>   break;
> + case KDB_REASON_SYSTEM_NMI:
> + kdb_printf("due to System NonMaskable Interrupt\n");
> + break;
>   case KDB_REASON_NMI:
>   kdb_printf("due to NonMaskable Interrupt @ "
>  kdb_machreg_fmt "\n",
> 
> -- 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/9] KGDB/KDB: add support for external NMI handler to call KGDB/KDB.

[Jason Wessel]

On 09/05/2013 05:50 PM, Mike Travis wrote:
> This patch adds a kgdb_nmicallin() interface that can be used by
> external NMI handlers to call the KGDB/KDB handler.  The primary need
> for this is for those types of NMI interrupts where all the CPUs
> have already received the NMI signal.  Therefore no send_IPI(NMI)
> is required, and in fact it will cause a 2nd unhandled NMI to occur.
> This generates the "Dazed and Confuzed" messages.
>
> Since all the CPUs are getting the NMI at roughly the same time, it's not
> guaranteed that the first CPU that hits the NMI handler will manage to
> enter KGDB and set the dbg_master_lock before the slaves start entering.

It should have been ok to have more than one master if this was some kind of 
watch dog.  The raw spin lock for the dbg_master_lock should have ensured that 
only a single CPU is in fact the master.  If it is the case that we cannot send 
a nested IPI at this point, the UV machine type should have replaced the 
kgdb_roundup_cpus() routine with something that will work, such as looking at 
the exception type on the way in and perhaps skipping the IPI send.

Also if there is no possibility of restarting the machine from this state it 
would have been possible to simply turn off kgdb_do_roundup in the custom 
kgdb_roundup_cpus().

The patch you created appears that it will work, but it comes at the cost of 
some complexity because you are also checking on the state of 
"kgdb_info[cpu].send_ready" in some other location in the NMI handler.  It 
might be better to consider not sending a nested NMI if all the CPUs are going 
to enter anyway in the master state.


>
> The new argument "send_ready" was added for KGDB to signal the NMI handler
> to release the slave CPUs for entry into KGDB.
>
> Signed-off-by: Mike Travis 
> Reviewed-by: Dimitri Sivanich 
> Reviewed-by: Hedi Berriche 
> ---
>  include/linux/kgdb.h  |1 +
>  kernel/debug/debug_core.c |   41 +
>  kernel/debug/debug_core.h |1 +
>  3 files changed, 43 insertions(+)
>
> --- linux.orig/include/linux/kgdb.h
> +++ linux/include/linux/kgdb.h
> @@ -310,6 +310,7 @@ extern int
>  kgdb_handle_exception(int ex_vector, int signo, int err_code,
>struct pt_regs *regs);
>  extern int kgdb_nmicallback(int cpu, void *regs);
> +extern int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t 
> *snd_rdy);
>  extern void gdbstub_exit(int status);
> 
>  extern intkgdb_single_step;
> --- linux.orig/kernel/debug/debug_core.c
> +++ linux/kernel/debug/debug_core.c
> @@ -578,6 +578,10 @@ return_normal:
>  /* Signal the other CPUs to enter kgdb_wait() */
>  if ((!kgdb_single_step) && kgdb_do_roundup)
>  kgdb_roundup_cpus(flags);
> +
> +/* If optional send ready pointer, signal CPUs to proceed */
> +if (kgdb_info[cpu].send_ready)
> +atomic_set(kgdb_info[cpu].send_ready, 1);
>  #endif
> 
>  /*
> @@ -729,6 +733,43 @@ int kgdb_nmicallback(int cpu, void *regs
>  return 0;
>  }
>  #endif
> +return 1;
> +}
> +
> +int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t *send_ready)
> +{
> +#ifdef CONFIG_SMP
> +if (!kgdb_io_ready(0))
> +return 1;
> +
> +if (kgdb_info[cpu].enter_kgdb == 0) {
> +struct kgdb_state kgdb_var;
> +struct kgdb_state *ks = _var;
> +int save_kgdb_do_roundup = kgdb_do_roundup;
> +
> +memset(ks, 0, sizeof(struct kgdb_state));
> +ks->cpu= cpu;
> +ks->ex_vector= trapnr;
> +ks->signo= SIGTRAP;
> +ks->err_code= 0;
> +ks->kgdb_usethreadid= 0;
> +ks->linux_regs= regs;
> +
> +/* Do not broadcast NMI */
> +kgdb_do_roundup = 0;
> +
> +/* Indicate there are slaves waiting */
> +kgdb_info[cpu].send_ready = send_ready;
> +kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);

This is the one part of the patch I don't quite understand.  Why does the 
kgdb_nmicallin() desire to be the master core?

It was not obvious the circumstance as to why this is called.  Is it some kind 
of watch dog where you really do want to enter the debugger or is it more to 
deal with nested slave interrupts were the round up would have possibly hung on 
this hardware.  If it is the later, I would have thought this should be a slave 
and not the master.

Perhaps a comment in the code can clear this up?

Thanks,
Jason.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 5/9] KGDB/KDB: add support for external NMI handler to call KGDB/KDB.

[Jason Wessel]

On 09/05/2013 05:50 PM, Mike Travis wrote:
 This patch adds a kgdb_nmicallin() interface that can be used by
 external NMI handlers to call the KGDB/KDB handler.  The primary need
 for this is for those types of NMI interrupts where all the CPUs
 have already received the NMI signal.  Therefore no send_IPI(NMI)
 is required, and in fact it will cause a 2nd unhandled NMI to occur.
 This generates the Dazed and Confuzed messages.

 Since all the CPUs are getting the NMI at roughly the same time, it's not
 guaranteed that the first CPU that hits the NMI handler will manage to
 enter KGDB and set the dbg_master_lock before the slaves start entering.

It should have been ok to have more than one master if this was some kind of 
watch dog.  The raw spin lock for the dbg_master_lock should have ensured that 
only a single CPU is in fact the master.  If it is the case that we cannot send 
a nested IPI at this point, the UV machine type should have replaced the 
kgdb_roundup_cpus() routine with something that will work, such as looking at 
the exception type on the way in and perhaps skipping the IPI send.

Also if there is no possibility of restarting the machine from this state it 
would have been possible to simply turn off kgdb_do_roundup in the custom 
kgdb_roundup_cpus().

The patch you created appears that it will work, but it comes at the cost of 
some complexity because you are also checking on the state of 
kgdb_info[cpu].send_ready in some other location in the NMI handler.  It 
might be better to consider not sending a nested NMI if all the CPUs are going 
to enter anyway in the master state.



 The new argument send_ready was added for KGDB to signal the NMI handler
 to release the slave CPUs for entry into KGDB.

 Signed-off-by: Mike Travis tra...@sgi.com
 Reviewed-by: Dimitri Sivanich sivan...@sgi.com
 Reviewed-by: Hedi Berriche hberr...@sgi.com
 ---
  include/linux/kgdb.h  |1 +
  kernel/debug/debug_core.c |   41 +
  kernel/debug/debug_core.h |1 +
  3 files changed, 43 insertions(+)

 --- linux.orig/include/linux/kgdb.h
 +++ linux/include/linux/kgdb.h
 @@ -310,6 +310,7 @@ extern int
  kgdb_handle_exception(int ex_vector, int signo, int err_code,
struct pt_regs *regs);
  extern int kgdb_nmicallback(int cpu, void *regs);
 +extern int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t 
 *snd_rdy);
  extern void gdbstub_exit(int status);
 
  extern intkgdb_single_step;
 --- linux.orig/kernel/debug/debug_core.c
 +++ linux/kernel/debug/debug_core.c
 @@ -578,6 +578,10 @@ return_normal:
  /* Signal the other CPUs to enter kgdb_wait() */
  if ((!kgdb_single_step)  kgdb_do_roundup)
  kgdb_roundup_cpus(flags);
 +
 +/* If optional send ready pointer, signal CPUs to proceed */
 +if (kgdb_info[cpu].send_ready)
 +atomic_set(kgdb_info[cpu].send_ready, 1);
  #endif
 
  /*
 @@ -729,6 +733,43 @@ int kgdb_nmicallback(int cpu, void *regs
  return 0;
  }
  #endif
 +return 1;
 +}
 +
 +int kgdb_nmicallin(int cpu, int trapnr, void *regs, atomic_t *send_ready)
 +{
 +#ifdef CONFIG_SMP
 +if (!kgdb_io_ready(0))
 +return 1;
 +
 +if (kgdb_info[cpu].enter_kgdb == 0) {
 +struct kgdb_state kgdb_var;
 +struct kgdb_state *ks = kgdb_var;
 +int save_kgdb_do_roundup = kgdb_do_roundup;
 +
 +memset(ks, 0, sizeof(struct kgdb_state));
 +ks-cpu= cpu;
 +ks-ex_vector= trapnr;
 +ks-signo= SIGTRAP;
 +ks-err_code= 0;
 +ks-kgdb_usethreadid= 0;
 +ks-linux_regs= regs;
 +
 +/* Do not broadcast NMI */
 +kgdb_do_roundup = 0;
 +
 +/* Indicate there are slaves waiting */
 +kgdb_info[cpu].send_ready = send_ready;
 +kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);

This is the one part of the patch I don't quite understand.  Why does the 
kgdb_nmicallin() desire to be the master core?

It was not obvious the circumstance as to why this is called.  Is it some kind 
of watch dog where you really do want to enter the debugger or is it more to 
deal with nested slave interrupts were the round up would have possibly hung on 
this hardware.  If it is the later, I would have thought this should be a slave 
and not the master.

Perhaps a comment in the code can clear this up?

Thanks,
Jason.


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 7/9] KGDB/KDB: add new system NMI entry code to KDB

[Jason Wessel]

On 09/05/2013 05:50 PM, Mike Travis wrote:
 This patch adds a new KDB_REASON code (KDB_REASON_SYSTEM_NMI).  This
 is purely cosmetic to distinguish it from the other various reasons that
 NMI may occur and are usually after an error occurred.  Also the dumping
 of registers is not done to more closely match what is displayed when KDB
 is entered manually via the sysreq 'g' key.


This patch is not quite right.   See below.


 
 Signed-off-by: Mike Travis tra...@sgi.com
 Reviewed-by: Dimitri Sivanich sivan...@sgi.com
 Reviewed-by: Hedi Berriche hberr...@sgi.com
 ---
  include/linux/kdb.h |1 +
  include/linux/kgdb.h|1 +
  kernel/debug/debug_core.c   |5 +
  kernel/debug/kdb/kdb_debugger.c |5 -
  kernel/debug/kdb/kdb_main.c |3 +++
  5 files changed, 14 insertions(+), 1 deletion(-)
 
 --- linux.orig/include/linux/kdb.h
 +++ linux/include/linux/kdb.h
 @@ -109,6 +109,7 @@ typedef enum {
   KDB_REASON_RECURSE, /* Recursive entry to kdb;
* regs probably valid */
   KDB_REASON_SSTEP,   /* Single Step trap. - regs valid */
 + KDB_REASON_SYSTEM_NMI,  /* In NMI due to SYSTEM cmd; regs valid */
  } kdb_reason_t;
  
  extern int kdb_trap_printk;
 --- linux.orig/include/linux/kgdb.h
 +++ linux/include/linux/kgdb.h
 @@ -52,6 +52,7 @@ extern int kgdb_connected;
  extern int kgdb_io_module_registered;
  
  extern atomic_t  kgdb_setting_breakpoint;
 +extern atomic_t  kgdb_system_nmi;


We don't need extra atomics.  You should add another variable to the kgdb_state 
which is processor specific in this case.

Better yet, just set the ks-err_code properly in your kgdb_nmicallin() or in 
the origination call to kgdb_nmicallback() from your nmi handler (remember I 
still have the question pending if we actually need kgdb_nmicallin() in the 
first place.  You already did the work of adding another NMI type to the enum.  
We just need to use the ks-err_code variable as well.


  extern atomic_t  kgdb_cpu_doing_single_step;
  
  extern struct task_struct*kgdb_usethread;
 --- linux.orig/kernel/debug/debug_core.c
 +++ linux/kernel/debug/debug_core.c
 @@ -125,6 +125,7 @@ static atomic_t   masters_in_kgdb;
  static atomic_t  slaves_in_kgdb;
  static atomic_t  kgdb_break_tasklet_var;
  atomic_t kgdb_setting_breakpoint;
 +atomic_t kgdb_system_nmi;
  
  struct task_struct   *kgdb_usethread;
  struct task_struct   *kgdb_contthread;
 @@ -760,7 +761,11 @@ int kgdb_nmicallin(int cpu, int trapnr,
  
   /* Indicate there are slaves waiting */
   kgdb_info[cpu].send_ready = send_ready;
 +
 + /* Use new reason code SYSTEM_NMI */
 + atomic_inc(kgdb_system_nmi);
   kgdb_cpu_enter(ks, regs, DCPU_WANT_MASTER);
 + atomic_dec(kgdb_system_nmi);
   kgdb_do_roundup = save_kgdb_do_roundup;
   kgdb_info[cpu].send_ready = NULL;
  
 --- linux.orig/kernel/debug/kdb/kdb_debugger.c
 +++ linux/kernel/debug/kdb/kdb_debugger.c
 @@ -69,7 +69,10 @@ int kdb_stub(struct kgdb_state *ks)
   if (atomic_read(kgdb_setting_breakpoint))
   reason = KDB_REASON_KEYBOARD;
  
 - if (in_nmi())
 + if (atomic_read(kgdb_system_nmi))
 + reason = KDB_REASON_SYSTEM_NMI;


This would get changed to if (ks-err == KDB_REASON_SYSNMI  ks-signo == 
SIGTRAP) 

Cheers,
Jason.

 +
 + else if (in_nmi())
   reason = KDB_REASON_NMI;
  
   for (i = 0, bp = kdb_breakpoints; i  KDB_MAXBPT; i++, bp++) {
 --- linux.orig/kernel/debug/kdb/kdb_main.c
 +++ linux/kernel/debug/kdb/kdb_main.c
 @@ -1200,6 +1200,9 @@ static int kdb_local(kdb_reason_t reason
  instruction_pointer(regs));
   kdb_dumpregs(regs);
   break;
 + case KDB_REASON_SYSTEM_NMI:
 + kdb_printf(due to System NonMaskable Interrupt\n);
 + break;
   case KDB_REASON_NMI:
   kdb_printf(due to NonMaskable Interrupt @ 
  kdb_machreg_fmt \n,
 
 -- 
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:perf/core] perf machine: Do not require /lib/modules/ * on a guest

[tip-bot for Jason Wessel]

Commit-ID:  8f76fcd902e3b3a7d6f6c695cc8bc053579eb179
Gitweb: http://git.kernel.org/tip/8f76fcd902e3b3a7d6f6c695cc8bc053579eb179
Author: Jason Wessel 
AuthorDate: Mon, 15 Jul 2013 15:27:53 -0500
Committer:  Arnaldo Carvalho de Melo 
CommitDate: Wed, 7 Aug 2013 17:35:41 -0300

perf machine: Do not require /lib/modules/* on a guest

For some types of work loads and special guest environments, you might
have a kernel that has no kernel modules.  The perf kvm record tool
fails instantiate vmlinux maps when the kernel modules directory cannot
be opened, even though the kallsyms has been properly processed.  This
leads to a perf kvm report that has no guest symbols resolved.

This patch changes the failure to locate kernel modules to be non-fatal.

Signed-off-by: Jason Wessel 
Acked-by: David Ahern 
Cc: Paul Mackerras 
Cc: Peter Zijlstra 
Link: 
http://lkml.kernel.org/r/1373920073-4874-1-git-send-email-jason.wes...@windriver.com
Signed-off-by: Arnaldo Carvalho de Melo 
---
 tools/perf/util/machine.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ef3b49c..6fcc358 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -806,7 +806,10 @@ static int machine__create_modules(struct machine *machine)
free(line);
fclose(file);
 
-   return machine__set_modules_path(machine);
+   if (machine__set_modules_path(machine) < 0) {
+   pr_debug("Problems setting modules path maps, continuing 
anyway...\n");
+   }
+   return 0;
 
 out_delete_line:
free(line);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[tip:perf/core] perf machine: Do not require /lib/modules/ * on a guest

[tip-bot for Jason Wessel]

Commit-ID:  8f76fcd902e3b3a7d6f6c695cc8bc053579eb179
Gitweb: http://git.kernel.org/tip/8f76fcd902e3b3a7d6f6c695cc8bc053579eb179
Author: Jason Wessel jason.wes...@windriver.com
AuthorDate: Mon, 15 Jul 2013 15:27:53 -0500
Committer:  Arnaldo Carvalho de Melo a...@redhat.com
CommitDate: Wed, 7 Aug 2013 17:35:41 -0300

perf machine: Do not require /lib/modules/* on a guest

For some types of work loads and special guest environments, you might
have a kernel that has no kernel modules.  The perf kvm record tool
fails instantiate vmlinux maps when the kernel modules directory cannot
be opened, even though the kallsyms has been properly processed.  This
leads to a perf kvm report that has no guest symbols resolved.

This patch changes the failure to locate kernel modules to be non-fatal.

Signed-off-by: Jason Wessel jason.wes...@windriver.com
Acked-by: David Ahern dsah...@gmail.com
Cc: Paul Mackerras pau...@samba.org
Cc: Peter Zijlstra pet...@infradead.org
Link: 
http://lkml.kernel.org/r/1373920073-4874-1-git-send-email-jason.wes...@windriver.com
Signed-off-by: Arnaldo Carvalho de Melo a...@redhat.com
---
 tools/perf/util/machine.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ef3b49c..6fcc358 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -806,7 +806,10 @@ static int machine__create_modules(struct machine *machine)
free(line);
fclose(file);
 
-   return machine__set_modules_path(machine);
+   if (machine__set_modules_path(machine)  0) {
+   pr_debug(Problems setting modules path maps, continuing 
anyway...\n);
+   }
+   return 0;
 
 out_delete_line:
free(line);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] perf: Do not require /lib/modules/* on a guest

[Jason Wessel]

For some types of work loads and special guest environments, you might
have a kernel that has no kernel modules.  The perf kvm record tool
fails instantiate vmlinux maps when the kernel modules directory
cannot be opened, even though the kallsyms has been properly
processed.  This leads to a perf kvm report that has no guest symbols
resolved.

This patch changes the failure to locate kernel modules to be
non-fatal.

Signed-off-by: Jason Wessel 
---
 tools/perf/util/machine.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index b2ecad6..eb9ebd6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -808,7 +808,10 @@ static int machine__create_modules(struct machine *machine)
free(line);
fclose(file);
 
-   return machine__set_modules_path(machine);
+   if (machine__set_modules_path(machine) < 0) {
+   pr_debug("Problems setting modules path maps, continuing 
anyway...\n");
+   }
+   return 0;
 
 out_delete_line:
free(line);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] perf: Do not require /lib/modules/* on a guest

[Jason Wessel]

For some types of work loads and special guest environments, you might
have a kernel that has no kernel modules.  The perf kvm record tool
fails instantiate vmlinux maps when the kernel modules directory
cannot be opened, even though the kallsyms has been properly
processed.  This leads to a perf kvm report that has no guest symbols
resolved.

This patch changes the failure to locate kernel modules to be
non-fatal.

Signed-off-by: Jason Wessel jason.wes...@windriver.com
---
 tools/perf/util/machine.c |5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index b2ecad6..eb9ebd6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -808,7 +808,10 @@ static int machine__create_modules(struct machine *machine)
free(line);
fclose(file);
 
-   return machine__set_modules_path(machine);
+   if (machine__set_modules_path(machine)  0) {
+   pr_debug(Problems setting modules path maps, continuing 
anyway...\n);
+   }
+   return 0;
 
 out_delete_line:
free(line);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kdb: kgdb: CONFIG_DEBUG_RODATA setting?

[Jason Wessel]

On 04/09/2013 08:06 AM, Sedat Dilek wrote:

>  config DEBUG_RODATA
>   bool "Write protect kernel read-only data structures"
> - default y
>   depends on DEBUG_KERNEL
> + default n if KGDB
> + default y
>   ---help---
> Mark the kernel read-only data as write-protected in the pagetables,
> in order to catch accidental (and incorrect) writes to such const


This is wrong and should not be merged.

A) You should not change these defaults in this manner

B) You have pointed out that the documentation is currently
   incorrect. The CONFIG_DEBUG_RODATA constraints were removed some time
   ago, per commit 3751d3e85cf693e10e2c47c03c8caa65e171099b (x86,kgdb:
   Fix DEBUG_RODATA limitation using text_poke())

Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: kdb: kgdb: CONFIG_DEBUG_RODATA setting?

[Jason Wessel]

On 04/09/2013 08:06 AM, Sedat Dilek wrote:

  config DEBUG_RODATA
   bool Write protect kernel read-only data structures
 - default y
   depends on DEBUG_KERNEL
 + default n if KGDB
 + default y
   ---help---
 Mark the kernel read-only data as write-protected in the pagetables,
 in order to catch accidental (and incorrect) writes to such const


This is wrong and should not be merged.

A) You should not change these defaults in this manner

B) You have pointed out that the documentation is currently
   incorrect. The CONFIG_DEBUG_RODATA constraints were removed some time
   ago, per commit 3751d3e85cf693e10e2c47c03c8caa65e171099b (x86,kgdb:
   Fix DEBUG_RODATA limitation using text_poke())

Jason.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/15] KDB: up the default LINES value

[Jason Wessel]

On 03/25/2013 01:50 PM, Mike Travis wrote:
> Currently the default for the # of lines displayed by the KDB pager
> is 24.  This does not allow all of the lines for the entry messages,
> reg dump and process trace.  Increase it to something more reasonable.
> 


Unfortunately this is something that breaks compatibility with the
standard VGA console, so this patch will not be merged.

Is it the case that your hardware specifies how many lines and columns
there are in the display?  We have some hooks into the console VT code
to properly detect this but perhaps something is wrong there, or it is
only called in the KMS (kernel mode setting case).  Example:
drivers/tty/vt/vt.c - look at con_debug_enter()

Another option might be to add a variable which allows you to change
the default at compile time, but I would prefer to get the auto detect
code working properly if it is an option.

Jason.



> Cc: Tim Bird 
> Reviewed-by: Dimitri Sivanich 
> Signed-off-by: Mike Travis 
> ---
>  kernel/debug/kdb/kdb_io.c |2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- linux.orig/kernel/debug/kdb/kdb_io.c
> +++ linux/kernel/debug/kdb/kdb_io.c
> @@ -586,7 +586,7 @@ int vkdb_printf(const char *fmt, va_list
>  
>   diag = kdbgetintenv("LINES", );
>   if (diag || linecount <= 1)
> - linecount = 24;
> + linecount = 60;
>  
>   diag = kdbgetintenv("COLUMNS", );
>   if (diag || colcount <= 1)
> 
> -- 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/15] KDB: fix the interrupt of the KDB btc command

[Jason Wessel]

On 03/25/2013 01:50 PM, Mike Travis wrote:
> The KDB 'btc' (backtrace cpus) command ignores the 'quit' reply
> to the 'more>' prompt.  This is quite annoying when you have a
> large number of processors and thousands of lines are being
> printed.  This fixes that problem.
> 

Merged to kgdb-next and added as a cc to -stable.

I'll be working my way through the rest of the patches in the series you sent 
and I'll merge anything that is going to -stable in the 3.9 series toward the 
end of the week assuming everything passes regression testing, and the rest 
will go into the 3.10 merge window.

Cheers,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 01/15] KDB: fix the interrupt of the KDB btc command

[Jason Wessel]

On 03/25/2013 01:50 PM, Mike Travis wrote:
 The KDB 'btc' (backtrace cpus) command ignores the 'quit' reply
 to the 'more' prompt.  This is quite annoying when you have a
 large number of processors and thousands of lines are being
 printed.  This fixes that problem.
 

Merged to kgdb-next and added as a cc to -stable.

I'll be working my way through the rest of the patches in the series you sent 
and I'll merge anything that is going to -stable in the 3.9 series toward the 
end of the week assuming everything passes regression testing, and the rest 
will go into the 3.10 merge window.

Cheers,
Jason.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 03/15] KDB: up the default LINES value

[Jason Wessel]

On 03/25/2013 01:50 PM, Mike Travis wrote:
 Currently the default for the # of lines displayed by the KDB pager
 is 24.  This does not allow all of the lines for the entry messages,
 reg dump and process trace.  Increase it to something more reasonable.
 


Unfortunately this is something that breaks compatibility with the
standard VGA console, so this patch will not be merged.

Is it the case that your hardware specifies how many lines and columns
there are in the display?  We have some hooks into the console VT code
to properly detect this but perhaps something is wrong there, or it is
only called in the KMS (kernel mode setting case).  Example:
drivers/tty/vt/vt.c - look at con_debug_enter()

Another option might be to add a variable which allows you to change
the default at compile time, but I would prefer to get the auto detect
code working properly if it is an option.

Jason.



 Cc: Tim Bird tim.b...@am.sony.com
 Reviewed-by: Dimitri Sivanich sivan...@sgi.com
 Signed-off-by: Mike Travis tra...@sgi.com
 ---
  kernel/debug/kdb/kdb_io.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 --- linux.orig/kernel/debug/kdb/kdb_io.c
 +++ linux/kernel/debug/kdb/kdb_io.c
 @@ -586,7 +586,7 @@ int vkdb_printf(const char *fmt, va_list
  
   diag = kdbgetintenv(LINES, linecount);
   if (diag || linecount = 1)
 - linecount = 24;
 + linecount = 60;
  
   diag = kdbgetintenv(COLUMNS, colcount);
   if (diag || colcount = 1)
 
 -- 
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/13] tracing/kdb: remove redundant checking

[Jason Wessel]

On 03/11/2013 09:09 AM, Steven Rostedt wrote:
> This is Jason's code.
>
> Jason, please give an Ack or Nack.
>
> Thanks,
>
> -- Steve
>
>
> On Mon, 2013-03-11 at 15:13 +0800, zhangwei(Jovi) wrote:
>> trace_empty is checking in while-loop, so the previous checking
>> is totally redundant, and more worse, the first trace entry is losted.
>>
>> so remove it.
>>
>> Signed-off-by: zhangwei(Jovi) 
>> ---
>>  kernel/trace/trace_kdb.c |3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
>> index 3c5c5df..6489b2e 100644
>> --- a/kernel/trace/trace_kdb.c
>> +++ b/kernel/trace/trace_kdb.c
>> @@ -57,8 +57,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file)
>>  ring_buffer_read_start(iter.buffer_iter[cpu_file]);
>>  tracing_iter_reset(, cpu_file);
>>  }
>> -if (!trace_empty())
>> -trace_find_next_entry_inc();
>> +
>>  while (!trace_empty()) {
>>  if (!cnt)
>>  kdb_printf("-\n");
>


This was just a copy of part of the logic used for printing the information in 
a similar manner to what you get when you cat the trace buffer to obtain the 
human readable version.

May I ask how you tested it though?

As far as I know the patch I sent Stephen quite a while ago got lost and the 
mainline version of the "ftdump" doesn't actually work.

Example:
[0]kdb> ftdump
Dumping ftrace buffer:
3BUG: sleeping function called from invalid context at 
/space/jw/git/kgdb/linux-2.6-kgdb/mm/slub.c:925
3in_atomic(): 1, irqs_disabled(): 1, pid: 81, name: sh
Pid: 81, comm: sh Not tainted 3.9.0-rc1-WR4.3.0.0_standard+ #568
Call Trace:
 <#DB>  [] __might_sleep+0xde/0x100
 [] kmem_cache_alloc_trace+0xdb/0x170
 [] ring_buffer_read_prepare+0x4d/0x70
 [] kdb_ftdump+0x1e8/0x410
 [] kdb_parse+0x209/0x690
 [] kdb_main_loop+0x67c/0x8c0
 [] kdb_stub+0x1d3/0x420
 [] ? __send_signal+0x1ad/0x3e0
 [] kgdb_cpu_enter+0x27e/0x590
 [] kgdb_handle_exception+0x161/0x1c0
 [] __kgdb_notify+0x31/0xe0
 [] kgdb_ll_trap+0x40/0x50
 [] do_int3+0x52/0x130
 [] int3+0x25/0x40
 [] ? sysrq_handle_dbg+0x32/0x60
 <>  [] __handle_sysrq+0x129/0x190


I think we need to actually empirically prove the code change is right or wrong 
before merging it, as well as cleaning up the change log slightly.

I'll go hunt down the patch the fixes the oops first.

Cheers,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 07/13] tracing/kdb: remove redundant checking

[Jason Wessel]

On 03/11/2013 09:09 AM, Steven Rostedt wrote:
 This is Jason's code.

 Jason, please give an Ack or Nack.

 Thanks,

 -- Steve


 On Mon, 2013-03-11 at 15:13 +0800, zhangwei(Jovi) wrote:
 trace_empty is checking in while-loop, so the previous checking
 is totally redundant, and more worse, the first trace entry is losted.

 so remove it.

 Signed-off-by: zhangwei(Jovi) jovi.zhang...@huawei.com
 ---
  kernel/trace/trace_kdb.c |3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

 diff --git a/kernel/trace/trace_kdb.c b/kernel/trace/trace_kdb.c
 index 3c5c5df..6489b2e 100644
 --- a/kernel/trace/trace_kdb.c
 +++ b/kernel/trace/trace_kdb.c
 @@ -57,8 +57,7 @@ static void ftrace_dump_buf(int skip_lines, long cpu_file)
  ring_buffer_read_start(iter.buffer_iter[cpu_file]);
  tracing_iter_reset(iter, cpu_file);
  }
 -if (!trace_empty(iter))
 -trace_find_next_entry_inc(iter);
 +
  while (!trace_empty(iter)) {
  if (!cnt)
  kdb_printf(-\n);



This was just a copy of part of the logic used for printing the information in 
a similar manner to what you get when you cat the trace buffer to obtain the 
human readable version.

May I ask how you tested it though?

As far as I know the patch I sent Stephen quite a while ago got lost and the 
mainline version of the ftdump doesn't actually work.

Example:
[0]kdb ftdump
Dumping ftrace buffer:
3BUG: sleeping function called from invalid context at 
/space/jw/git/kgdb/linux-2.6-kgdb/mm/slub.c:925
3in_atomic(): 1, irqs_disabled(): 1, pid: 81, name: sh
Pid: 81, comm: sh Not tainted 3.9.0-rc1-WR4.3.0.0_standard+ #568
Call Trace:
 #DB  [81069ffe] __might_sleep+0xde/0x100
 [8112611b] kmem_cache_alloc_trace+0xdb/0x170
 [810c01bd] ring_buffer_read_prepare+0x4d/0x70
 [810d4d28] kdb_ftdump+0x1e8/0x410
 [810a9a89] kdb_parse+0x209/0x690
 [810aad0c] kdb_main_loop+0x67c/0x8c0
 [810ad4b3] kdb_stub+0x1d3/0x420
 [8104ccfd] ? __send_signal+0x1ad/0x3e0
 [810a33be] kgdb_cpu_enter+0x27e/0x590
 [810a3981] kgdb_handle_exception+0x161/0x1c0
 [81027cf1] __kgdb_notify+0x31/0xe0
 [81027e10] kgdb_ll_trap+0x40/0x50
 [81002e12] do_int3+0x52/0x130
 [81674345] int3+0x25/0x40
 [810a2be2] ? sysrq_handle_dbg+0x32/0x60
 EOE  [813e1e69] __handle_sysrq+0x129/0x190


I think we need to actually empirically prove the code change is right or wrong 
before merging it, as well as cleaning up the change log slightly.

I'll go hunt down the patch the fixes the oops first.

Cheers,
Jason.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] menuconfig,check-lxdiaglog.sh: Allow specification of ncurses location

[Jason Wessel]

In some cross build environments such as the Yocto Project build
environment it provides an ncurses library that is compiled
differently than the host's version.  This causes display corruption
problems when the host's curses includes are used instead of the
includes from the provided compiler are overridden.  There is a second
case where there is no curses libraries at all on the host system and
menuconfig will just fail entirely.

The solution is simply to allow an override variable in
check-lxdialog.sh for environments such as the Yocto Project.  Adding
a CROSS_CURSES_LIB and CROSS_CURSES_INC solves the issue and allowing
compiling and linking against the right headers and libraries.

Signed-off-by: Jason Wessel 
cc: Michal Marek 
cc: linux-kbu...@vger.kernel.org
---
 scripts/kconfig/lxdialog/check-lxdialog.sh |8 
 1 file changed, 8 insertions(+)

diff --git a/scripts/kconfig/lxdialog/check-lxdialog.sh 
b/scripts/kconfig/lxdialog/check-lxdialog.sh
index 8078813..99473e4 100644
--- a/scripts/kconfig/lxdialog/check-lxdialog.sh
+++ b/scripts/kconfig/lxdialog/check-lxdialog.sh
@@ -4,6 +4,10 @@
 # What library to link
 ldflags()
 {
+   if [ x"$CROSS_CURSES_LIB" != x ]; then
+   echo "$CROSS_CURSES_LIB"
+   exit
+   fi
for ext in so a dll.a dylib ; do
for lib in ncursesw ncurses curses ; do
$cc -print-file-name=lib${lib}.${ext} | grep -q /
@@ -19,6 +23,10 @@ ldflags()
 # Where is ncurses.h?
 ccflags()
 {
+   if [ x"$CROSS_CURSES_INC" != x ]; then
+   echo "$CROSS_CURSES_INC"
+   exit
+   fi
if [ -f /usr/include/ncursesw/curses.h ]; then
echo '-I/usr/include/ncursesw 
-DCURSES_LOC=""'
echo ' -DNCURSES_WIDECHAR=1'
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] menuconfig,check-lxdiaglog.sh: Allow specification of ncurses location

[Jason Wessel]

In some cross build environments such as the Yocto Project build
environment it provides an ncurses library that is compiled
differently than the host's version.  This causes display corruption
problems when the host's curses includes are used instead of the
includes from the provided compiler are overridden.  There is a second
case where there is no curses libraries at all on the host system and
menuconfig will just fail entirely.

The solution is simply to allow an override variable in
check-lxdialog.sh for environments such as the Yocto Project.  Adding
a CROSS_CURSES_LIB and CROSS_CURSES_INC solves the issue and allowing
compiling and linking against the right headers and libraries.

Signed-off-by: Jason Wessel jason.wes...@windriver.com
cc: Michal Marek mma...@suse.cz
cc: linux-kbu...@vger.kernel.org
---
 scripts/kconfig/lxdialog/check-lxdialog.sh |8 
 1 file changed, 8 insertions(+)

diff --git a/scripts/kconfig/lxdialog/check-lxdialog.sh 
b/scripts/kconfig/lxdialog/check-lxdialog.sh
index 8078813..99473e4 100644
--- a/scripts/kconfig/lxdialog/check-lxdialog.sh
+++ b/scripts/kconfig/lxdialog/check-lxdialog.sh
@@ -4,6 +4,10 @@
 # What library to link
 ldflags()
 {
+   if [ x$CROSS_CURSES_LIB != x ]; then
+   echo $CROSS_CURSES_LIB
+   exit
+   fi
for ext in so a dll.a dylib ; do
for lib in ncursesw ncurses curses ; do
$cc -print-file-name=lib${lib}.${ext} | grep -q /
@@ -19,6 +23,10 @@ ldflags()
 # Where is ncurses.h?
 ccflags()
 {
+   if [ x$CROSS_CURSES_INC != x ]; then
+   echo $CROSS_CURSES_INC
+   exit
+   fi
if [ -f /usr/include/ncursesw/curses.h ]; then
echo '-I/usr/include/ncursesw 
-DCURSES_LOC=ncursesw/curses.h'
echo ' -DNCURSES_WIDECHAR=1'
-- 
1.7.9.5

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.9

[Jason Wessel]

Linus,

Please pull the kgdb tree's fixes/improvements.

For a change we removed more code than we added.  If people aren't using
it we shouldn't be carrying it. :-)

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.9

Thanks,
Jason.

The following changes since commit d895cb1af15c04c522a25c79cc429076987c089b:

  Merge branch 'for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs (2013-02-26 20:16:07 
-0800)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linux-3.9

for you to fetch changes up to 397884d2e367aed778de69761181d85e348393bf:

  kdb: Remove unhandled ssb command (2013-03-02 06:53:22 -0600)


KGDB/KDB fixes and cleanups
 Cleanups
   Remove kdb ssb command - there is no in kernel disassembler to support it
   Remove kdb ll command - Always caused a kernel oops and there were no
   bug reports so no one was using this command
   Use kernel ARRAY_SIZE macro instead of array computations

 Fixes
   Stop oops in kdb if user executes kdb_defcmd with args
   kdb help command truncated text
   ppc64 support for kgdbts
   Add missing kconfig option from original kdb port for dealing with
  catastrophic kernel crashes such that you can reboot automatically
  on continue from kdb


Jason Wessel (4):
  kdb: Fix overlap in buffers with strcpy
  kdb_main: fix help print
  kdb: Remove the ll command
  kdb: Prevent kernel oops with kdb_defcmd

John Blackwood (1):
  kdb: A fix for kdb command table expansion

Matt Klein (1):
  kdb: Setup basic kdb state before invoking commands via kgdb

Robert Obermeier (1):
  Fixed dead ifdef block by adding missing Kconfig option.

Sasha Levin (1):
  kdb: use ARRAY_SIZE where possible

Tiejun Chen (1):
  kgdb/kgdbts: support ppc64

Vincent (1):
  kdb: Remove unhandled ssb command

 drivers/misc/kgdbts.c   |2 +
 kernel/debug/debug_core.h   |2 +
 kernel/debug/gdbstub.c  |3 +
 kernel/debug/kdb/kdb_bp.c   |   20 +-
 kernel/debug/kdb/kdb_debugger.c |   25 ++--
 kernel/debug/kdb/kdb_main.c |  135 +++
 kernel/debug/kdb/kdb_private.h  |4 --
 lib/Kconfig.kgdb|   18 ++
 8 files changed, 82 insertions(+), 127 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.9

[Jason Wessel]

Linus,

Please pull the kgdb tree's fixes/improvements.

For a change we removed more code than we added.  If people aren't using
it we shouldn't be carrying it. :-)

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.9

Thanks,
Jason.

The following changes since commit d895cb1af15c04c522a25c79cc429076987c089b:

  Merge branch 'for-linus' of 
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs (2013-02-26 20:16:07 
-0800)

are available in the git repository at:


  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linux-3.9

for you to fetch changes up to 397884d2e367aed778de69761181d85e348393bf:

  kdb: Remove unhandled ssb command (2013-03-02 06:53:22 -0600)


KGDB/KDB fixes and cleanups
 Cleanups
   Remove kdb ssb command - there is no in kernel disassembler to support it
   Remove kdb ll command - Always caused a kernel oops and there were no
   bug reports so no one was using this command
   Use kernel ARRAY_SIZE macro instead of array computations

 Fixes
   Stop oops in kdb if user executes kdb_defcmd with args
   kdb help command truncated text
   ppc64 support for kgdbts
   Add missing kconfig option from original kdb port for dealing with
  catastrophic kernel crashes such that you can reboot automatically
  on continue from kdb


Jason Wessel (4):
  kdb: Fix overlap in buffers with strcpy
  kdb_main: fix help print
  kdb: Remove the ll command
  kdb: Prevent kernel oops with kdb_defcmd

John Blackwood (1):
  kdb: A fix for kdb command table expansion

Matt Klein (1):
  kdb: Setup basic kdb state before invoking commands via kgdb

Robert Obermeier (1):
  Fixed dead ifdef block by adding missing Kconfig option.

Sasha Levin (1):
  kdb: use ARRAY_SIZE where possible

Tiejun Chen (1):
  kgdb/kgdbts: support ppc64

Vincent (1):
  kdb: Remove unhandled ssb command

 drivers/misc/kgdbts.c   |2 +
 kernel/debug/debug_core.h   |2 +
 kernel/debug/gdbstub.c  |3 +
 kernel/debug/kdb/kdb_bp.c   |   20 +-
 kernel/debug/kdb/kdb_debugger.c |   25 ++--
 kernel/debug/kdb/kdb_main.c |  135 +++
 kernel/debug/kdb/kdb_private.h  |4 --
 lib/Kconfig.kgdb|   18 ++
 8 files changed, 82 insertions(+), 127 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] powerpc: kernel/kgdb.c: fix memory leakage

[Jason Wessel]

On 01/14/2013 11:26 AM, Cong Ding wrote:
> the variable backup_current_thread_info isn't freed before existing the
> function.
> 
> Signed-off-by: Cong Ding 
> ---
>  arch/powerpc/kernel/kgdb.c |5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
> index 8747447..5ca82cd 100644
> --- a/arch/powerpc/kernel/kgdb.c
> +++ b/arch/powerpc/kernel/kgdb.c
> @@ -154,12 +154,12 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
>  static int kgdb_singlestep(struct pt_regs *regs)
>  {
>   struct thread_info *thread_info, *exception_thread_info;
> - struct thread_info *backup_current_thread_info = \
> - (struct thread_info *)kmalloc(sizeof(struct thread_info), 
> GFP_KERNEL);
> + struct thread_info *backup_current_thread_info;



Woh...  This is definitely wrong.  You have found a problem for sure,
but this is not the right way to fix it.

It is not a good idea to kmalloc while single stepping because you can
hang the kernel if you single step any operation in kmalloc().

I am in the process of going through all the kgdb mails from the last
few months while I had been away from the project, so I didn't catch
this one and I see it has upstream commit (fefd9e6f8).  I'll submit
another patch to fix this the right way and use a static variable.
This is ok to use a static variable here because this is not something
we can recursively call at a single CPU level.

If Ben prefers we not burn the memory unless kgdb is active we can
kmalloc / kfree the space we need at the time that kgdb is
initialized.  Else we can go with this patch you see below.  We'll see
what Ben desires.

-
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index a7bc752..bb12c8b 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -151,15 +151,16 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
return 1;
 }
 
+static struct thread_info kgdb_backup_thread_info[NR_CPUS];
+
 static int kgdb_singlestep(struct pt_regs *regs)
 {
struct thread_info *thread_info, *exception_thread_info;
-   struct thread_info *backup_current_thread_info;
+   int cpu = raw_smp_processor_id();
 
if (user_mode(regs))
return 0;
 
-   backup_current_thread_info = (struct thread_info 
*)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
/*
 * On Book E and perhaps other processors, singlestep is handled on
 * the critical exception stack.  This causes current_thread_info()
@@ -175,7 +176,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
 
if (thread_info != exception_thread_info) {
/* Save the original current_thread_info. */
-   memcpy(backup_current_thread_info, exception_thread_info, 
sizeof *thread_info);
+   memcpy(_backup_thread_info[cpu], exception_thread_info, 
sizeof *thread_info);
memcpy(exception_thread_info, thread_info, sizeof *thread_info);
}
 
@@ -183,9 +184,8 @@ static int kgdb_singlestep(struct pt_regs *regs)
 
if (thread_info != exception_thread_info)
/* Restore current_thread_info lastly. */
-   memcpy(exception_thread_info, backup_current_thread_info, 
sizeof *thread_info);
+   memcpy(exception_thread_info, _backup_thread_info[cpu], 
sizeof *thread_info);
 
-   kfree(backup_current_thread_info);
return 1;
 }
 

-


Thanks,
Jason.


>  
>   if (user_mode(regs))
>   return 0;
>  
> + backup_current_thread_info = (struct thread_info 
> *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
>   /*
>* On Book E and perhaps other processors, singlestep is handled on
>* the critical exception stack.  This causes current_thread_info()
> @@ -185,6 +185,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
>   /* Restore current_thread_info lastly. */
>   memcpy(exception_thread_info, backup_current_thread_info, 
> sizeof *thread_info);
>  
> + kfree(backup_current_thread_info);
>   return 1;
>  }
>  
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] powerpc: kernel/kgdb.c: fix memory leakage

[Jason Wessel]

On 01/14/2013 11:26 AM, Cong Ding wrote:
 the variable backup_current_thread_info isn't freed before existing the
 function.
 
 Signed-off-by: Cong Ding ding...@gmail.com
 ---
  arch/powerpc/kernel/kgdb.c |5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)
 
 diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
 index 8747447..5ca82cd 100644
 --- a/arch/powerpc/kernel/kgdb.c
 +++ b/arch/powerpc/kernel/kgdb.c
 @@ -154,12 +154,12 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
  static int kgdb_singlestep(struct pt_regs *regs)
  {
   struct thread_info *thread_info, *exception_thread_info;
 - struct thread_info *backup_current_thread_info = \
 - (struct thread_info *)kmalloc(sizeof(struct thread_info), 
 GFP_KERNEL);
 + struct thread_info *backup_current_thread_info;



Woh...  This is definitely wrong.  You have found a problem for sure,
but this is not the right way to fix it.

It is not a good idea to kmalloc while single stepping because you can
hang the kernel if you single step any operation in kmalloc().

I am in the process of going through all the kgdb mails from the last
few months while I had been away from the project, so I didn't catch
this one and I see it has upstream commit (fefd9e6f8).  I'll submit
another patch to fix this the right way and use a static variable.
This is ok to use a static variable here because this is not something
we can recursively call at a single CPU level.

If Ben prefers we not burn the memory unless kgdb is active we can
kmalloc / kfree the space we need at the time that kgdb is
initialized.  Else we can go with this patch you see below.  We'll see
what Ben desires.

-
diff --git a/arch/powerpc/kernel/kgdb.c b/arch/powerpc/kernel/kgdb.c
index a7bc752..bb12c8b 100644
--- a/arch/powerpc/kernel/kgdb.c
+++ b/arch/powerpc/kernel/kgdb.c
@@ -151,15 +151,16 @@ static int kgdb_handle_breakpoint(struct pt_regs *regs)
return 1;
 }
 
+static struct thread_info kgdb_backup_thread_info[NR_CPUS];
+
 static int kgdb_singlestep(struct pt_regs *regs)
 {
struct thread_info *thread_info, *exception_thread_info;
-   struct thread_info *backup_current_thread_info;
+   int cpu = raw_smp_processor_id();
 
if (user_mode(regs))
return 0;
 
-   backup_current_thread_info = (struct thread_info 
*)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
/*
 * On Book E and perhaps other processors, singlestep is handled on
 * the critical exception stack.  This causes current_thread_info()
@@ -175,7 +176,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
 
if (thread_info != exception_thread_info) {
/* Save the original current_thread_info. */
-   memcpy(backup_current_thread_info, exception_thread_info, 
sizeof *thread_info);
+   memcpy(kgdb_backup_thread_info[cpu], exception_thread_info, 
sizeof *thread_info);
memcpy(exception_thread_info, thread_info, sizeof *thread_info);
}
 
@@ -183,9 +184,8 @@ static int kgdb_singlestep(struct pt_regs *regs)
 
if (thread_info != exception_thread_info)
/* Restore current_thread_info lastly. */
-   memcpy(exception_thread_info, backup_current_thread_info, 
sizeof *thread_info);
+   memcpy(exception_thread_info, kgdb_backup_thread_info[cpu], 
sizeof *thread_info);
 
-   kfree(backup_current_thread_info);
return 1;
 }
 

-


Thanks,
Jason.


  
   if (user_mode(regs))
   return 0;
  
 + backup_current_thread_info = (struct thread_info 
 *)kmalloc(sizeof(struct thread_info), GFP_KERNEL);
   /*
* On Book E and perhaps other processors, singlestep is handled on
* the critical exception stack.  This causes current_thread_info()
 @@ -185,6 +185,7 @@ static int kgdb_singlestep(struct pt_regs *regs)
   /* Restore current_thread_info lastly. */
   memcpy(exception_thread_info, backup_current_thread_info, 
 sizeof *thread_info);
  
 + kfree(backup_current_thread_info);
   return 1;
  }
  
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 57/76] ARC: kgdb support

[Jason Wessel]

On 01/18/2013 07:31 AM, Vineet Gupta wrote:
> On Friday 18 January 2013 06:45 PM, Jason Wessel wrote:
>> On 01/18/2013 06:25 AM, Vineet Gupta wrote:
>>> From: Mischa Jonker 
>>>
>>> Signed-off-by: Mischa Jonker 
>>> Signed-off-by: Vineet Gupta 
>>> Cc: Jason Wessel 
>> Acked-by: Jason Wessel 
>>
>>> ---
>>>  arch/arc/Kconfig|3 +-
>>>  arch/arc/include/asm/kgdb.h |   61 +
>>>  arch/arc/kernel/Makefile|1 +
>>>  arch/arc/kernel/kgdb.c  |  205 
>>> +++
>>>  arch/arc/kernel/traps.c |6 ++
>>>  5 files changed, 275 insertions(+), 1 deletions(-)
>>>  create mode 100644 arch/arc/include/asm/kgdb.h
>>>  create mode 100644 arch/arc/kernel/kgdb.c
>>>
>>> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
>>> index d735de8..b954387 100644
>>> --- a/arch/arc/Kconfig
>>> +++ b/arch/arc/Kconfig
>>> @@ -23,6 +23,7 @@ config ARC
>>> select GENERIC_PENDING_IRQ if SMP
>>> select GENERIC_SIGALTSTACK
>>> select GENERIC_SMP_IDLE_THREAD
>>> +   select HAVE_ARCH_KGDB
>>> select HAVE_ARCH_TRACEHOOK
>>> select HAVE_GENERIC_HARDIRQS
>>> select HAVE_KPROBES
>>> @@ -383,7 +384,7 @@ config ARC_DW2_UNWIND
>>>  
>>>  config ARC_DBG_TLB_PARANOIA
>>> bool "Paranoia Checks in Low Level TLB Handlers"
>>> -   depends on ARC_DBG && !SMP
>>> +   depends on ARC_DBG
>>> default n
>>>  
>>>  config ARC_DBG_TLB_MISS_COUNT
>>> diff --git a/arch/arc/include/asm/kgdb.h b/arch/arc/include/asm/kgdb.h
>>> new file mode 100644
>>> index 000..f3c4934
>>> --- /dev/null
>>> +++ b/arch/arc/include/asm/kgdb.h
>>> @@ -0,0 +1,61 @@
>>> +/*
>>> + * kgdb support for ARC
>>> + *
>>> + * Copyright (C) 2012 Synopsys, Inc. (www.synopsys.com)
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify
>>> + * it under the terms of the GNU General Public License version 2 as
>>> + * published by the Free Software Foundation.
>>> + */
>>> +
>>> +#ifndef __ARC_KGDB_H__
>>> +#define __ARC_KGDB_H__
>>> +
>>> +#ifdef CONFIG_KGDB
>>> +
>>> +#include 
>>> +
>>> +/* to ensure compatibility with Linux 2.6.35, we don't implement the 
>>> get/set
>>> + * register API yet */
>> I did think this note was slightly odd.   Is this mainly because you have 
>> some kind of out of tree version of this code against an older kernel?
> Thanks for your review and ACK, Jason. We did the initial port against 2.6.35 
> and
> later moved over to 3.2 - both of which were out-of-tree. We will take a look 
> at
> it again and clean this up.
>
>> The main thing that you lose by not doing this the ability to display and 
>> set registers from the kdb shell.
>>
>> From the pure gdb only end of things, gdb operates more efficiently for 
>> doing things like setting or reading the program counter, frame pointer or 
>> variable mapped registers when using a rs232 connection.  There is no loss 
>> or gain in actual functionality by not implementing the individual get/set 
>> register method if you only intend to use gdb.
>>
>> Cheers,
>> Jason.
> OK - we will fix this up - but I presume it is not a gating item for the port
> submission.

No gate what so ever.   I was just pointing out what you lose access to by 
without implementing the optional functions. :-)

Cheers,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 57/76] ARC: kgdb support

[Jason Wessel]

On 01/18/2013 06:25 AM, Vineet Gupta wrote:
> From: Mischa Jonker 
>
> Signed-off-by: Mischa Jonker 
> Signed-off-by: Vineet Gupta 
> Cc: Jason Wessel 

Acked-by: Jason Wessel 

> ---
>  arch/arc/Kconfig|3 +-
>  arch/arc/include/asm/kgdb.h |   61 +
>  arch/arc/kernel/Makefile|1 +
>  arch/arc/kernel/kgdb.c  |  205 
> +++
>  arch/arc/kernel/traps.c |6 ++
>  5 files changed, 275 insertions(+), 1 deletions(-)
>  create mode 100644 arch/arc/include/asm/kgdb.h
>  create mode 100644 arch/arc/kernel/kgdb.c
>
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index d735de8..b954387 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -23,6 +23,7 @@ config ARC
>   select GENERIC_PENDING_IRQ if SMP
>   select GENERIC_SIGALTSTACK
>   select GENERIC_SMP_IDLE_THREAD
> + select HAVE_ARCH_KGDB
>   select HAVE_ARCH_TRACEHOOK
>   select HAVE_GENERIC_HARDIRQS
>   select HAVE_KPROBES
> @@ -383,7 +384,7 @@ config ARC_DW2_UNWIND
>  
>  config ARC_DBG_TLB_PARANOIA
>   bool "Paranoia Checks in Low Level TLB Handlers"
> - depends on ARC_DBG && !SMP
> + depends on ARC_DBG
>   default n
>  
>  config ARC_DBG_TLB_MISS_COUNT
> diff --git a/arch/arc/include/asm/kgdb.h b/arch/arc/include/asm/kgdb.h
> new file mode 100644
> index 000..f3c4934
> --- /dev/null
> +++ b/arch/arc/include/asm/kgdb.h
> @@ -0,0 +1,61 @@
> +/*
> + * kgdb support for ARC
> + *
> + * Copyright (C) 2012 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ARC_KGDB_H__
> +#define __ARC_KGDB_H__
> +
> +#ifdef CONFIG_KGDB
> +
> +#include 
> +
> +/* to ensure compatibility with Linux 2.6.35, we don't implement the get/set
> + * register API yet */


I did think this note was slightly odd.   Is this mainly because you have some 
kind of out of tree version of this code against an older kernel?

The main thing that you lose by not doing this the ability to display and set 
registers from the kdb shell.

>From the pure gdb only end of things, gdb operates more efficiently for doing 
>things like setting or reading the program counter, frame pointer or variable 
>mapped registers when using a rs232 connection.  There is no loss or gain in 
>actual functionality by not implementing the individual get/set register 
>method if you only intend to use gdb.

Cheers,
Jason.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 57/76] ARC: kgdb support

[Jason Wessel]

On 01/18/2013 06:25 AM, Vineet Gupta wrote:
 From: Mischa Jonker mjon...@synopsys.com

 Signed-off-by: Mischa Jonker mjon...@synopsys.com
 Signed-off-by: Vineet Gupta vgu...@synopsys.com
 Cc: Jason Wessel jason.wes...@windriver.com

Acked-by: Jason Wessel jason.wes...@windriver.com

 ---
  arch/arc/Kconfig|3 +-
  arch/arc/include/asm/kgdb.h |   61 +
  arch/arc/kernel/Makefile|1 +
  arch/arc/kernel/kgdb.c  |  205 
 +++
  arch/arc/kernel/traps.c |6 ++
  5 files changed, 275 insertions(+), 1 deletions(-)
  create mode 100644 arch/arc/include/asm/kgdb.h
  create mode 100644 arch/arc/kernel/kgdb.c

 diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
 index d735de8..b954387 100644
 --- a/arch/arc/Kconfig
 +++ b/arch/arc/Kconfig
 @@ -23,6 +23,7 @@ config ARC
   select GENERIC_PENDING_IRQ if SMP
   select GENERIC_SIGALTSTACK
   select GENERIC_SMP_IDLE_THREAD
 + select HAVE_ARCH_KGDB
   select HAVE_ARCH_TRACEHOOK
   select HAVE_GENERIC_HARDIRQS
   select HAVE_KPROBES
 @@ -383,7 +384,7 @@ config ARC_DW2_UNWIND
  
  config ARC_DBG_TLB_PARANOIA
   bool Paranoia Checks in Low Level TLB Handlers
 - depends on ARC_DBG  !SMP
 + depends on ARC_DBG
   default n
  
  config ARC_DBG_TLB_MISS_COUNT
 diff --git a/arch/arc/include/asm/kgdb.h b/arch/arc/include/asm/kgdb.h
 new file mode 100644
 index 000..f3c4934
 --- /dev/null
 +++ b/arch/arc/include/asm/kgdb.h
 @@ -0,0 +1,61 @@
 +/*
 + * kgdb support for ARC
 + *
 + * Copyright (C) 2012 Synopsys, Inc. (www.synopsys.com)
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + */
 +
 +#ifndef __ARC_KGDB_H__
 +#define __ARC_KGDB_H__
 +
 +#ifdef CONFIG_KGDB
 +
 +#include asm/user.h
 +
 +/* to ensure compatibility with Linux 2.6.35, we don't implement the get/set
 + * register API yet */


I did think this note was slightly odd.   Is this mainly because you have some 
kind of out of tree version of this code against an older kernel?

The main thing that you lose by not doing this the ability to display and set 
registers from the kdb shell.

From the pure gdb only end of things, gdb operates more efficiently for doing 
things like setting or reading the program counter, frame pointer or variable 
mapped registers when using a rs232 connection.  There is no loss or gain in 
actual functionality by not implementing the individual get/set register 
method if you only intend to use gdb.

Cheers,
Jason.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 57/76] ARC: kgdb support

[Jason Wessel]

On 01/18/2013 07:31 AM, Vineet Gupta wrote:
 On Friday 18 January 2013 06:45 PM, Jason Wessel wrote:
 On 01/18/2013 06:25 AM, Vineet Gupta wrote:
 From: Mischa Jonker mjon...@synopsys.com

 Signed-off-by: Mischa Jonker mjon...@synopsys.com
 Signed-off-by: Vineet Gupta vgu...@synopsys.com
 Cc: Jason Wessel jason.wes...@windriver.com
 Acked-by: Jason Wessel jason.wes...@windriver.com

 ---
  arch/arc/Kconfig|3 +-
  arch/arc/include/asm/kgdb.h |   61 +
  arch/arc/kernel/Makefile|1 +
  arch/arc/kernel/kgdb.c  |  205 
 +++
  arch/arc/kernel/traps.c |6 ++
  5 files changed, 275 insertions(+), 1 deletions(-)
  create mode 100644 arch/arc/include/asm/kgdb.h
  create mode 100644 arch/arc/kernel/kgdb.c

 diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
 index d735de8..b954387 100644
 --- a/arch/arc/Kconfig
 +++ b/arch/arc/Kconfig
 @@ -23,6 +23,7 @@ config ARC
 select GENERIC_PENDING_IRQ if SMP
 select GENERIC_SIGALTSTACK
 select GENERIC_SMP_IDLE_THREAD
 +   select HAVE_ARCH_KGDB
 select HAVE_ARCH_TRACEHOOK
 select HAVE_GENERIC_HARDIRQS
 select HAVE_KPROBES
 @@ -383,7 +384,7 @@ config ARC_DW2_UNWIND
  
  config ARC_DBG_TLB_PARANOIA
 bool Paranoia Checks in Low Level TLB Handlers
 -   depends on ARC_DBG  !SMP
 +   depends on ARC_DBG
 default n
  
  config ARC_DBG_TLB_MISS_COUNT
 diff --git a/arch/arc/include/asm/kgdb.h b/arch/arc/include/asm/kgdb.h
 new file mode 100644
 index 000..f3c4934
 --- /dev/null
 +++ b/arch/arc/include/asm/kgdb.h
 @@ -0,0 +1,61 @@
 +/*
 + * kgdb support for ARC
 + *
 + * Copyright (C) 2012 Synopsys, Inc. (www.synopsys.com)
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 as
 + * published by the Free Software Foundation.
 + */
 +
 +#ifndef __ARC_KGDB_H__
 +#define __ARC_KGDB_H__
 +
 +#ifdef CONFIG_KGDB
 +
 +#include asm/user.h
 +
 +/* to ensure compatibility with Linux 2.6.35, we don't implement the 
 get/set
 + * register API yet */
 I did think this note was slightly odd.   Is this mainly because you have 
 some kind of out of tree version of this code against an older kernel?
 Thanks for your review and ACK, Jason. We did the initial port against 2.6.35 
 and
 later moved over to 3.2 - both of which were out-of-tree. We will take a look 
 at
 it again and clean this up.

 The main thing that you lose by not doing this the ability to display and 
 set registers from the kdb shell.

 From the pure gdb only end of things, gdb operates more efficiently for 
 doing things like setting or reading the program counter, frame pointer or 
 variable mapped registers when using a rs232 connection.  There is no loss 
 or gain in actual functionality by not implementing the individual get/set 
 register method if you only intend to use gdb.

 Cheers,
 Jason.
 OK - we will fix this up - but I presume it is not a gating item for the port
 submission.

No gate what so ever.   I was just pointing out what you lose access to by 
without implementing the optional functions. :-)

Cheers,
Jason.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 003/193] Documentation: remove CONFIG_EXPERIMENTAL

[Jason Wessel]

On 10/23/2012 03:01 PM, Kees Cook wrote:
> This config item has not carried much meaning for a while now and is
> almost always enabled by default. As agreed during the Linux kernel
> summit, remove it.
> 

Works for me.

Acked-by: Jason Wessel 

Cheers,
Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 003/193] Documentation: remove CONFIG_EXPERIMENTAL

[Jason Wessel]

On 10/23/2012 03:01 PM, Kees Cook wrote:
 This config item has not carried much meaning for a while now and is
 almost always enabled by default. As agreed during the Linux kernel
 summit, remove it.
 

Works for me.

Acked-by: Jason Wessel jason.wes...@windriver.com

Cheers,
Jason.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.7

[Jason Wessel]

Linus,

Please pull the kgdb tree's usual minor fixes/improvements. 

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.7

Thanks,
Jason.

-
The following changes since commit a0d271cbfed1dd50278c6b06bead3d00ba0a88f9:

  Linux 3.6 (2012-09-30 16:47:46 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.7

for you to fetch changes up to f2f0945e356daef87cdb01c0302801fb11cf382a:

  tty/console: fix warnings in drivers/tty/serial/kgdboc.c (2012-10-12 06:37:36 
-0500)


KGDB/KDB fixes and cleanups
 Cleanups
   Clean up compile warnings in kgdboc.c and x86/kernel/kgdb.c
   Add module event hooks for simplified debugging with gdb
 Fixes
   Fix kdb to stop paging with 'q' on bta and dmesg
   Fix for data that scrolls off the vga console due to line wrapping
 when using the kdb pager
 New
   The debug core registers for kernel module events which allows a
 kernel aware gdb to automatically load symbols and break on entry
 to a kernel module
   Allow kgdboc=kdb to setup kdb on the vga console


Arnd Bergmann (1):
  tty/console: fix warnings in drivers/tty/serial/kgdboc.c

Jason Wessel (6):
  kgdb: Add module event hooks
  mips,kgdb: fix recursive page fault with CONFIG_KPROBES
  kgdb,x86: fix warning about unused variable
  kgdboc: Accept either kbd or kdb to activate the vga + keyboard kdb shell
  kdb: Fix dmesg/bta scroll to quit with 'q'
  kdb,vt_console: Fix missed data due to pager overruns

 arch/mips/kernel/kgdb.c |9 +
 arch/x86/kernel/kgdb.c  |2 ++
 drivers/tty/serial/kgdboc.c |3 ++-
 drivers/tty/vt/vt.c |   13 +
 include/linux/console.h |   10 --
 kernel/debug/debug_core.c   |   18 ++
 kernel/debug/kdb/kdb_bt.c   |2 ++
 kernel/debug/kdb/kdb_io.c   |   33 -
 kernel/debug/kdb/kdb_main.c |2 ++
 9 files changed, 84 insertions(+), 8 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[GIT PULL] KGDB/KDB fixes for 3.7

[Jason Wessel]

Linus,

Please pull the kgdb tree's usual minor fixes/improvements. 

git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.7

Thanks,
Jason.

-
The following changes since commit a0d271cbfed1dd50278c6b06bead3d00ba0a88f9:

  Linux 3.6 (2012-09-30 16:47:46 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/kgdb.git 
tags/for_linus-3.7

for you to fetch changes up to f2f0945e356daef87cdb01c0302801fb11cf382a:

  tty/console: fix warnings in drivers/tty/serial/kgdboc.c (2012-10-12 06:37:36 
-0500)


KGDB/KDB fixes and cleanups
 Cleanups
   Clean up compile warnings in kgdboc.c and x86/kernel/kgdb.c
   Add module event hooks for simplified debugging with gdb
 Fixes
   Fix kdb to stop paging with 'q' on bta and dmesg
   Fix for data that scrolls off the vga console due to line wrapping
 when using the kdb pager
 New
   The debug core registers for kernel module events which allows a
 kernel aware gdb to automatically load symbols and break on entry
 to a kernel module
   Allow kgdboc=kdb to setup kdb on the vga console


Arnd Bergmann (1):
  tty/console: fix warnings in drivers/tty/serial/kgdboc.c

Jason Wessel (6):
  kgdb: Add module event hooks
  mips,kgdb: fix recursive page fault with CONFIG_KPROBES
  kgdb,x86: fix warning about unused variable
  kgdboc: Accept either kbd or kdb to activate the vga + keyboard kdb shell
  kdb: Fix dmesg/bta scroll to quit with 'q'
  kdb,vt_console: Fix missed data due to pager overruns

 arch/mips/kernel/kgdb.c |9 +
 arch/x86/kernel/kgdb.c  |2 ++
 drivers/tty/serial/kgdboc.c |3 ++-
 drivers/tty/vt/vt.c |   13 +
 include/linux/console.h |   10 --
 kernel/debug/debug_core.c   |   18 ++
 kernel/debug/kdb/kdb_bt.c   |2 ++
 kernel/debug/kdb/kdb_io.c   |   33 -
 kernel/debug/kdb/kdb_main.c |2 ++
 9 files changed, 84 insertions(+), 8 deletions(-)
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 09/12] tty/console: fix warnings in drivers/tty/serial/kgdboc.c

[Jason Wessel]

On 09/28/2012 04:36 PM, Arnd Bergmann wrote:
> The con_debug_leave/con_debug_enter functions are stubbed out
> by defining them to (0), which causes harmless build warnings.
> Using proper inline functions is the normal way to deal with
> this.
>
> Without this patch, building the ARM bcm2835_defconfig results in:
>
> drivers/tty/serial/kgdboc.c: In function 'kgdboc_pre_exp_handler':
> drivers/tty/serial/kgdboc.c:279:3: warning: statement with no effect 
> [-Wunused-value]
> drivers/tty/serial/kgdboc.c: In function 'kgdboc_post_exp_handler':
> drivers/tty/serial/kgdboc.c:293:3: warning: statement with no effect 
> [-Wunused-value]


Thanks Arnd!

I'll put this in kgdb-next for the upcoming merge window, unless Greg pulls it 
into his queue first.

Acked-by: Jason Wessel 

Cheers,
Jason.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/