[PATCH v3 1/2] panic: add option to dump task maps info in panic_print

[PATCH v3 1/2] panic: add option to dump task maps info in panic_print

[qiwu.chen]

Currently, it's hard to debug panic issues caused by kill init,
since there is no debug info from user mode in current panic msg
such as the user_regs and maps info.

This patch adds an option to dump task maps info in panic_print.

Signed-off-by: qiwu.chen <qiwu.chen@transsion.com>
---
 .../admin-guide/kernel-parameters.txt         |  1 +
 Documentation/admin-guide/sysctl/kernel.rst   |  1 +
 kernel/panic.c                                | 82 +++++++++++++++++++
 3 files changed, 84 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 8337d0fed311..f76709deef6c 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -4253,6 +4253,7 @@
 			bit 5: print all printk messages in buffer
 			bit 6: print all CPUs backtrace (if available in the arch)
 			bit 7: print only tasks in uninterruptible (blocked) state
+			bit 8: print task maps info
 			*Be aware* that this option may print a _lot_ of lines,
 			so there are risks of losing older messages in the log.
 			Use this option carefully, maybe worth to setup a
diff --git a/Documentation/admin-guide/sysctl/kernel.rst b/Documentation/admin-guide/sysctl/kernel.rst
index f8bc1630eba0..558e365b76a9 100644
--- a/Documentation/admin-guide/sysctl/kernel.rst
+++ b/Documentation/admin-guide/sysctl/kernel.rst
@@ -872,6 +872,7 @@ bit 4  print ftrace buffer
 bit 5  print all printk messages in buffer
 bit 6  print all CPUs backtrace (if available in the arch)
 bit 7  print only tasks in uninterruptible (blocked) state
+bit 8  print task maps info
 =====  ============================================
 
 So for example to print tasks and memory info on panic, user can::
diff --git a/kernel/panic.c b/kernel/panic.c
index 753d12f4dc8f..5b76ff92a6fc 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -77,6 +77,8 @@ EXPORT_SYMBOL_GPL(panic_timeout);
 #define PANIC_PRINT_ALL_PRINTK_MSG	0x00000020
 #define PANIC_PRINT_ALL_CPU_BT		0x00000040
 #define PANIC_PRINT_BLOCKED_TASKS	0x00000080
+#define PANIC_PRINT_TASK_MAPS_INFO	0x00000100
+
 unsigned long panic_print;
 
 ATOMIC_NOTIFIER_HEAD(panic_notifier_list);
@@ -208,6 +210,83 @@ void nmi_panic(struct pt_regs *regs, const char *msg)
 }
 EXPORT_SYMBOL(nmi_panic);
 
+/*
+ * This function is called in panic proccess if the PANIC_PRINT_TASK_MAPS_INFO
+ * flag is specified in panic_print, which is helpful to debug panic issues due
+ * to an unhandled falut in user mode such as kill init.
+ */
+static void dump_task_maps_info(struct task_struct *tsk)
+{
+	struct pt_regs *user_ret = task_pt_regs(tsk);
+	struct mm_struct *mm = tsk->mm;
+	struct vm_area_struct *vma;
+
+	if (!mm || !user_mode(user_ret))
+		return;
+
+	pr_info("Dump task %s:%d maps start\n", tsk->comm, task_pid_nr(tsk));
+	mmap_read_lock(mm);
+	VMA_ITERATOR(vmi, mm, 0);
+	for_each_vma(vmi, vma) {
+		struct file *file = vma->vm_file;
+		int flags = vma->vm_flags;
+		unsigned long long pgoff = ((loff_t)vma->vm_pgoff) << PAGE_SHIFT;
+		struct anon_vma_name *anon_name = NULL;
+		struct mm_struct *mm = vma->vm_mm;
+		char buf[256] = {0};
+		const char *name = NULL;
+
+		if (mm)
+			anon_name = anon_vma_name(vma);
+
+		if (file) {
+			if (anon_name) {
+				snprintf(buf, sizeof(buf), "[anon_shmem:%s]", anon_name->name);
+				name = buf;
+			} else {
+				char *f_path = d_path(file_user_path(file), buf, sizeof(buf));
+
+				name = IS_ERR(f_path) ? "?" : f_path;
+			}
+			goto print;
+		}
+
+		if (vma->vm_ops && vma->vm_ops->name) {
+			name = vma->vm_ops->name(vma);
+			if (name)
+				goto print;
+		}
+
+		name = arch_vma_name(vma);
+		if (!name) {
+			if (mm) {
+				if (vma_is_initial_heap(vma))
+					name = "[heap]";
+				else if (vma_is_initial_stack(vma))
+					name = "[stack]";
+			} else
+				name = "[vdso]";
+
+			if (anon_name) {
+				snprintf(buf, sizeof(buf), "[anon:%s]", anon_name->name);
+				name = buf;
+			}
+		}
+
+print:
+		if (name)
+			pr_info("%08lx-%08lx %c%c%c%c %08llx %s\n",
+				vma->vm_start, vma->vm_end,
+				flags & VM_READ ? 'r' : '-',
+				flags & VM_WRITE ? 'w' : '-',
+				flags & VM_EXEC ? 'x' : '-',
+				flags & VM_MAYSHARE ? 's' : 'p',
+				pgoff, name);
+	}
+	mmap_read_unlock(mm);
+	pr_info("Dump task %s:%d maps end\n", tsk->comm, task_pid_nr(tsk));
+}
+
 static void panic_print_sys_info(bool console_flush)
 {
 	if (console_flush) {
@@ -233,6 +312,9 @@ static void panic_print_sys_info(bool console_flush)
 
 	if (panic_print & PANIC_PRINT_BLOCKED_TASKS)
 		show_state_filter(TASK_UNINTERRUPTIBLE);
+
+	if (panic_print & PANIC_PRINT_TASK_MAPS_INFO)
+		dump_task_maps_info(current);
 }
 
 void check_panic_on_warn(const char *origin)
-- 
2.25.1

[PATCH v3 2/2] exit: dump current pt_regs info on global init exit

[qiwu.chen]

Currently, it's hard to debug panic issues caused by kill init,
since there is no debug info from user mode in current panic msg
such as the user_regs and maps info.

This patch dumps current pt_regs info on global init exit for debugging.

Signed-off-by: qiwu.chen <qiwu.chen@transsion.com>
---
 kernel/exit.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 0d62a53605df..3d5aa88ba517 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -11,6 +11,7 @@
 #include <linux/sched/mm.h>
 #include <linux/sched/stat.h>
 #include <linux/sched/task.h>
+#include <linux/sched/debug.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sched/cputime.h>
 #include <linux/interrupt.h>
@@ -847,10 +848,12 @@ void __noreturn do_exit(long code)
 		 * If the last thread of global init has exited, panic
 		 * immediately to get a useable coredump.
 		 */
-		if (unlikely(is_global_init(tsk)))
+		if (unlikely(is_global_init(tsk))) {
+			/* dump the pt_regs of current thread for debugging. */
+			show_regs(task_pt_regs(tsk));
 			panic("Attempted to kill init! exitcode=0x%08x\n",
 				tsk->signal->group_exit_code ?: (int)code);
-
+		}
 #ifdef CONFIG_POSIX_TIMERS
 		hrtimer_cancel(&tsk->signal->real_timer);
 		exit_itimers(tsk);
-- 
2.25.1

Re: [PATCH v3 2/2] exit: dump current pt_regs info on global init exit

[Oleg Nesterov]

Add lkml.

On 09/22, qiwu.chen wrote:
>
> @@ -847,10 +848,12 @@ void __noreturn do_exit(long code)
>  		 * If the last thread of global init has exited, panic
>  		 * immediately to get a useable coredump.
>  		 */
> -		if (unlikely(is_global_init(tsk)))
> +		if (unlikely(is_global_init(tsk))) {
> +			/* dump the pt_regs of current thread for debugging. */
> +			show_regs(task_pt_regs(tsk));
>  			panic("Attempted to kill init! exitcode=0x%08x\n",
>  				tsk->signal->group_exit_code ?: (int)code);

Well, this means that show_regs() will be called twice if CONFIG_DEBUG_BUGVERBOSE
at least on x86, see dump_stack() in panic(). See also show_regs_if_on_stack()
in show_trace_log_lvl().

Not good...

Oleg.

Re: [PATCH v3 2/2] exit: dump current pt_regs info on global init exit

[chenqiwu]

On Sun, Sep 22, 2024 at 02:54:59PM +0200, Oleg Nesterov wrote:
> 
> Well, this means that show_regs() will be called twice if CONFIG_DEBUG_BUGVERBOSE
> at least on x86, see dump_stack() in panic(). See also show_regs_if_on_stack()
> in show_trace_log_lvl().
> 
> Not good...
> 
Okay, I see. The dump_stack() is arch-dependent function, it doesn't call show_regs()
on arm64. Do you have any suggestion where to dump regs without arch-dependence
on task exiting flow?

Thanks
Qiwu

Re: [PATCH v3 1/2] panic: add option to dump task maps info in panic_print

[Oleg Nesterov]

On 09/22, qiwu.chen wrote:
>
> +	for_each_vma(vmi, vma) {
> +		struct file *file = vma->vm_file;
> +		int flags = vma->vm_flags;
> +		unsigned long long pgoff = ((loff_t)vma->vm_pgoff) << PAGE_SHIFT;
> +		struct anon_vma_name *anon_name = NULL;
> +		struct mm_struct *mm = vma->vm_mm;
> +		char buf[256] = {0};
> +		const char *name = NULL;
> +
> +		if (mm)
> +			anon_name = anon_vma_name(vma);
> +
> +		if (file) {
> +			if (anon_name) {
> +				snprintf(buf, sizeof(buf), "[anon_shmem:%s]", anon_name->name);
> +				name = buf;
> +			} else {
> +				char *f_path = d_path(file_user_path(file), buf, sizeof(buf));
> +
> +				name = IS_ERR(f_path) ? "?" : f_path;
> +			}
> +			goto print;
> +		}
> +
> +		if (vma->vm_ops && vma->vm_ops->name) {
> +			name = vma->vm_ops->name(vma);
> +			if (name)
> +				goto print;
> +		}
> +
> +		name = arch_vma_name(vma);
> +		if (!name) {
> +			if (mm) {
> +				if (vma_is_initial_heap(vma))
> +					name = "[heap]";
> +				else if (vma_is_initial_stack(vma))
> +					name = "[stack]";
> +			} else
> +				name = "[vdso]";
> +
> +			if (anon_name) {
> +				snprintf(buf, sizeof(buf), "[anon:%s]", anon_name->name);
> +				name = buf;
> +			}
> +		}
> +

Wouldn't it be better to export/reuse get_vma_name() rather than duplicate
its code ?

Oleg.

Re: [PATCH v3 1/2] panic: add option to dump task maps info in panic_print

[chenqiwu]

On Sun, Sep 22, 2024 at 03:12:37PM +0200, Oleg Nesterov wrote:
> 
> Wouldn't it be better to export/reuse get_vma_name() rather than duplicate
> its code ?
> 
Sure, I will export get_vma_name() in task_mmu.c for reusing, and post
it as v4.
Thanks
Qiwu

Re: [PATCH v3 1/2] panic: add option to dump task maps info in panic_print

[Oleg Nesterov]

On 09/22, qiwu.chen wrote:
>
> +	for_each_vma(vmi, vma) {
> +		struct file *file = vma->vm_file;
> +		int flags = vma->vm_flags;
> +		unsigned long long pgoff = ((loff_t)vma->vm_pgoff) << PAGE_SHIFT;
> +		struct anon_vma_name *anon_name = NULL;
> +		struct mm_struct *mm = vma->vm_mm;
> +		char buf[256] = {0};
> +		const char *name = NULL;
> +
> +		if (mm)
> +			anon_name = anon_vma_name(vma);
> +
> +		if (file) {
> +			if (anon_name) {
> +				snprintf(buf, sizeof(buf), "[anon_shmem:%s]", anon_name->name);
> +				name = buf;
> +			} else {
> +				char *f_path = d_path(file_user_path(file), buf, sizeof(buf));
> +
> +				name = IS_ERR(f_path) ? "?" : f_path;
> +			}
> +			goto print;
> +		}
> +
> +		if (vma->vm_ops && vma->vm_ops->name) {
> +			name = vma->vm_ops->name(vma);
> +			if (name)
> +				goto print;
> +		}
> +
> +		name = arch_vma_name(vma);
> +		if (!name) {
> +			if (mm) {
> +				if (vma_is_initial_heap(vma))
> +					name = "[heap]";
> +				else if (vma_is_initial_stack(vma))
> +					name = "[stack]";
> +			} else
> +				name = "[vdso]";
> +
> +			if (anon_name) {
> +				snprintf(buf, sizeof(buf), "[anon:%s]", anon_name->name);
> +				name = buf;
> +			}
> +		}
> +

Wouldn't it be better to export/reuse get_vma_name() rather than duplicate
its code ?

Oleg.