On Tue, Jul 07, 2015 at 10:03:15AM +0530, Maninder Singh wrote: > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index 1ef2940..53d57db 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -51,8 +51,9 @@ int show_unhandled_signals = 1; > /* > * Dump out the contents of some memory nicely... > */ > +
Unnecessary empty line added here. > static void dump_mem(const char *lvl, const char *str, unsigned long bottom, > - unsigned long top) > + unsigned long top, unsigned int width) I would add a WARN_ON(width != 4 || width != 8). Alternatively, just change this to a "bool wide" argument and a local width variable (it helps with the is_compat_thread below). > { > unsigned long first; > mm_segment_t fs; > @@ -70,18 +71,27 @@ static void dump_mem(const char *lvl, const char *str, > unsigned long bottom, > > for (first = bottom & ~31; first < top; first += 32) { > unsigned long p; > - char str[sizeof(" 12345678") * 8 + 1]; > + char str[sizeof(" 1234567812345678") * 8 + 1]; Does this need to be just "* 4" for 8 byte width? In which case, the original size covers it since you just have a few spaces less. > memset(str, ' ', sizeof(str)); > str[sizeof(str) - 1] = '\0'; > > - for (p = first, i = 0; i < 8 && p < top; i++, p += 4) { > + for (p = first, i = 0; i < (width > 4 ? width-4 : width+4) > + && p < top; i++, p += width) { It's probably clearer as: for (p = first, i = 0; i < 32 / width && p < top; i++, p += width) (the upper limit for i is 32-bytes per line divided by how many bytes we read at once: 4 or 8) > if (p >= bottom && p < top) { > - unsigned int val; > - if (__get_user(val, (unsigned int *)p) == 0) > - sprintf(str + i * 9, " %08x", val); > - else > - sprintf(str + i * 9, " ????????"); > + unsigned long val; > + > + if (width == 8) { > + if (__get_user(val, (unsigned long *)p) > == 0) > + sprintf(str + i * 17, " > %016lx", val); > + else > + sprintf(str + i * 17, " > ????????????????"); > + } else { > + if (__get_user(val, (unsigned int *)p) > == 0) > + sprintf(str + i * 9, " %08x", > (unsigned int)val); I don't think you need the (unsigned int) cast here, just " %08lx". > @@ -95,7 +105,7 @@ static void dump_backtrace_entry(unsigned long where, > unsigned long stack) > print_ip_sym(where); > if (in_exception_text(where)) > dump_mem("", "Exception stack", stack, > - stack + sizeof(struct pt_regs)); > + stack + sizeof(struct pt_regs), 8); > } > > static void dump_instr(const char *lvl, struct pt_regs *regs) > @@ -191,6 +201,7 @@ static int __die(const char *str, int err, struct > thread_info *thread, > struct task_struct *tsk = thread->task; > static int die_counter; > int ret; > + unsigned int width = 8; You could just set width here: unsigned int width = is_compat_thread(thread) ? 4 : 8; Or, if you pass the argument as bool to dump_mem, just use is_compat_thread() directly. > > pr_emerg("Internal error: %s: %x [#%d]" S_PREEMPT S_SMP "\n", > str, err, ++die_counter); > @@ -206,8 +217,19 @@ static int __die(const char *str, int err, struct > thread_info *thread, > TASK_COMM_LEN, tsk->comm, task_pid_nr(tsk), thread + 1); > > if (!user_mode(regs) || in_interrupt()) { > - dump_mem(KERN_EMERG, "Stack: ", regs->sp, > - THREAD_SIZE + (unsigned long)task_stack_page(tsk)); > + > + if (regs->sp > (unsigned long)task_stack_page(tsk)) { > + dump_mem(KERN_EMERG, "Stack: ", regs->sp, > + THREAD_SIZE + > + (unsigned long)task_stack_page(tsk), width); > + } else { > + if (compat_user_mode(regs)) > + width = 4; > + dump_mem(KERN_EMERG, "Stack: ", > + (unsigned long)task_stack_page(tsk), > + THREAD_SIZE + > + (unsigned long)task_stack_page(tsk), width); > + } I don't understand this change. If you have a good reason for it, please add it as a separate patch as it doesn't look to me like it has anything to do with the width as described in this patch. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/