mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [GIT PULL] kgdb regression fixes for 2.6.35-rc5
@ 2010-07-22  0:36 Jason Wessel
  2010-07-22  0:36 ` [PATCH 1/5] kdb: break out of kdb_ll() when command is terminated Jason Wessel
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-22  0:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

Linus, please pull the for_linus branch for 2.6.35-rc5.  

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

The following patch set covers cleaning up problems that have been
found since the merge of kdb to the 2.6.35-rc1 kernel.

Thanks,
Jason.


---
Jason Wessel (4):
      repair gdbstub to match the gdbserial protocol specification
      Fix merge regression from external kdb to upstream kdb
      debug_core,kdb: fix kgdb_connected bit set in the wrong place
      sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function

Martin Hicks (1):
      kdb: break out of kdb_ll() when command is terminated

 drivers/char/sysrq.c        |    2 +-
 include/linux/sysrq.h       |    1 +
 kernel/debug/debug_core.c   |    2 +-
 kernel/debug/gdbstub.c      |    9 +++------
 kernel/debug/kdb/kdb_main.c |    7 +++++--
 5 files changed, 11 insertions(+), 10 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] kdb: break out of kdb_ll() when command is terminated
  2010-07-22  0:36 [GIT PULL] kgdb regression fixes for 2.6.35-rc5 Jason Wessel
@ 2010-07-22  0:36 ` Jason Wessel
  2010-07-22  0:36   ` [PATCH 2/5] repair gdbstub to match the gdbserial protocol specification Jason Wessel
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-22  0:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

From: Martin Hicks <mort@sgi.com>

Without this patch the "ll" linked-list traversal command won't
terminate when you hit q/Q.

Signed-off-by: Martin Hicks <mort@sgi.com>
Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/kdb/kdb_main.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 184cd82..a7fe2e9 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -2291,6 +2291,9 @@ static int kdb_ll(int argc, const char **argv)
 	while (va) {
 		char buf[80];
 
+		if (KDB_FLAG(CMD_INTERRUPT))
+			return 0;
+
 		sprintf(buf, "%s " kdb_machreg_fmt "\n", command, va);
 		diag = kdb_parse(buf);
 		if (diag)
-- 
1.6.3.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/5] repair gdbstub to match the gdbserial protocol specification
  2010-07-22  0:36 ` [PATCH 1/5] kdb: break out of kdb_ll() when command is terminated Jason Wessel
@ 2010-07-22  0:36   ` Jason Wessel
  2010-07-22  0:36     ` [PATCH 3/5] Fix merge regression from external kdb to upstream kdb Jason Wessel
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-22  0:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

The gdbserial protocol handler should return an empty packet instead
of an error string when ever it responds to a command it does not
implement.

The problem cases come from a debugger client sending
qTBuffer, qTStatus, qSearch, qSupported.

The incorrect response from the gdbstub leads the debugger clients to
not function correctly.  Recent versions of gdb will not detach correctly as a result of this behavior.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Signed-off-by: Dongdong Deng <dongdong.deng@windriver.com>
---
 kernel/debug/gdbstub.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/kernel/debug/gdbstub.c b/kernel/debug/gdbstub.c
index 4b17b32..e8fd686 100644
--- a/kernel/debug/gdbstub.c
+++ b/kernel/debug/gdbstub.c
@@ -621,10 +621,8 @@ static void gdb_cmd_query(struct kgdb_state *ks)
 	switch (remcom_in_buffer[1]) {
 	case 's':
 	case 'f':
-		if (memcmp(remcom_in_buffer + 2, "ThreadInfo", 10)) {
-			error_packet(remcom_out_buffer, -EINVAL);
+		if (memcmp(remcom_in_buffer + 2, "ThreadInfo", 10))
 			break;
-		}
 
 		i = 0;
 		remcom_out_buffer[0] = 'm';
@@ -665,10 +663,9 @@ static void gdb_cmd_query(struct kgdb_state *ks)
 		pack_threadid(remcom_out_buffer + 2, thref);
 		break;
 	case 'T':
-		if (memcmp(remcom_in_buffer + 1, "ThreadExtraInfo,", 16)) {
-			error_packet(remcom_out_buffer, -EINVAL);
+		if (memcmp(remcom_in_buffer + 1, "ThreadExtraInfo,", 16))
 			break;
-		}
+
 		ks->threadid = 0;
 		ptr = remcom_in_buffer + 17;
 		kgdb_hex2long(&ptr, &ks->threadid);
-- 
1.6.3.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 3/5] Fix merge regression from external kdb to upstream kdb
  2010-07-22  0:36   ` [PATCH 2/5] repair gdbstub to match the gdbserial protocol specification Jason Wessel
@ 2010-07-22  0:36     ` Jason Wessel
  2010-07-22  0:36       ` [PATCH 4/5] debug_core,kdb: fix kgdb_connected bit set in the wrong place Jason Wessel
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-22  0:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

In the process of merging kdb to the mainline, the kdb lsmod command
stopped printing the base load address of kernel modules.  This is
needed for using kdb in conjunction with external tools such as gdb.

Simply restore the functionality by adding a kdb_printf for the base
load address of the kernel modules.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/kdb/kdb_main.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index a7fe2e9..7e9bfd5 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1883,6 +1883,7 @@ static int kdb_lsmod(int argc, const char **argv)
 			kdb_printf(" (Loading)");
 		else
 			kdb_printf(" (Live)");
+		kdb_printf(" 0x%p", mod->module_core);
 
 #ifdef CONFIG_MODULE_UNLOAD
 		{
-- 
1.6.3.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 4/5] debug_core,kdb: fix kgdb_connected bit set in the wrong place
  2010-07-22  0:36     ` [PATCH 3/5] Fix merge regression from external kdb to upstream kdb Jason Wessel
@ 2010-07-22  0:36       ` Jason Wessel
  2010-07-22  0:36         ` [PATCH 5/5] sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function Jason Wessel
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-22  0:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

Immediately following an exit from the kdb shell the kgdb_connected
variable should be set to zero, unless there are breakpoints planted.
If the kgdb_connected variable is not zeroed out with kdb, it is
impossible to turn off kdb.

This patch is merely a work around for now, the real fix will check
for the breakpoints.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
---
 kernel/debug/debug_core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/debug/debug_core.c b/kernel/debug/debug_core.c
index 5cb7cd1..8bc5eef 100644
--- a/kernel/debug/debug_core.c
+++ b/kernel/debug/debug_core.c
@@ -605,13 +605,13 @@ cpu_master_loop:
 		if (dbg_kdb_mode) {
 			kgdb_connected = 1;
 			error = kdb_stub(ks);
+			kgdb_connected = 0;
 		} else {
 			error = gdb_serial_stub(ks);
 		}
 
 		if (error == DBG_PASS_EVENT) {
 			dbg_kdb_mode = !dbg_kdb_mode;
-			kgdb_connected = 0;
 		} else if (error == DBG_SWITCH_CPU_EVENT) {
 			dbg_cpu_switch(cpu, dbg_switch_cpu);
 			goto cpu_loop;
-- 
1.6.3.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 5/5] sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function
  2010-07-22  0:36       ` [PATCH 4/5] debug_core,kdb: fix kgdb_connected bit set in the wrong place Jason Wessel
@ 2010-07-22  0:36         ` Jason Wessel
  0 siblings, 0 replies; 11+ messages in thread
From: Jason Wessel @ 2010-07-22  0:36 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

The kdb code should not toggle the sysrq state in case an end user
wants to try and resume the normal kernel execution.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/char/sysrq.c        |    2 +-
 include/linux/sysrq.h       |    1 +
 kernel/debug/kdb/kdb_main.c |    3 +--
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/char/sysrq.c b/drivers/char/sysrq.c
index 5d64e3a..878ac0c 100644
--- a/drivers/char/sysrq.c
+++ b/drivers/char/sysrq.c
@@ -493,7 +493,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
                 sysrq_key_table[i] = op_p;
 }
 
-static void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
+void __handle_sysrq(int key, struct tty_struct *tty, int check_mask)
 {
 	struct sysrq_key_op *op_p;
 	int orig_log_level;
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 4496322..609e8ca 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -45,6 +45,7 @@ struct sysrq_key_op {
  */
 
 void handle_sysrq(int key, struct tty_struct *tty);
+void __handle_sysrq(int key, struct tty_struct *tty, int check_mask);
 int register_sysrq_key(int key, struct sysrq_key_op *op);
 int unregister_sysrq_key(int key, struct sysrq_key_op *op);
 struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 7e9bfd5..ebe4a28 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1820,9 +1820,8 @@ static int kdb_sr(int argc, const char **argv)
 {
 	if (argc != 1)
 		return KDB_ARGCOUNT;
-	sysrq_toggle_support(1);
 	kdb_trap_printk++;
-	handle_sysrq(*argv[1], NULL);
+	__handle_sysrq(*argv[1], NULL, 0);
 	kdb_trap_printk--;
 
 	return 0;
-- 
1.6.3.3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5
  2010-07-29  0:14     ` Jason Wessel
@ 2010-07-29  1:06       ` Frederic Weisbecker
  0 siblings, 0 replies; 11+ messages in thread
From: Frederic Weisbecker @ 2010-07-29  1:06 UTC (permalink / raw)
  To: Jason Wessel; +Cc: Linus Torvalds, linux-kernel, kgdb-bugreport

On Wed, Jul 28, 2010 at 07:14:42PM -0500, Jason Wessel wrote:
> On 07/28/2010 04:26 PM, Linus Torvalds wrote:
> > On Wed, Jul 28, 2010 at 2:17 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> >> I'm sorry I spot it a bit late.
> >> There is a little issue in this patch, irqs won't
> >> be restored:
> >>
> >> @@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
> >>        if (!kgdb_io_module_registered)
> >>                return NOTIFY_DONE;
> >>
> >> +       local_irq_save(flags);
> >>        return __kgdb_notify(&args, cmd);
> >> +       local_irq_restore(flags);
> >>  }
> > 
> > Yeah, that's obviously crap. I also wonder why __kgdb_notify messes up
> > the flags to begin with.
> > 
> 
> This should not be needed as the irqs should already be off while in the overflow handler.



Interrupts are not disabled in the debug exception handler.

I'm not sure what would be the state of dr6 if a nested debug exception
would happen while do_debug() is interrupted.

Perhaps the previous dr6 value is dropped. Intel manuals don't seem to say anything
about that.

Anyway that's a separate issue. The current patch fixes a regression and
it looks good.

Thanks.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5
  2010-07-28 21:26   ` Linus Torvalds
@ 2010-07-29  0:14     ` Jason Wessel
  2010-07-29  1:06       ` Frederic Weisbecker
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-29  0:14 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Frederic Weisbecker, linux-kernel, kgdb-bugreport

On 07/28/2010 04:26 PM, Linus Torvalds wrote:
> On Wed, Jul 28, 2010 at 2:17 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>> I'm sorry I spot it a bit late.
>> There is a little issue in this patch, irqs won't
>> be restored:
>>
>> @@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
>>        if (!kgdb_io_module_registered)
>>                return NOTIFY_DONE;
>>
>> +       local_irq_save(flags);
>>        return __kgdb_notify(&args, cmd);
>> +       local_irq_restore(flags);
>>  }
> 
> Yeah, that's obviously crap. I also wonder why __kgdb_notify messes up
> the flags to begin with.
> 

This should not be needed as the irqs should already be off while in the overflow handler.

Sorry for the noise on the one.   I updated the for_linus branch with the final version.


 1 files changed, 7 insertions(+), 2 deletions(-)


--
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -572,7 +572,6 @@ static int __kgdb_notify(struct die_args *args, unsigned long cmd)
 	return NOTIFY_STOP;
 }
 
-#ifdef CONFIG_KGDB_LOW_LEVEL_TRAP
 int kgdb_ll_trap(int cmd, const char *str,
 		 struct pt_regs *regs, long err, int trap, int sig)
 {
@@ -590,7 +589,6 @@ int kgdb_ll_trap(int cmd, const char *str,
 
 	return __kgdb_notify(&args, cmd);
 }
-#endif /* CONFIG_KGDB_LOW_LEVEL_TRAP */
 
 static int
 kgdb_notify(struct notifier_block *self, unsigned long cmd, void *ptr)
@@ -625,6 +623,12 @@ int kgdb_arch_init(void)
 	return register_die_notifier(&kgdb_notifier);
 }
 
+static void kgdb_hw_overflow_handler(struct perf_event *event, int nmi,
+		struct perf_sample_data *data, struct pt_regs *regs)
+{
+	kgdb_ll_trap(DIE_DEBUG, "debug", regs, 0, 0, SIGTRAP);
+}
+
 void kgdb_arch_late(void)
 {
 	int i, cpu;
@@ -655,6 +659,7 @@ void kgdb_arch_late(void)
 		for_each_online_cpu(cpu) {
 			pevent = per_cpu_ptr(breakinfo[i].pev, cpu);
 			pevent[0]->hw.sample_period = 1;
+			pevent[0]->overflow_handler = kgdb_hw_overflow_handler;
 			if (pevent[0]->destroy != NULL) {
 				pevent[0]->destroy = NULL;
 				release_bp_slot(*pevent);

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5
  2010-07-28 21:17 ` Frederic Weisbecker
@ 2010-07-28 21:26   ` Linus Torvalds
  2010-07-29  0:14     ` Jason Wessel
  0 siblings, 1 reply; 11+ messages in thread
From: Linus Torvalds @ 2010-07-28 21:26 UTC (permalink / raw)
  To: Frederic Weisbecker; +Cc: Jason Wessel, linux-kernel, kgdb-bugreport

On Wed, Jul 28, 2010 at 2:17 PM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
>
> I'm sorry I spot it a bit late.
> There is a little issue in this patch, irqs won't
> be restored:
>
> @@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
>        if (!kgdb_io_module_registered)
>                return NOTIFY_DONE;
>
> +       local_irq_save(flags);
>        return __kgdb_notify(&args, cmd);
> +       local_irq_restore(flags);
>  }

Yeah, that's obviously crap. I also wonder why __kgdb_notify messes up
the flags to begin with.

             Linus

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [GIT PULL] kgdb regression fixes for 2.6.35-rc5
  2010-07-28 20:39 [GIT PULL] kgdb regression fixes for 2.6.35-rc5 Jason Wessel
@ 2010-07-28 21:17 ` Frederic Weisbecker
  2010-07-28 21:26   ` Linus Torvalds
  0 siblings, 1 reply; 11+ messages in thread
From: Frederic Weisbecker @ 2010-07-28 21:17 UTC (permalink / raw)
  To: Jason Wessel; +Cc: torvalds, linux-kernel, kgdb-bugreport

On Wed, Jul 28, 2010 at 03:39:49PM -0500, Jason Wessel wrote:
> Linus, please pull the for_linus branch for 2.6.35-rc6.  
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/jwessel/linux-2.6-kgdb.git for_linus
> 
> The kgdb validation tests found a regression where HW breakpoints
> stopped working with kdb / kgdb.  The problem was bisected and after
> talking with Frederic Weisbecker a patch was generated to repair the
> regression.
> 
> Thanks,
> Jason.
> 
> 
> ---
> Jason Wessel (1):
>       x86,kgdb: Fix hw breakpoint regression
> 
>  arch/x86/kernel/kgdb.c |   12 ++++++++++--
>  1 files changed, 10 insertions(+), 2 deletions(-)



I'm sorry I spot it a bit late.
There is a little issue in this patch, irqs won't
be restored:

@@ -588,9 +588,10 @@ int kgdb_ll_trap(int cmd, const char *str,
        if (!kgdb_io_module_registered)
                return NOTIFY_DONE;
 
+       local_irq_save(flags);
        return __kgdb_notify(&args, cmd);
+       local_irq_restore(flags);
 }


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [GIT PULL] kgdb regression fixes for 2.6.35-rc5
@ 2010-07-28 20:39 Jason Wessel
  2010-07-28 21:17 ` Frederic Weisbecker
  0 siblings, 1 reply; 11+ messages in thread
From: Jason Wessel @ 2010-07-28 20:39 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel, kgdb-bugreport

Linus, please pull the for_linus branch for 2.6.35-rc6.  

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

The kgdb validation tests found a regression where HW breakpoints
stopped working with kdb / kgdb.  The problem was bisected and after
talking with Frederic Weisbecker a patch was generated to repair the
regression.

Thanks,
Jason.


---
Jason Wessel (1):
      x86,kgdb: Fix hw breakpoint regression

 arch/x86/kernel/kgdb.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-07-29  1:06 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-07-22  0:36 [GIT PULL] kgdb regression fixes for 2.6.35-rc5 Jason Wessel
2010-07-22  0:36 ` [PATCH 1/5] kdb: break out of kdb_ll() when command is terminated Jason Wessel
2010-07-22  0:36   ` [PATCH 2/5] repair gdbstub to match the gdbserial protocol specification Jason Wessel
2010-07-22  0:36     ` [PATCH 3/5] Fix merge regression from external kdb to upstream kdb Jason Wessel
2010-07-22  0:36       ` [PATCH 4/5] debug_core,kdb: fix kgdb_connected bit set in the wrong place Jason Wessel
2010-07-22  0:36         ` [PATCH 5/5] sysrq,kdb: Use __handle_sysrq() for kdb's sysrq function Jason Wessel
2010-07-28 20:39 [GIT PULL] kgdb regression fixes for 2.6.35-rc5 Jason Wessel
2010-07-28 21:17 ` Frederic Weisbecker
2010-07-28 21:26   ` Linus Torvalds
2010-07-29  0:14     ` Jason Wessel
2010-07-29  1:06       ` Frederic Weisbecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®