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

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 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 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