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

[chenqiwu <qiwuchen55@gmail.com>]

On Tue, Sep 24, 2024 at 01:33:54PM +0200, Oleg Nesterov wrote:
> 
> You didn't move get_vma_name() from fs/proc/task_mmu.c, so it also depends
> on CONFIG_PROC_FS.
>
Sure, thanks for your advice. it should be moved to mm tree without CONFIG_PROC_FS
dependence.

> > +		if (path) {
> > +			name = d_path(path, name_buf, sizeof(name_buf));
> > +			name = IS_ERR(name) ? "?" : name;
> 
I think this is an easier way to get file path name which deals with IS_ERR(name) case.
> perhaps this needs mangle_path() ...
>

> > +		} else if (name || name_fmt) {
> > +			snprintf(name_buf, sizeof(name_buf), name_fmt ?: "%s", name);
> > +			name = name_buf;
> > +		}
> 
This refers to the code section of do_procmap_query().

> Why not
> 
> 		} else if (name_fmt) {
> 			snprintf(name_buf, sizeof(name_buf), name_fmt, name);
> 			name = name_buf;
> 		}
> ?
>
Sure, this is a better way to deal with name_fmt case.

> > +		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);
> 
> I don't really understand why you skip vma if !name...
> 
Well, the vma without name seems no sense for debugging, skipping it can reduce the
maps space. But the disadvantage is debuggers cannot get full maps of task,
perhaps we shouldn't skip it.

Thanks
Qiwu