Skip to contentSkip to conversation

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

PATCH In review Douglas Miller 2 participants 4 messages Nov 10, 2015 – Nov 13, 2015 PowerPC

lore
Share & tools
lore

Douglas Miller

author
#0 lore raw

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

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

Quoted 1 line

From 5f2dd7c955443332835de86cfde2097e298fc657 Mon Sep 17 00:00:00 2001

From: Douglas Miller <[email protected]>
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(-)
arch/powerpc/xmon/xmon.c +37 -0
index 13c6e20..745ecf5 100644
@@ -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];
Signature · 1 line
1.7.1
author
#1 lore raw

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

Here's is the corrected patch.

Quoted 10 lines · Douglas Miller

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

Quoted 6 lines

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

From 18a3fff40f84973639ddf1e519a5976b90cec33f Mon Sep 17 00:00:00 2001

From: Douglas Miller <[email protected]>
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(-)
arch/powerpc/xmon/xmon.c +38 -0
index 13c6e20..72f0404 100644
@@ -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];
Signature · 1 line
1.7.1
#2 lore raw

arch/powerpc/xmon/xmon.c memzcan(void) · line 2490

Quoted 13 lines · Douglas Miller
arch/powerpc/xmon/xmon.c +13 -0
+{
+	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.

author
#3 lore raw

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

Quoted 29 lines · Benjamin Herrenschmidt

Review

Quoted 13 lines · Douglas Miller
(quoted diff) +13 -0
+{
+	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 !)

Quoted 6 lines

Cheers,
Ben.

From a0e7eeef8c51ecedad131aa4c04541f37fc083c4 Mon Sep 17 00:00:00 2001

From: Douglas Miller <[email protected]>
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(-)
arch/powerpc/xmon/xmon.c +58 -0
index 13c6e20..5c24f55 100644
@@ -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];
Signature · 1 line
1.7.1