> -----Original Message----- > From: Michael Ellerman <m...@ellerman.id.au> > Sent: Tuesday, 31 July 2018 7:32 PM > To: Murilo Opsfelder Araujo <muri...@linux.ibm.com>; LEROY Christophe > <christophe.le...@c-s.fr> > Cc: linux-ker...@vger.kernel.org; Alastair D'Silva <alast...@d-silva.org>; > Andrew Donnellan <andrew.donnel...@au1.ibm.com>; Balbir Singh > <bsinghar...@gmail.com>; Benjamin Herrenschmidt > <b...@kernel.crashing.org>; Cyril Bur <cyril...@gmail.com>; Eric W . > Biederman <ebied...@xmission.com>; Joe Perches <j...@perches.com>; > Michael Neuling <mi...@neuling.org>; Nicholas Piggin > <npig...@gmail.com>; Paul Mackerras <pau...@samba.org>; Simon Guo > <wei.guo.si...@gmail.com>; Sukadev Bhattiprolu > <suka...@linux.vnet.ibm.com>; Tobin C . Harding <m...@tobin.cc>; linuxppc- > d...@lists.ozlabs.org > Subject: Re: [PATCH v2 04/10] powerpc/traps: Use REG_FMT in > show_signal_msg() > > Murilo Opsfelder Araujo <muri...@linux.ibm.com> writes: > > On Mon, Jul 30, 2018 at 06:30:47PM +0200, LEROY Christophe wrote: > >> Murilo Opsfelder Araujo <muri...@linux.ibm.com> a écrit : > >> > On Fri, Jul 27, 2018 at 06:40:23PM +0200, LEROY Christophe wrote: > >> > > Murilo Opsfelder Araujo <muri...@linux.ibm.com> a écrit : > >> > > > >> > > > Simplify the message format by using REG_FMT as the register > >> > > > format. This avoids having two different formats and avoids > checking for MSR_64BIT. > >> > > > >> > > Are you sure it is what we want ? > >> > > >> > Yes. > >> > > >> > > Won't it change the behaviour for a 32 bits app running on a 64bits > kernel ? > >> > > >> > In fact, this changes how many zeroes are prefixed when displaying > >> > the registers (%016lx vs. %08lx format). For example, 32-bits > >> > userspace, 64-bits kernel: > >> > >> Indeed that's what I suspected. What is the real benefit of this change ? > >> Why not keep the current format for 32bits userspace ? All those > >> leading zeroes are pointless to me. > > > > One of the benefits is simplifying the code by removing some checks. > > Another is deduplicating almost identical format strings in favor of a > > unified > one. > > > > After reading Joe's comment [1], %px seems to be the format we're > looking for. > > An extract from Documentation/core-api/printk-formats.rst: > > > > "%px is functionally equivalent to %lx (or %lu). %px is preferred because > > it > > is more uniquely grep'able." > > > > So I guess we don't need to worry about the format (%016lx vs. %08lx), > > let's just use %px, as per the guideline. > > I don't think I like %px.
Me neither, semantically, it's for pointers, and the data being displayed is not a pointer. > It makes the format string cleaner, but it means we have to cast everything > to void * which is ugly as heck. > > I actually don't think the leading zeroes are helpful at all in the signal > message, ie. we should just use %lx there. > > They are useful in show_regs() because we want everything to line up. > > So I think I'll drop patch 3 and use 0x%lx in show_signal_msg(), meaning we > end up with, eg: > > [ 73.414535] segv[3759]: segfault (11) at 0x0 nip 0x10000420 lr 0xfe61854 > code 0x1 in segv[10000000+10000] > [ 73.414641] segv[3759]: code: 4e800421 80010014 38210010 7c0803a6 > 4bffff30 9421ffd0 93e1002c 7c3f0b78 > [ 73.414665] segv[3759]: code: 39200000 913f001c 813f001c 39400001 > <91490000> 39200000 7d234b78 397f0030 Or better yet, "%#lx" - the hash adds the appropriate prefix in the right case for the format. -- Alastair D'Silva mob: 0423 762 819 skype: alastair_dsilva msn: alast...@d-silva.org blog: http://alastair.d-silva.org Twitter: @EvilDeece
