[xmon PATCH 1/1] RFC: Add xmon command to dump process/task similar to ps(1)

[xmon PATCH 1/1] RFC: Add xmon command to dump process/task similar to ps(1)

[Douglas Miller]

[-- Attachment #1: Type: text/plain, Size: 615 bytes --]

RFC: Not sure if I need locking when accessing task structs from xmon. 
Please comment.

I found this new xmon command indispensable while debugging a KDUMP 
problem. It dumps some task information similar to the ps(1) command, 
plus includes the kernel stack pointer for use with "t" traceback 
command. In the case of KDUMP on PKVM, it was running single-CPU so 
locking issues would likely not have shown up. Certainly if xmon runs in 
an environment where some CPUs are still running tasks, this information 
can be very fleeting.

Let me know what else needs to be done to make this patch ready.

Thanks,
Doug

[-- Attachment #2: 0001-Add-xmon-command-to-dump-process-task-similar-to-ps-.patch --]
[-- Type: text/x-patch, Size: 2093 bytes --]

>From 5f2dd7c955443332835de86cfde2097e298fc657 Mon Sep 17 00:00:00 2001
From: Douglas Miller <dougmill@linux.vnet.ibm.com>
Date: Thu, 5 Nov 2015 07:55:33 -0600
Subject: [PATCH] Add xmon command to dump process/task similar to ps(1)

---
 arch/powerpc/xmon/xmon.c |   37 +++++++++++++++++++++++++++++++++++++
 1 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 13c6e20..745ecf5 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -149,6 +149,7 @@ static int  cpu_cmd(void);
 static void csum(void);
 static void bootcmds(void);
 static void proccall(void);
+static void proclist(void);
 void dump_segments(void);
 static void symbol_lookup(void);
 static void xmon_show_stack(unsigned long sp, unsigned long lr,
@@ -228,6 +229,7 @@ Commands:\n\
   mz	zero a block of memory\n\
   mi	show information about memory allocation\n\
   p 	call a procedure\n\
+  P 	list processes/tasks\n\
   r	print registers\n\
   s	single step\n"
 #ifdef CONFIG_SPU_BASE
@@ -947,6 +949,9 @@ cmds(struct pt_regs *excp)
 		case 'p':
 			proccall();
 			break;
+		case 'P':
+			proclist();
+			break;
 #ifdef CONFIG_PPC_STD_MMU
 		case 'u':
 			dump_segments();
@@ -2450,6 +2455,38 @@ memzcan(void)
 		printf("%.8x\n", a - mskip);
 }
 
+static void procshow(struct task_struct *tsk)
+{
+	char state;
+	state = (tsk->state == 0) ? 'R' : 
+		(tsk->state < 0) ? 'U' : 
+		(tsk->state & TASK_UNINTERRUPTIBLE) ? 'D' : 
+		(tsk->state & TASK_STOPPED) ? 'T' : 
+		(tsk->state & TASK_TRACED) ? 'C' : 
+		(tsk->exit_state & EXIT_ZOMBIE) ? 'Z' : 
+		(tsk->exit_state & EXIT_DEAD) ? 'E' : 
+		(tsk->state & TASK_INTERRUPTIBLE) ? 'S' : '?';
+
+	printf("%p %016lx %6d %6d %c %2d %s\n", tsk,
+		tsk->thread.ksp,
+		tsk->pid, tsk->parent->pid,
+		state, task_thread_info(tsk)->cpu,
+		tsk->comm);
+}
+
+static void proclist(void)
+{
+	struct task_struct *tsk;
+
+	if (scanhex(&tsk)) {
+		procshow(tsk);
+	} else {
+		for_each_process(tsk) {
+			procshow(tsk);
+		}
+	}
+}
+
 static void proccall(void)
 {
 	unsigned long args[8];
-- 
1.7.1

Re: [xmon PATCH 1/1] RFC: Add xmon command to dump process/task similar to ps(1)

[Douglas Miller]

[-- Attachment #1.1: Type: text/plain, Size: 956 bytes --]

Sorry, rookie mistake: last minute changes without checking compile.

Here's is the corrected patch.



On 11/10/2015 09:31 AM, Douglas Miller wrote:
> RFC: Not sure if I need locking when accessing task structs from xmon. 
> Please comment.
>
> I found this new xmon command indispensable while debugging a KDUMP 
> problem. It dumps some task information similar to the ps(1) command, 
> plus includes the kernel stack pointer for use with "t" traceback 
> command. In the case of KDUMP on PKVM, it was running single-CPU so 
> locking issues would likely not have shown up. Certainly if xmon runs 
> in an environment where some CPUs are still running tasks, this 
> information can be very fleeting.
>
> Let me know what else needs to be done to make this patch ready.
>
> Thanks,
> Doug
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev


[-- Attachment #1.2: Type: text/html, Size: 1707 bytes --]

[-- Attachment #2: 0001-Add-xmon-command-to-dump-process-task-similar-to-ps-.patch --]
[-- Type: text/x-patch, Size: 2133 bytes --]

>From 18a3fff40f84973639ddf1e519a5976b90cec33f Mon Sep 17 00:00:00 2001
From: Douglas Miller <dougmill@linux.vnet.ibm.com>
Date: Thu, 5 Nov 2015 07:55:33 -0600
Subject: [PATCH] Add xmon command to dump process/task similar to ps(1)

---
 arch/powerpc/xmon/xmon.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 13c6e20..72f0404 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -149,6 +149,7 @@ static int  cpu_cmd(void);
 static void csum(void);
 static void bootcmds(void);
 static void proccall(void);
+static void proclist(void);
 void dump_segments(void);
 static void symbol_lookup(void);
 static void xmon_show_stack(unsigned long sp, unsigned long lr,
@@ -228,6 +229,7 @@ Commands:\n\
   mz	zero a block of memory\n\
   mi	show information about memory allocation\n\
   p 	call a procedure\n\
+  P 	list processes/tasks\n\
   r	print registers\n\
   s	single step\n"
 #ifdef CONFIG_SPU_BASE
@@ -947,6 +949,9 @@ cmds(struct pt_regs *excp)
 		case 'p':
 			proccall();
 			break;
+		case 'P':
+			proclist();
+			break;
 #ifdef CONFIG_PPC_STD_MMU
 		case 'u':
 			dump_segments();
@@ -2450,6 +2455,39 @@ memzcan(void)
 		printf("%.8x\n", a - mskip);
 }
 
+static void procshow(struct task_struct *tsk)
+{
+	char state;
+	state = (tsk->state == 0) ? 'R' :
+		(tsk->state < 0) ? 'U' :
+		(tsk->state & TASK_UNINTERRUPTIBLE) ? 'D' :
+		(tsk->state & TASK_STOPPED) ? 'T' :
+		(tsk->state & TASK_TRACED) ? 'C' :
+		(tsk->exit_state & EXIT_ZOMBIE) ? 'Z' :
+		(tsk->exit_state & EXIT_DEAD) ? 'E' :
+		(tsk->state & TASK_INTERRUPTIBLE) ? 'S' : '?';
+
+	printf("%p %016lx %6d %6d %c %2d %s\n", tsk,
+		tsk->thread.ksp,
+		tsk->pid, tsk->parent->pid,
+		state, task_thread_info(tsk)->cpu,
+		tsk->comm);
+}
+
+static void proclist(void)
+{
+	unsigned long tskv;
+	struct task_struct *tsk;
+
+	if (scanhex(&tskv)) {
+		procshow((struct task_struct *)tskv);
+	} else {
+		for_each_process(tsk) {
+			procshow(tsk);
+		}
+	}
+}
+
 static void proccall(void)
 {
 	unsigned long args[8];
-- 
1.7.1

Re: [xmon PATCH 1/1] RFC: Add xmon command to dump process/task similar to ps(1)

[Benjamin Herrenschmidt]

On Tue, 2015-11-10 at 10:50 -0600, Douglas Miller wrote:
> +{
> +	unsigned long tskv;
> +	struct task_struct *tsk;
> +
> +	if (scanhex(&tskv)) {
> +		procshow((struct task_struct *)tskv);
> +	} else {
> +		for_each_process(tsk) {
> +			procshow(tsk);
> +		}
> +	}
> +}
> +

The main thing is wrap it with the setjump magic that xmon uses for
other things that allows it to recover if you hit a bad pointer
along the way:

	if (setjmp(bus_error_jmp) == 0) {
		catch_memory_errors = 1;
		sync();
		... do your stuff...
		sync();
	}
	catch_memory_errors = 0;

We could make some kind of helper or macro to factor that out
while it it ;-) (For bonus points !)

Cheers,
Ben.

Re: [xmon PATCH 1/1] RFC: Add xmon command to dump process/task similar to ps(1)

[Douglas Miller]

[-- Attachment #1: Type: text/plain, Size: 1499 bytes --]

Thanks, Ben, for the feedback.

Here is an updated patch. I added a macro XMON_PROTECT() to wrap a 
statement in what I think is the correct setjmp/protection code. Each 
place seems to do it slightly differently, so I wasn't sure what the 
right way was and didn't want to change all other places until we were 
certain. If you have a better way to implement this, please let me know. 
It may be difficult to make a macro that fits all use cases, too.

I checked using a task point of NULL and it errored out safely, and also 
tried a pointer to valid memory but not a valid task_struct and it also 
errored out cleanly. Of course, random memory might not always cause a 
fault but rather just print garbage.

Thanks,
Doug

On 11/11/2015 02:09 AM, Benjamin Herrenschmidt wrote:
> On Tue, 2015-11-10 at 10:50 -0600, Douglas Miller wrote:
>> +{
>> +	unsigned long tskv;
>> +	struct task_struct *tsk;
>> +
>> +	if (scanhex(&tskv)) {
>> +		procshow((struct task_struct *)tskv);
>> +	} else {
>> +		for_each_process(tsk) {
>> +			procshow(tsk);
>> +		}
>> +	}
>> +}
>> +
> The main thing is wrap it with the setjump magic that xmon uses for
> other things that allows it to recover if you hit a bad pointer
> along the way:
>
> 	if (setjmp(bus_error_jmp) == 0) {
> 		catch_memory_errors = 1;
> 		sync();
> 		... do your stuff...
> 		sync();
> 	}
> 	catch_memory_errors = 0;
>
> We could make some kind of helper or macro to factor that out
> while it it ;-) (For bonus points !)
>
> Cheers,
> Ben.
>
>


[-- Attachment #2: 0001-Add-xmon-command-to-dump-process-task-similar-to-ps-.patch --]
[-- Type: text/x-patch, Size: 2937 bytes --]

>From a0e7eeef8c51ecedad131aa4c04541f37fc083c4 Mon Sep 17 00:00:00 2001
From: Douglas Miller <dougmill@linux.vnet.ibm.com>
Date: Thu, 5 Nov 2015 07:55:33 -0600
Subject: [PATCH] Add xmon command to dump process/task similar to ps(1)

---
 arch/powerpc/xmon/xmon.c |   58 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 13c6e20..5c24f55 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -149,6 +149,7 @@ static int  cpu_cmd(void);
 static void csum(void);
 static void bootcmds(void);
 static void proccall(void);
+static void proclist(void);
 void dump_segments(void);
 static void symbol_lookup(void);
 static void xmon_show_stack(unsigned long sp, unsigned long lr,
@@ -191,6 +192,24 @@ extern void xmon_leave(void);
 			 || ('A' <= (c) && (c) <= 'Z'))
 #define isspace(c)	(c == ' ' || c == '\t' || c == 10 || c == 13 || c == 0)
 
+/*
+ * Wrap a statement (typically function call) in setjmp to
+ * protect it from memory access errors. msg... are printf
+ * fmt+args used if error is trapped.
+ */
+#define XMON_PROTECT(stmt, msg...) 		\
+	if (setjmp(bus_error_jmp) != 0) {	\
+		catch_memory_errors = 0;	\
+		printf(msg);			\
+	} else {				\
+		catch_memory_errors = 1;	\
+		sync();				\
+		stmt;				\
+		sync();				\
+		__delay(200);			\
+		catch_memory_errors = 0;	\
+	}
+
 static char *help_string = "\
 Commands:\n\
   b	show breakpoints\n\
@@ -228,6 +247,7 @@ Commands:\n\
   mz	zero a block of memory\n\
   mi	show information about memory allocation\n\
   p 	call a procedure\n\
+  P 	list processes/tasks\n\
   r	print registers\n\
   s	single step\n"
 #ifdef CONFIG_SPU_BASE
@@ -947,6 +967,9 @@ cmds(struct pt_regs *excp)
 		case 'p':
 			proccall();
 			break;
+		case 'P':
+			proclist();
+			break;
 #ifdef CONFIG_PPC_STD_MMU
 		case 'u':
 			dump_segments();
@@ -2450,6 +2473,41 @@ memzcan(void)
 		printf("%.8x\n", a - mskip);
 }
 
+static void procshow(struct task_struct *tsk)
+{
+	char state;
+
+	state = (tsk->state == 0) ? 'R' :
+		(tsk->state < 0) ? 'U' :
+		(tsk->state & TASK_UNINTERRUPTIBLE) ? 'D' :
+		(tsk->state & TASK_STOPPED) ? 'T' :
+		(tsk->state & TASK_TRACED) ? 'C' :
+		(tsk->exit_state & EXIT_ZOMBIE) ? 'Z' :
+		(tsk->exit_state & EXIT_DEAD) ? 'E' :
+		(tsk->state & TASK_INTERRUPTIBLE) ? 'S' : '?';
+
+	printf("%p %016lx %6d %6d %c %2d %s\n", tsk,
+		tsk->thread.ksp,
+		tsk->pid, tsk->parent->pid,
+		state, task_thread_info(tsk)->cpu,
+		tsk->comm);
+}
+
+static void proclist(void)
+{
+	unsigned long tskv;
+	struct task_struct *tsk;
+
+	if (scanhex(&tskv)) {
+		tsk = (struct task_struct *)tskv;
+		XMON_PROTECT(procshow(tsk), "*** Error dumping task %p\n", tsk);
+	} else {
+		for_each_process(tsk) {
+			XMON_PROTECT(procshow(tsk), "*** Error dumping task %p\n", tsk);
+		}
+	}
+}
+
 static void proccall(void)
 {
 	unsigned long args[8];
-- 
1.7.1