All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk,
	Jason Wessel <jason.wessel@windriver.com>
Subject: [ 36/59] kgdbts: (2 of 2) fix single step awareness to work correctly with SMP
Date: Wed, 11 Apr 2012 16:11:11 -0700	[thread overview]
Message-ID: <20120411231041.557802263@linuxfoundation.org> (raw)
In-Reply-To: <20120411231213.GA13124@kroah.com>

3.2-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Jason Wessel <jason.wessel@windriver.com>

commit 23bbd8e346f1ef3fc1219c79cea53d8d52b207d8 upstream.

The do_fork and sys_open tests have never worked properly on anything
other than a UP configuration with the kgdb test suite.  This is
because the test suite did not fully implement the behavior of a real
debugger.  A real debugger tracks the state of what thread it asked to
single step and can correctly continue other threads of execution or
conditionally stop while waiting for the original thread single step
request to return.

Below is a simple method to cause a fatal kernel oops with the kgdb
test suite on a 2 processor ARM system:

while [ 1 ] ; do ls > /dev/null 2> /dev/null; done&
while [ 1 ] ; do ls > /dev/null 2> /dev/null; done&
echo V1I1F100 > /sys/module/kgdbts/parameters/kgdbts

Very soon after starting the test the kernel will start warning with
messages like:

kgdbts: BP mismatch c002487c expected c0024878
------------[ cut here ]------------
WARNING: at drivers/misc/kgdbts.c:317 check_and_rewind_pc+0x9c/0xc4()
[<c01f6520>] (check_and_rewind_pc+0x9c/0xc4)
[<c01f595c>] (validate_simple_test+0x3c/0xc4)
[<c01f60d4>] (run_simple_test+0x1e8/0x274)

The kernel will eventually recovers, but the test suite has completely
failed to test anything useful.

This patch implements behavior similar to a real debugger that does
not rely on hardware single stepping by using only software planted
breakpoints.

In order to mimic a real debugger, the kgdb test suite now tracks the
most recent thread that was continued (cont_thread_id), with the
intent to single step just this thread.  When the response to the
single step request stops in a different thread that hit the original
break point that thread will now get continued, while the debugger
waits for the thread with the single step pending.  Here is a high
level description of the sequence of events.

   cont_instead_of_sstep = 0;

1) set breakpoint at do_fork
2) continue
3)   Save the thread id where we stop to cont_thread_id
4) Remove breakpoint at do_fork
5) Reset the PC if needed depending on kernel exception type
6) soft single step
7)   Check where we stopped
       if current thread != cont_thread_id {
           if (here for more than 2 times for the same thead) {
              ### must be a really busy system, start test again ###
	      goto step 1
           }
           goto step 5
       } else {
           cont_instead_of_sstep = 0;
       }
8) clean up and run test again if needed
9) Clear out any threads that were waiting on a break point at the
   point in time the test is ended with get_cont_catch().  This
   happens sometimes because breakpoints are used in place of single
   stepping and some threads could have been in the debugger exception
   handling queue because breakpoints were hit concurrently on
   different CPUs.  This also means we wait at least one second before
   unplumbing the debugger connection at the very end, so as respond
   to any debug threads waiting to be serviced.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 drivers/misc/kgdbts.c |   73 ++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 62 insertions(+), 11 deletions(-)

--- a/drivers/misc/kgdbts.c
+++ b/drivers/misc/kgdbts.c
@@ -142,7 +142,9 @@ static int arch_needs_sstep_emulation =
 #else
 static int arch_needs_sstep_emulation;
 #endif
+static unsigned long cont_addr;
 static unsigned long sstep_addr;
+static int restart_from_top_after_write;
 static int sstep_state;
 
 /* Storage for the registers, in GDB format. */
@@ -190,7 +192,8 @@ static int kgdbts_unreg_thread(void *ptr
 	 */
 	while (!final_ack)
 		msleep_interruptible(1500);
-
+	/* Pause for any other threads to exit after final ack. */
+	msleep_interruptible(1000);
 	if (configured)
 		kgdb_unregister_io_module(&kgdbts_io_ops);
 	configured = 0;
@@ -312,13 +315,21 @@ static int check_and_rewind_pc(char *put
 	if (addr + BREAK_INSTR_SIZE == ip)
 		offset = -BREAK_INSTR_SIZE;
 #endif
-	if (strcmp(arg, "silent") && ip + offset != addr) {
+
+	if (arch_needs_sstep_emulation && sstep_addr &&
+	    ip + offset == sstep_addr &&
+	    ((!strcmp(arg, "sys_open") || !strcmp(arg, "do_fork")))) {
+		/* This is special case for emulated single step */
+		v2printk("Emul: rewind hit single step bp\n");
+		restart_from_top_after_write = 1;
+	} else if (strcmp(arg, "silent") && ip + offset != addr) {
 		eprintk("kgdbts: BP mismatch %lx expected %lx\n",
 			   ip + offset, addr);
 		return 1;
 	}
 	/* Readjust the instruction pointer if needed */
 	ip += offset;
+	cont_addr = ip;
 #ifdef GDB_ADJUSTS_BREAK_OFFSET
 	instruction_pointer_set(&kgdbts_regs, ip);
 #endif
@@ -328,6 +339,8 @@ static int check_and_rewind_pc(char *put
 static int check_single_step(char *put_str, char *arg)
 {
 	unsigned long addr = lookup_addr(arg);
+	static int matched_id;
+
 	/*
 	 * From an arch indepent point of view the instruction pointer
 	 * should be on a different instruction
@@ -338,17 +351,28 @@ static int check_single_step(char *put_s
 	v2printk("Singlestep stopped at IP: %lx\n",
 		   instruction_pointer(&kgdbts_regs));
 
-	if (sstep_thread_id != cont_thread_id && !arch_needs_sstep_emulation) {
+	if (sstep_thread_id != cont_thread_id) {
 		/*
 		 * Ensure we stopped in the same thread id as before, else the
 		 * debugger should continue until the original thread that was
 		 * single stepped is scheduled again, emulating gdb's behavior.
 		 */
 		v2printk("ThrID does not match: %lx\n", cont_thread_id);
+		if (arch_needs_sstep_emulation) {
+			if (matched_id &&
+			    instruction_pointer(&kgdbts_regs) != addr)
+				goto continue_test;
+			matched_id++;
+			ts.idx -= 2;
+			sstep_state = 0;
+			return 0;
+		}
 		cont_instead_of_sstep = 1;
 		ts.idx -= 4;
 		return 0;
 	}
+continue_test:
+	matched_id = 0;
 	if (instruction_pointer(&kgdbts_regs) == addr) {
 		eprintk("kgdbts: SingleStep failed at %lx\n",
 			   instruction_pointer(&kgdbts_regs));
@@ -390,6 +414,31 @@ static int got_break(char *put_str, char
 	return 1;
 }
 
+static void get_cont_catch(char *arg)
+{
+	/* Always send detach because the test is completed at this point */
+	fill_get_buf("D");
+}
+
+static int put_cont_catch(char *put_str, char *arg)
+{
+	/* This is at the end of the test and we catch any and all input */
+	v2printk("kgdbts: cleanup task: %lx\n", sstep_thread_id);
+	ts.idx--;
+	return 0;
+}
+
+static int emul_reset(char *put_str, char *arg)
+{
+	if (strncmp(put_str, "$OK", 3))
+		return 1;
+	if (restart_from_top_after_write) {
+		restart_from_top_after_write = 0;
+		ts.idx = -1;
+	}
+	return 0;
+}
+
 static void emul_sstep_get(char *arg)
 {
 	if (!arch_needs_sstep_emulation) {
@@ -443,8 +492,7 @@ static int emul_sstep_put(char *put_str,
 		v2printk("Stopped at IP: %lx\n",
 			 instruction_pointer(&kgdbts_regs));
 		/* Want to stop at IP + break instruction size by default */
-		sstep_addr = instruction_pointer(&kgdbts_regs) +
-			BREAK_INSTR_SIZE;
+		sstep_addr = cont_addr + BREAK_INSTR_SIZE;
 		break;
 	case 2:
 		if (strncmp(put_str, "$OK", 3)) {
@@ -456,6 +504,9 @@ static int emul_sstep_put(char *put_str,
 		if (strncmp(put_str, "$T0", 3)) {
 			eprintk("kgdbts: failed continue sstep\n");
 			return 1;
+		} else {
+			char *ptr = &put_str[11];
+			kgdb_hex2long(&ptr, &sstep_thread_id);
 		}
 		break;
 	case 4:
@@ -558,13 +609,13 @@ static struct test_struct do_fork_test[]
 	{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
 	{ "do_fork", "OK", sw_rem_break }, /*remove breakpoint */
 	{ "g", "do_fork", NULL, check_and_rewind_pc }, /* check location */
-	{ "write", "OK", write_regs }, /* Write registers */
+	{ "write", "OK", write_regs, emul_reset }, /* Write registers */
 	{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
 	{ "g", "do_fork", NULL, check_single_step },
 	{ "do_fork", "OK", sw_break, }, /* set sw breakpoint */
 	{ "7", "T0*", skip_back_repeat_test }, /* Loop based on repeat_test */
 	{ "D", "OK", NULL, final_ack_set }, /* detach and unregister I/O */
-	{ "", "" },
+	{ "", "", get_cont_catch, put_cont_catch },
 };
 
 /* Test for hitting a breakpoint at sys_open for what ever the number
@@ -576,13 +627,13 @@ static struct test_struct sys_open_test[
 	{ "c", "T0*", NULL, get_thread_id_continue }, /* Continue */
 	{ "sys_open", "OK", sw_rem_break }, /*remove breakpoint */
 	{ "g", "sys_open", NULL, check_and_rewind_pc }, /* check location */
-	{ "write", "OK", write_regs }, /* Write registers */
+	{ "write", "OK", write_regs, emul_reset }, /* Write registers */
 	{ "s", "T0*", emul_sstep_get, emul_sstep_put }, /* Single step */
 	{ "g", "sys_open", NULL, check_single_step },
 	{ "sys_open", "OK", sw_break, }, /* set sw breakpoint */
 	{ "7", "T0*", skip_back_repeat_test }, /* Loop based on repeat_test */
 	{ "D", "OK", NULL, final_ack_set }, /* detach and unregister I/O */
-	{ "", "" },
+	{ "", "", get_cont_catch, put_cont_catch },
 };
 
 /*
@@ -725,8 +776,8 @@ static int run_simple_test(int is_get_ch
 	/* This callback is a put char which is when kgdb sends data to
 	 * this I/O module.
 	 */
-	if (ts.tst[ts.idx].get[0] == '\0' &&
-		ts.tst[ts.idx].put[0] == '\0') {
+	if (ts.tst[ts.idx].get[0] == '\0' && ts.tst[ts.idx].put[0] == '\0' &&
+	    !ts.tst[ts.idx].get_handler) {
 		eprintk("kgdbts: ERROR: beyond end of test on"
 			   " '%s' line %i\n", ts.name, ts.idx);
 		return 0;



  parent reply	other threads:[~2012-04-11 23:23 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-11 23:12 [ 00/59] 3.2.15-stable review Greg KH
2012-04-11 23:10 ` [ 01/59] x86 bpf_jit: fix a bug in emitting the 16-bit immediate operand of AND Greg KH
2012-04-11 23:10 ` [ 02/59] tg3: Fix 5717 serdes powerdown problem Greg KH
2012-04-11 23:10 ` [ 03/59] sky2: dont overwrite settings for PHY Quick link Greg KH
2012-04-11 23:10 ` [ 04/59] rose_dev: fix memcpy-bug in rose_set_mac_address Greg KH
2012-04-11 23:10 ` [ 05/59] net: usb: cdc_eem: fix mtu Greg KH
2012-04-11 23:10 ` [ 06/59] Fix non TBI PHY access; a bad merge undid bug fix in a previous commit Greg KH
2012-04-12  4:55   ` Ben Hutchings
     [not found]     ` <OFFBE17A27.42E05B7A-ONC12579DE.00262282-C12579DE.0026AD98@transmode.se>
2012-04-12 15:02       ` Ben Hutchings
2012-04-16  0:16     ` Paul Gortmaker
2012-04-16  0:16       ` Paul Gortmaker
2012-04-16  0:32       ` Ben Hutchings
2012-04-11 23:10 ` [ 07/59] ASoC: wm8994: Update WM8994 DCS calibration Greg KH
2012-04-11 23:10 ` [ 08/59] mtd: ixp4xx: oops in ixp4xx_flash_probe Greg KH
2012-04-11 23:10 ` [ 09/59] mtd: mips: lantiq: reintroduce support for cmdline partitions Greg KH
2012-04-11 23:10 ` [ 10/59] mtd: nand: gpmi: use correct member for checking NAND_BBT_USE_FLASH Greg KH
2012-04-11 23:10 ` [ 11/59] mtd: sst25l: initialize writebufsize Greg KH
2012-04-11 23:10 ` [ 12/59] mtd: block2mtd: " Greg KH
2012-04-11 23:10 ` [ 13/59] mtd: lart: " Greg KH
2012-04-11 23:10 ` [ 14/59] mtd: m25p80: set writebufsize Greg KH
2012-04-11 23:10 ` [ 15/59] ACPI: Do cpufreq clamping for throttling per package v2 Greg KH
2012-04-11 23:10 ` [ 16/59] PNPACPI: Fix device ref leaking in acpi_pnp_match Greg KH
2012-04-11 23:10 ` [ 17/59] ACPICA: Fix regression in FADT revision checks Greg KH
2012-04-11 23:10 ` [ 18/59] modpost: fix ALL_INIT_DATA_SECTIONS Greg KH
2012-04-11 23:10 ` [ 19/59] genirq: Adjust irq thread affinity on IRQ_SET_MASK_OK_NOCOPY return value Greg KH
2012-04-11 23:10 ` [ 20/59] tracing: Fix ftrace stack trace entries Greg KH
2012-04-11 23:10 ` [ 21/59] tracing: Fix ent_size in trace output Greg KH
2012-04-11 23:10 ` [ 22/59] m68k/mac: Add missing platform check before registering platform devices Greg KH
2012-04-11 23:10 ` [ 23/59] mac80211: fix possible tid_rx->reorder_timer use after free Greg KH
2012-04-11 23:10 ` [ 24/59] rtlwifi: rtl8192ce: rtl8192cu: rtl8192de: Fix low-gain setting when scanning Greg KH
2012-04-11 23:11 ` [ 25/59] drm: Validate requested virtual size against allocated fb size Greg KH
2012-04-11 23:11 ` [ 26/59] drm/radeon/kms: fix fans after resume Greg KH
2012-04-11 23:11 ` [ 27/59] drm/i915: no-lvds quirk on MSI DC500 Greg KH
2012-04-11 23:11 ` [ 28/59] drm/i915: Sanitize BIOS debugging bits from PIPECONF Greg KH
2012-04-11 23:11 ` [ 29/59] drm/i915: Add lock on drm_helper_resume_force_mode Greg KH
2012-04-11 23:11 ` [ 30/59] drm/i915: quirk away broken OpRegion VBT Greg KH
2012-04-11 23:11 ` [ 31/59] r8169: runtime resume before shutdown Greg KH
2012-04-11 23:11 ` [ 32/59] target: Fix unsupported WRITE_SAME sense payload Greg KH
2012-04-11 23:11 ` [ 33/59] kgdb,debug_core: pass the breakpoint struct instead of address and memory Greg KH
2012-04-11 23:11 ` [ 34/59] kgdbts: Fix kernel oops with CONFIG_DEBUG_RODATA Greg KH
2012-04-11 23:11 ` [ 35/59] kgdbts: (1 of 2) fix single step awareness to work correctly with SMP Greg KH
2012-04-11 23:11 ` Greg KH [this message]
2012-04-11 23:11 ` [ 37/59] x86,kgdb: Fix DEBUG_RODATA limitation using text_poke() Greg KH
2012-04-11 23:11 ` [ 38/59] CIFS: Fix VFS lock usage for oplocked files Greg KH
2012-04-11 23:11 ` [ 39/59] [PATCH] ARM: tegra: remove Tegra30 errata from MACH_TEGRA_DT Greg KH
2012-04-11 23:11 ` [ 40/59] mmc: sdhci-dove: Fix compile error by including module.h Greg KH
2012-04-11 23:11 ` [ 41/59] mmc: atmel-mci: correct data timeout computation Greg KH
2012-04-11 23:11 ` [ 42/59] tcm_fc: Add abort flag for gracefully handling exchange timeout Greg KH
2012-04-11 23:11 ` [ 43/59] tcm_fc: Do not free tpg structure during wq allocation failure Greg KH
2012-04-11 23:11 ` [ 44/59] sysctl: fix write access to dmesg_restrict/kptr_restrict Greg KH
2012-04-11 23:11 ` [ 45/59] modpost: Fix modpost license checking of vmlinux.o Greg KH
2012-04-11 23:11 ` [ 46/59] x86/PCI: use host bridge _CRS info on MSI MS-7253 Greg KH
2012-04-11 23:11 ` [ 47/59] x86/PCI: do not tie MSI MS-7253 use_crs quirk to BIOS version Greg KH
2012-04-11 23:11 ` [ 48/59] TOMOYO: Fix mount flags checking order Greg KH
2012-04-11 23:11 ` [ 49/59] Revert "x86/ioapic: Add register level checks to detect bogus io-apic entries" Greg KH
2012-04-11 23:11 ` [ 50/59] acer-wmi: No wifi rfkill on Sony machines Greg KH
2012-04-11 23:11 ` [ 51/59] Fix length of buffer copied in __nfs4_get_acl_uncached Greg KH
2012-04-11 23:11 ` [ 52/59] sched/x86: Fix overflow in cyc2ns_offset Greg KH
2012-04-11 23:11 ` [ 53/59] mfd: Clear twl6030 IRQ status register only once Greg KH
2012-04-11 23:11 ` [ 54/59] USB: Add Motorola Rokr E6 Id to the USBNet driver "zaurus" Greg KH
2012-04-11 23:11 ` [ 55/59] ioat: fix size of completion for Xen Greg KH
2012-04-11 23:11 ` [ 56/59] ASoC: ak4642: fixup: mute needs +1 step Greg KH
2012-04-11 23:11 ` [ 57/59] cred: copy_process() should clear child->replacement_session_keyring Greg KH
2012-04-11 23:11 ` [ 58/59] iommu/amd: Make sure IOMMU interrupts are re-enabled on resume Greg KH
2012-04-11 23:11 ` [ 59/59] Bluetooth: Fix l2cap conn failures for ssp devices Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120411231041.557802263@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=jason.wessel@windriver.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.