2022-08-08 13:11:34

by Helge Deller

[permalink] [raw]
Subject: [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation

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 <[email protected]>
---
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


2022-10-10 05:34:56

by Vineet Gupta

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation

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 <[email protected]>

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
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-snps-arc

2022-10-10 06:25:06

by Helge Deller

[permalink] [raw]
Subject: Re: [PATCH v3 4/4] arc: Use generic dump_stack_print_cmdline() implementation

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 <[email protected]>
>
> 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);