linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] Dump command line of faulting process to syslog
@ 2022-08-08 13:09 Helge Deller
  2022-08-08 13:09 ` [PATCH v3 1/4] proc: Add get_task_cmdline_kernel() function Helge Deller
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Helge Deller @ 2022-08-08 13:09 UTC (permalink / raw)
  To: linux-s390, Josh Triplett, x86, linux-snps-arc, linux-fsdevel,
	linux-parisc, linux-kernel, linuxppc-dev, linux-arm-kernel

This patch series dumps the command line (including the program parameters) of
a faulting process to the syslog.

The motivation for this patch is that it's sometimes quite hard to find out and
annoying to not know which program *exactly* faulted when looking at the syslog.

For example, a dump on parisc shows:
   do_page_fault() command='cc1' type=15 address=0x00000000 in libc-2.33.so[f6abb000+184000]

-> We see the "cc1" compiler crashed, but it would be useful to know which file was compiled.
With this patch you will see that cc1 crashed while compiling some haskell code:

   cc1[13472] cmdline: /usr/lib/gcc/hppa-linux-gnu/12/cc1 -quiet @/tmp/ccRkFSfY -imultilib . -imultiarch hppa-linux-gnu -D USE_MINIINTERPRETER -D NO_REGS -D _HPUX_SOURCE -D NOSMP -D THREADED_RTS -include /build/ghc/ghc-9.0.2/includes/dist-install/build/ghcversion.h -iquote compiler/GHC/Iface -quiet -dumpdir /tmp/ghc13413_0/ -dumpbase ghc_5.hc -dumpbase-ext .hc -O -Wimplicit -fno-PIC -fwrapv -fno-builtin -fno-strict-aliasing -o /tmp/ghc13413_0/ghc_5.s

Another example are the glibc testcases which always segfault in "ld.so.1" with no other info:

   do_page_fault() command='ld.so.1' type=15 address=0x565921d8 in libc.so[f7339000+1bb000]

-> With the patch you can see it was the "tst-safe-linking-malloc-hugetlb1" testcase:

   ld.so.1[1151] cmdline: /home/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/gnu/glibc/objdir:/home/gnu/glibc/objdir/math:/home/gnu/
        /home/gnu/glibc/objdir/malloc/tst-safe-linking-malloc-hugetlb1

An example of a typical x86 fault shows up as:
   crash[2326]: segfault at 0 ip 0000561a7969c12e sp 00007ffe97a05630 error 6 in crash[561a7969c000+1000]
   Code: 68 ff ff ff c6 05 19 2f 00 00 01 5d c3 0f 1f 80 00 00 00 00 c3 0f 1f 80 00 00 ...

-> with this patch you now see the whole command line:
   crash[2326] cmdline: ./crash test_write_to_page_0

The patches are relatively small, and reuse functions which are used
to create the output for the /proc/<pid>/cmdline files.

The relevant changes are in patches #1 and #2.
Patch #3 adds the cmdline dump on x86.
Patch #4 drops code from arc which now becomes unnecessary as this is done by generic code.

Helge

---

Changes in v3:
- require task to be locked by caller, noticed by kernel test robot
- add parameter names in header files, noticed by kernel test robot

Changes in v2:
- Don't dump all or parts of the commandline depending on the
  kptr_restrict sysctl value (suggested by Josh Triplett).
- Patch sent to more arch mailing lists

Helge Deller (4):
  proc: Add get_task_cmdline_kernel() function
  lib/dump_stack: Add dump_stack_print_cmdline() and wire up in
    dump_stack_print_info()
  x86/fault: Dump command line of faulting process to syslog
  arc: Use generic dump_stack_print_cmdline() implementation

 arch/arc/kernel/troubleshoot.c | 24 -----------
 arch/x86/mm/fault.c            |  2 +
 fs/proc/base.c                 | 74 +++++++++++++++++++++++-----------
 include/linux/printk.h         |  5 +++
 include/linux/proc_fs.h        |  5 +++
 lib/dump_stack.c               | 34 ++++++++++++++++
 6 files changed, 97 insertions(+), 47 deletions(-)

--
2.37.1


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

* [PATCH v3 1/4] proc: Add get_task_cmdline_kernel() function
  2022-08-08 13:09 [PATCH v3 0/4] Dump command line of faulting process to syslog Helge Deller
@ 2022-08-08 13:09 ` Helge Deller
  2022-08-08 13:09 ` [PATCH v3 2/4] lib/dump_stack: Add dump_stack_print_cmdline() and wire up in dump_stack_print_info() Helge Deller
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Helge Deller @ 2022-08-08 13:09 UTC (permalink / raw)
  To: linux-s390, Josh Triplett, x86, linux-snps-arc, linux-fsdevel,
	linux-parisc, linux-kernel, linuxppc-dev, linux-arm-kernel

Add a new function get_task_cmdline_kernel() which reads the command
line of a process into a kernel buffer. This command line can then be
dumped by arch code to give additional debug info via the parameters
with which a faulting process was started.

The new function re-uses the existing code which provides the cmdline
for the procfs. For that the existing functions were modified so that
the buffer page is allocated outside of get_mm_proctitle() and
get_mm_cmdline() and instead provided as parameter.

Signed-off-by: Helge Deller <deller@gmx.de>

--
Changes in v3:
- add parameter names in header files, noticed by: kernel test robot
- require task to be locked by caller
---
 fs/proc/base.c          | 74 ++++++++++++++++++++++++++++-------------
 include/linux/proc_fs.h |  5 +++
 2 files changed, 56 insertions(+), 23 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 8dfa36a99c74..e2d4152aed34 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -217,20 +217,17 @@ static int proc_root_link(struct dentry *dentry, struct path *path)
  */
 static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
 				size_t count, unsigned long pos,
-				unsigned long arg_start)
+				unsigned long arg_start, char *page)
 {
-	char *page;
 	int ret, got;
+	size_t size;

-	if (pos >= PAGE_SIZE)
+	size = min_t(size_t, PAGE_SIZE, count);
+	if (pos >= size)
 		return 0;

-	page = (char *)__get_free_page(GFP_KERNEL);
-	if (!page)
-		return -ENOMEM;
-
 	ret = 0;
-	got = access_remote_vm(mm, arg_start, page, PAGE_SIZE, FOLL_ANON);
+	got = access_remote_vm(mm, arg_start, page, size, FOLL_ANON);
 	if (got > 0) {
 		int len = strnlen(page, got);

@@ -238,7 +235,9 @@ static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
 		if (len < got)
 			len++;

-		if (len > pos) {
+		if (!buf)
+			ret = len;
+		else if (len > pos) {
 			len -= pos;
 			if (len > count)
 				len = count;
@@ -248,16 +247,15 @@ static ssize_t get_mm_proctitle(struct mm_struct *mm, char __user *buf,
 			ret = len;
 		}
 	}
-	free_page((unsigned long)page);
 	return ret;
 }

 static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
-			      size_t count, loff_t *ppos)
+			      size_t count, loff_t *ppos, char *page)
 {
 	unsigned long arg_start, arg_end, env_start, env_end;
 	unsigned long pos, len;
-	char *page, c;
+	char c;

 	/* Check if process spawned far enough to have cmdline. */
 	if (!mm->env_end)
@@ -283,7 +281,7 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 	len = env_end - arg_start;

 	/* We're not going to care if "*ppos" has high bits set */
-	pos = *ppos;
+	pos = ppos ? *ppos : 0;
 	if (pos >= len)
 		return 0;
 	if (count > len - pos)
@@ -299,7 +297,7 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 	 * pos is 0, and set a flag in the 'struct file'.
 	 */
 	if (access_remote_vm(mm, arg_end-1, &c, 1, FOLL_ANON) == 1 && c)
-		return get_mm_proctitle(mm, buf, count, pos, arg_start);
+		return get_mm_proctitle(mm, buf, count, pos, arg_start, page);

 	/*
 	 * For the non-setproctitle() case we limit things strictly
@@ -311,10 +309,6 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 	if (count > arg_end - pos)
 		count = arg_end - pos;

-	page = (char *)__get_free_page(GFP_KERNEL);
-	if (!page)
-		return -ENOMEM;
-
 	len = 0;
 	while (count) {
 		int got;
@@ -323,7 +317,8 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 		got = access_remote_vm(mm, pos, page, size, FOLL_ANON);
 		if (got <= 0)
 			break;
-		got -= copy_to_user(buf, page, got);
+		if (buf)
+			got -= copy_to_user(buf, page, got);
 		if (unlikely(!got)) {
 			if (!len)
 				len = -EFAULT;
@@ -335,12 +330,11 @@ static ssize_t get_mm_cmdline(struct mm_struct *mm, char __user *buf,
 		count -= got;
 	}

-	free_page((unsigned long)page);
 	return len;
 }

 static ssize_t get_task_cmdline(struct task_struct *tsk, char __user *buf,
-				size_t count, loff_t *pos)
+				size_t count, loff_t *pos, char *page)
 {
 	struct mm_struct *mm;
 	ssize_t ret;
@@ -349,23 +343,57 @@ static ssize_t get_task_cmdline(struct task_struct *tsk, char __user *buf,
 	if (!mm)
 		return 0;

-	ret = get_mm_cmdline(mm, buf, count, pos);
+	ret = get_mm_cmdline(mm, buf, count, pos, page);
 	mmput(mm);
 	return ret;
 }

+/*
+ * Place up to maxcount chars of the command line of the process into the
+ * cmdline buffer.
+ * NOTE: Requires that the task was locked with task_lock(task) by the caller.
+ */
+void get_task_cmdline_kernel(struct task_struct *task,
+			char *cmdline, size_t maxcount)
+{
+	struct mm_struct *mm;
+	int i;
+
+	mm = task->mm;
+	if (!mm || (task->flags & PF_KTHREAD))
+		return;
+
+	memset(cmdline, 0, maxcount);
+	get_mm_cmdline(mm, NULL, maxcount - 1, NULL, cmdline);
+
+	/* remove NULs between parameters */
+	for (i = 0; i < maxcount - 2; i++) {
+		if (cmdline[i])
+			continue;
+		if (cmdline[i+1] == 0)
+			break;
+		cmdline[i] = ' ';
+	}
+}
+
 static ssize_t proc_pid_cmdline_read(struct file *file, char __user *buf,
 				     size_t count, loff_t *pos)
 {
 	struct task_struct *tsk;
 	ssize_t ret;
+	char *page;

 	BUG_ON(*pos < 0);

 	tsk = get_proc_task(file_inode(file));
 	if (!tsk)
 		return -ESRCH;
-	ret = get_task_cmdline(tsk, buf, count, pos);
+	page = (char *)__get_free_page(GFP_KERNEL);
+	if (page) {
+		ret = get_task_cmdline(tsk, buf, count, pos, page);
+		free_page((unsigned long)page);
+	} else
+		ret = -ENOMEM;
 	put_task_struct(tsk);
 	if (ret > 0)
 		*pos += ret;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 81d6e4ec2294..c802bc668656 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -158,6 +158,9 @@ int proc_pid_arch_status(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task);
 #endif /* CONFIG_PROC_PID_ARCH_STATUS */

+void get_task_cmdline_kernel(struct task_struct *task,
+			char *cmdline, size_t maxcount);
+
 #else /* CONFIG_PROC_FS */

 static inline void proc_root_init(void)
@@ -216,6 +219,8 @@ static inline struct pid *tgid_pidfd_to_pid(const struct file *file)
 	return ERR_PTR(-EBADF);
 }

+static inline void get_task_cmdline_kernel(struct task_struct *task, char *cmdl, size_t m) { }
+
 #endif /* CONFIG_PROC_FS */

 struct net;
--
2.37.1


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

* [PATCH v3 2/4] lib/dump_stack: Add dump_stack_print_cmdline() and wire up in dump_stack_print_info()
  2022-08-08 13:09 [PATCH v3 0/4] Dump command line of faulting process to syslog Helge Deller
  2022-08-08 13:09 ` [PATCH v3 1/4] proc: Add get_task_cmdline_kernel() function Helge Deller
@ 2022-08-08 13:09 ` Helge Deller
  2022-08-08 13:09 ` [PATCH v3 3/4] x86/fault: Dump command line of faulting process to syslog Helge Deller
  2022-08-08 13:09 ` [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation Helge Deller
  3 siblings, 0 replies; 7+ messages in thread
From: Helge Deller @ 2022-08-08 13:09 UTC (permalink / raw)
  To: linux-s390, Josh Triplett, x86, linux-snps-arc, linux-fsdevel,
	linux-parisc, linux-kernel, linuxppc-dev, linux-arm-kernel

Add the function dump_stack_print_cmdline() which can be used by arch
code to print the command line of the current processs.  This function
is useful in arch code when dumping information for a faulting process.

Wire this function up in the dump_stack_print_info() function to include
the dumping of the command line for architectures which use
dump_stack_print_info().

As an example, with this patch a failing glibc testcase (which uses
ld.so.1 as starting program) up to now reported just "ld.so.1" failing:

 do_page_fault() command='ld.so.1' type=15 address=0x565921d8 in libc.so[f7339000+1bb000]
 trap #15: Data TLB miss fault, vm_start = 0x0001a000, vm_end = 0x0001b000

and now it reports in addition:

 ld.so.1[1151] cmdline: /home/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/gnu/glibc/objdir:/home/gnu/glibc/objdir/math:/home/gnu/
    /home/gnu/glibc/objdir/malloc/tst-safe-linking-malloc-hugetlb1

Josh Triplett noted that dumping such command line parameters into
syslog may theoretically lead to information disclosure.
That's why this patch checks the value of the kptr_restrict sysctl
variable and will not print any information if kptr_restrict==2, and
will not show the program parameters if kptr_restrict==1.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 include/linux/printk.h |  5 +++++
 lib/dump_stack.c       | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index cf7d666ab1f8..5290a32a197d 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -191,6 +191,7 @@ u32 log_buf_len_get(void);
 void log_buf_vmcoreinfo_setup(void);
 void __init setup_log_buf(int early);
 __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...);
+void dump_stack_print_cmdline(const char *log_lvl);
 void dump_stack_print_info(const char *log_lvl);
 void show_regs_print_info(const char *log_lvl);
 extern asmlinkage void dump_stack_lvl(const char *log_lvl) __cold;
@@ -262,6 +263,10 @@ static inline __printf(1, 2) void dump_stack_set_arch_desc(const char *fmt, ...)
 {
 }

+static inline void dump_stack_print_cmdline(const char *log_lvl)
+{
+}
+
 static inline void dump_stack_print_info(const char *log_lvl)
 {
 }
diff --git a/lib/dump_stack.c b/lib/dump_stack.c
index 83471e81501a..38ef1067c7eb 100644
--- a/lib/dump_stack.c
+++ b/lib/dump_stack.c
@@ -14,6 +14,7 @@
 #include <linux/kexec.h>
 #include <linux/utsname.h>
 #include <linux/stop_machine.h>
+#include <linux/proc_fs.h>

 static char dump_stack_arch_desc_str[128];

@@ -45,6 +46,37 @@ void __init dump_stack_set_arch_desc(const char *fmt, ...)
 #define BUILD_ID_VAL ""
 #endif

+/**
+ * dump_stack_print_cmdline - print the command line of current process
+ * @log_lvl: log level
+ */
+void dump_stack_print_cmdline(const char *log_lvl)
+{
+	char cmdline[256];
+
+	if (kptr_restrict >= 2)
+		return; /* never show command line */
+
+	/* get command line */
+	get_task_cmdline_kernel(current, cmdline, sizeof(cmdline));
+
+	if (kptr_restrict == 1) {
+		char *p;
+
+		/* if restricted show program path only */
+		p = strchr(cmdline, ' ');
+		if (p) {
+			*p = 0;
+			strlcat(cmdline,
+				" ... [parameters hidden due to kptr_restrict]",
+				sizeof(cmdline));
+		}
+	}
+
+	printk("%s%s[%d] cmdline: %s\n", log_lvl, current->comm,
+		current->pid, cmdline);
+}
+
 /**
  * dump_stack_print_info - print generic debug info for dump_stack()
  * @log_lvl: log level
@@ -62,6 +94,8 @@ void dump_stack_print_info(const char *log_lvl)
 	       (int)strcspn(init_utsname()->version, " "),
 	       init_utsname()->version, BUILD_ID_VAL);

+	dump_stack_print_cmdline(log_lvl);
+
 	if (dump_stack_arch_desc_str[0] != '\0')
 		printk("%sHardware name: %s\n",
 		       log_lvl, dump_stack_arch_desc_str);
--
2.37.1


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

* [PATCH v3 3/4] x86/fault: Dump command line of faulting process to syslog
  2022-08-08 13:09 [PATCH v3 0/4] Dump command line of faulting process to syslog Helge Deller
  2022-08-08 13:09 ` [PATCH v3 1/4] proc: Add get_task_cmdline_kernel() function Helge Deller
  2022-08-08 13:09 ` [PATCH v3 2/4] lib/dump_stack: Add dump_stack_print_cmdline() and wire up in dump_stack_print_info() Helge Deller
@ 2022-08-08 13:09 ` Helge Deller
  2022-08-08 13:09 ` [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation Helge Deller
  3 siblings, 0 replies; 7+ messages in thread
From: Helge Deller @ 2022-08-08 13:09 UTC (permalink / raw)
  To: linux-s390, Josh Triplett, x86, linux-snps-arc, linux-fsdevel,
	linux-parisc, linux-kernel, linuxppc-dev, linux-arm-kernel

If a process segfaults, include the command line of the faulting process
in the syslog.

In the example below, the "crash" program (which simply writes zero to address 0)
was called with the parameters "this is a test":

 crash[2326]: segfault at 0 ip 0000561a7969c12e sp 00007ffe97a05630 error 6 in crash[561a7969c000+1000]
 crash[2326] cmdline: ./crash this is a test
 Code: 68 ff ff ff c6 05 19 2f 00 00 01 5d c3 0f 1f 80 00 00 00 00 c3 0f 1f ...

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/x86/mm/fault.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index fad8faa29d04..d4e21c402e29 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -784,6 +784,8 @@ show_signal_msg(struct pt_regs *regs, unsigned long error_code,

 	printk(KERN_CONT "\n");

+	dump_stack_print_cmdline(loglvl);
+
 	show_opcodes(regs, loglvl);
 }

--
2.37.1


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

* [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation
  2022-08-08 13:09 [PATCH v3 0/4] Dump command line of faulting process to syslog Helge Deller
                   ` (2 preceding siblings ...)
  2022-08-08 13:09 ` [PATCH v3 3/4] x86/fault: Dump command line of faulting process to syslog Helge Deller
@ 2022-08-08 13:09 ` Helge Deller
  2022-10-10  5:18   ` Vineet Gupta
  3 siblings, 1 reply; 7+ messages in thread
From: Helge Deller @ 2022-08-08 13:09 UTC (permalink / raw)
  To: linux-s390, Josh Triplett, x86, linux-snps-arc, linux-fsdevel,
	linux-parisc, linux-kernel, linuxppc-dev, linux-arm-kernel

The process program name and command line is now shown in generic code
in dump_stack_print_info(), so drop the arc-specific implementation.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/arc/kernel/troubleshoot.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
index 7654c2e42dc0..9807e590ee55 100644
--- a/arch/arc/kernel/troubleshoot.c
+++ b/arch/arc/kernel/troubleshoot.c
@@ -51,29 +51,6 @@ static void print_regs_callee(struct callee_regs *regs)
 		regs->r24, regs->r25);
 }

-static void print_task_path_n_nm(struct task_struct *tsk)
-{
-	char *path_nm = NULL;
-	struct mm_struct *mm;
-	struct file *exe_file;
-	char buf[ARC_PATH_MAX];
-
-	mm = get_task_mm(tsk);
-	if (!mm)
-		goto done;
-
-	exe_file = get_mm_exe_file(mm);
-	mmput(mm);
-
-	if (exe_file) {
-		path_nm = file_path(exe_file, buf, ARC_PATH_MAX-1);
-		fput(exe_file);
-	}
-
-done:
-	pr_info("Path: %s\n", !IS_ERR(path_nm) ? path_nm : "?");
-}
-
 static void show_faulting_vma(unsigned long address)
 {
 	struct vm_area_struct *vma;
@@ -176,7 +153,6 @@ void show_regs(struct pt_regs *regs)
 	 */
 	preempt_enable();

-	print_task_path_n_nm(tsk);
 	show_regs_print_info(KERN_INFO);

 	show_ecr_verbose(regs);
--
2.37.1


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

* Re: [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation
  2022-08-08 13:09 ` [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation Helge Deller
@ 2022-10-10  5:18   ` Vineet Gupta
  2022-10-10  6:16     ` Helge Deller
  0 siblings, 1 reply; 7+ messages in thread
From: Vineet Gupta @ 2022-10-10  5:18 UTC (permalink / raw)
  To: Helge Deller, linux-s390, Josh Triplett, x86, linux-snps-arc,
	linux-fsdevel, linux-parisc, linux-kernel, linuxppc-dev,
	linux-arm-kernel
  Cc: Alexey Brodkin, Shahab Vahedi

On 8/8/22 06:09, Helge Deller wrote:
> The process program name and command line is now shown in generic code
> in dump_stack_print_info(), so drop the arc-specific implementation.
>
> Signed-off-by: Helge Deller <deller@gmx.de>

But that info printing was added back in 2018 by e36df28f532f882.
I don't think arc is using show_regs_print_info -> dump_stack_print_info 
yet.
Or is there a different code path now which calls here ?

> ---
>   arch/arc/kernel/troubleshoot.c | 24 ------------------------
>   1 file changed, 24 deletions(-)
>
> diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
> index 7654c2e42dc0..9807e590ee55 100644
> --- a/arch/arc/kernel/troubleshoot.c
> +++ b/arch/arc/kernel/troubleshoot.c
> @@ -51,29 +51,6 @@ static void print_regs_callee(struct callee_regs *regs)
>   		regs->r24, regs->r25);
>   }
>
> -static void print_task_path_n_nm(struct task_struct *tsk)
> -{
> -	char *path_nm = NULL;
> -	struct mm_struct *mm;
> -	struct file *exe_file;
> -	char buf[ARC_PATH_MAX];
> -
> -	mm = get_task_mm(tsk);
> -	if (!mm)
> -		goto done;
> -
> -	exe_file = get_mm_exe_file(mm);
> -	mmput(mm);
> -
> -	if (exe_file) {
> -		path_nm = file_path(exe_file, buf, ARC_PATH_MAX-1);
> -		fput(exe_file);
> -	}
> -
> -done:
> -	pr_info("Path: %s\n", !IS_ERR(path_nm) ? path_nm : "?");
> -}
> -
>   static void show_faulting_vma(unsigned long address)
>   {
>   	struct vm_area_struct *vma;
> @@ -176,7 +153,6 @@ void show_regs(struct pt_regs *regs)
>   	 */
>   	preempt_enable();

Maybe we remove preempt* as well now (perhaps as a follow up patch) 
since that was added by f731a8e89f8c78 "ARC: show_regs: lockdep: 
re-enable preemption" where show_regs -> print_task_path_n_nm -> mmput 
was triggering lockdep splat which is supposedly removed.

>
> -	print_task_path_n_nm(tsk);
>   	show_regs_print_info(KERN_INFO);
>
>   	show_ecr_verbose(regs);
> --
> 2.37.1
>
>
> _______________________________________________
> linux-snps-arc mailing list
> linux-snps-arc@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc


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

* Re: [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation
  2022-10-10  5:18   ` Vineet Gupta
@ 2022-10-10  6:16     ` Helge Deller
  0 siblings, 0 replies; 7+ messages in thread
From: Helge Deller @ 2022-10-10  6:16 UTC (permalink / raw)
  To: Vineet Gupta, linux-s390, Josh Triplett, x86, linux-snps-arc,
	linux-fsdevel, linux-parisc, linux-kernel, linuxppc-dev,
	linux-arm-kernel
  Cc: Alexey Brodkin, Shahab Vahedi

On 10/10/22 07:18, Vineet Gupta wrote:
> On 8/8/22 06:09, Helge Deller wrote:
>> The process program name and command line is now shown in generic code
>> in dump_stack_print_info(), so drop the arc-specific implementation.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>
> But that info printing was added back in 2018 by e36df28f532f882.
> I don't think arc is using show_regs_print_info -> dump_stack_print_info yet.
> Or is there a different code path now which calls here ?

Right.
See patches #1 and #2 of this series which added this
info to dump_stack_print_info().


>> ---
>>   arch/arc/kernel/troubleshoot.c | 24 ------------------------
>>   1 file changed, 24 deletions(-)
>>
>> diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
>> index 7654c2e42dc0..9807e590ee55 100644
>> --- a/arch/arc/kernel/troubleshoot.c
>> +++ b/arch/arc/kernel/troubleshoot.c
>> @@ -51,29 +51,6 @@ static void print_regs_callee(struct callee_regs *regs)
>>           regs->r24, regs->r25);
>>   }
>>
>> -static void print_task_path_n_nm(struct task_struct *tsk)
>> -{
>> -    char *path_nm = NULL;
>> -    struct mm_struct *mm;
>> -    struct file *exe_file;
>> -    char buf[ARC_PATH_MAX];
>> -
>> -    mm = get_task_mm(tsk);
>> -    if (!mm)
>> -        goto done;
>> -
>> -    exe_file = get_mm_exe_file(mm);
>> -    mmput(mm);
>> -
>> -    if (exe_file) {
>> -        path_nm = file_path(exe_file, buf, ARC_PATH_MAX-1);
>> -        fput(exe_file);
>> -    }
>> -
>> -done:
>> -    pr_info("Path: %s\n", !IS_ERR(path_nm) ? path_nm : "?");
>> -}
>> -
>>   static void show_faulting_vma(unsigned long address)
>>   {
>>       struct vm_area_struct *vma;
>> @@ -176,7 +153,6 @@ void show_regs(struct pt_regs *regs)
>>        */
>>       preempt_enable();
>
> Maybe we remove preempt* as well now (perhaps as a follow up patch)
> since that was added by f731a8e89f8c78 "ARC: show_regs: lockdep:
> re-enable preemption" where show_regs -> print_task_path_n_nm ->
> mmput was triggering lockdep splat which is supposedly removed.

The patch series was dropped from Andrew's queue, because the kernel
test robot showed some issues:
https://lore.kernel.org/lkml/Yu59QdVpPgnXUnQC@xsang-OptiPlex-9020/
Maybe adding preempt_enable() in my patches would fix that -
sadly I haven't had time to follow up on this yet ...

Helge

>
>>
>> -    print_task_path_n_nm(tsk);
>>       show_regs_print_info(KERN_INFO);
>>
>>       show_ecr_verbose(regs);


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

end of thread, other threads:[~2022-10-10  6:16 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-08 13:09 [PATCH v3 0/4] Dump command line of faulting process to syslog Helge Deller
2022-08-08 13:09 ` [PATCH v3 1/4] proc: Add get_task_cmdline_kernel() function Helge Deller
2022-08-08 13:09 ` [PATCH v3 2/4] lib/dump_stack: Add dump_stack_print_cmdline() and wire up in dump_stack_print_info() Helge Deller
2022-08-08 13:09 ` [PATCH v3 3/4] x86/fault: Dump command line of faulting process to syslog Helge Deller
2022-08-08 13:09 ` [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation Helge Deller
2022-10-10  5:18   ` Vineet Gupta
2022-10-10  6:16     ` Helge Deller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).