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

[Oleg Nesterov <oleg@redhat.com>]

On 09/24, qiwu.chen wrote:
>
> +EXPORT_SYMBOL(get_vma_name);

Why?

> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -3566,6 +3566,10 @@ static inline bool range_in_vma(struct vm_area_struct *vma,
>  #ifdef CONFIG_MMU
>  pgprot_t vm_get_page_prot(unsigned long vm_flags);
>  void vma_set_page_prot(struct vm_area_struct *vma);
> +void get_vma_name(struct vm_area_struct *vma,
> +			 const struct path **path,
> +			 const char **name,
> +			 const char **name_fmt);
>  #else

You didn't move get_vma_name() from fs/proc/task_mmu.c, so it also depends
on CONFIG_PROC_FS.

> +/*
> + * 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) {
> +		int flags = vma->vm_flags;
> +		unsigned long long pgoff = ((loff_t)vma->vm_pgoff) << PAGE_SHIFT;
> +		const struct path *path;
> +		const char *name_fmt, *name;
> +		char name_buf[SZ_256];
> +
> +		get_vma_name(vma, &path, &name, &name_fmt);

So this code won't compile if CONFIG_MMU=n ?

> +		if (path) {
> +			name = d_path(path, name_buf, sizeof(name_buf));
> +			name = IS_ERR(name) ? "?" : name;

perhaps this needs mangle_path() ...

> +		} else if (name || name_fmt) {
> +			snprintf(name_buf, sizeof(name_buf), name_fmt ?: "%s", name);
> +			name = name_buf;
> +		}

Why not

		} else if (name_fmt) {
			snprintf(name_buf, sizeof(name_buf), name_fmt, name);
			name = name_buf;
		}
?

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

Oleg.