From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752832AbdHOT6s (ORCPT ); Tue, 15 Aug 2017 15:58:48 -0400 Received: from mout.gmx.net ([212.227.17.22]:64378 "EHLO mout.gmx.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751628AbdHOT6q (ORCPT ); Tue, 15 Aug 2017 15:58:46 -0400 Subject: Re: [PATCH] printk-formats.txt: Add examples for %pS and %pF To: Petr Mladek Cc: Sergey Senozhatsky , Sergey Senozhatsky , Steven Rostedt , linux-kernel@vger.kernel.org, linux-parisc@vger.kernel.org References: <20170810173533.GA11600@ls3530.fritz.box> <20170811001521.GA520@jagdpanzerIV.localdomain> <47718712-9eb2-8ad7-965e-d0971d07e985@gmx.de> <20170815113625.GA2738@pathway.suse.cz> From: Helge Deller Message-ID: Date: Tue, 15 Aug 2017 21:58:33 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170815113625.GA2738@pathway.suse.cz> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:DPfCJiBZNkPEnMMKuoSQ8/ePfuCBNAqbG9eDYxPOwD6+Iui6EtG U4Bq3eB3TI3SgzAD76dYT8nfmI+NgoGalm9OoIxzdcTk/opxNYNWxmjPqxdyc9pBUS/0P73 YUP4V/CcdBjqztR4ZSz5+Apfe9x5islm1a2JJp+n2SB5qiWrgR4ibXTzTWsGE8o09xXeEZb Ywx/ACi7dy6O8NfAP1soA== X-UI-Out-Filterresults: notjunk:1;V01:K0:FXN42OpzXJE=:EtTldUXZ1CZwLP45xhkNlK bGTSgVDJpm60MlUDGq6Q3q+bDQy9gQPn8FlzoXmVzBBfhYj6QYuPugiJSc5Y17gpBX8NUC4xF LFSmd+g6D5N2P5IzDbViCQ3Do03RmSyH2T21IYC2DqD/QRYbovBE0/DIUY7VhzIT1TmC5gSFj xvhD0FusNmiY0PAmCoVfcnyzS2G3f3aMb05xtGxX7xN7v1+RZiGwwvyFWO/u8G89r+CnzLlwv nK67i/AeINA2Cn0CSLQYiwgUReYUnF+n7lfWEHQaFaQC+CgA6GZ2PTPzc+1ltCsFfPwM9UAAf 1CZFnniG/72diBbcGv7WfoOMtYYrbF9eQJ8qQpRdXgXTcSk+IyI7oxA0OvWYN6Bnh7IUGxfyz ID3PJKnLXg75HzbDRYRHWcZBpaRKb6mPfArsWXyaAIZREqWu9RE5UDGXqAmgxMrN215ddXOtM v4xnDcqI+4gGGvYLCfruRJL+V6j7qgcu0XcKoPB0uISxvqgSlTa4Ds1VO7ZiOrA1YlH2MaByv jrzP4jI4/vk4ggzUZQr/pDmbS9wg1KQnmTW2xFbVuXyzAJMwmez0ZCoQxGiG+aR21Bz//1zPY Aspef7zdw2CGQXRNQ2zliPslYPob/4hiV68OA13grzfFht3F00WS1yqFeKnf5H3WQbrK4411E r5h7ihwGMwS6BxiODsyUS9V48i24SEzucJKI8VOI89miBAQbtE9DpTV9xrJ2bNeXqQxr1ctFq 7IHTt+N2JV8Z01DUak3fkTbl1bwgykpA/y871TOlg3TCm8dTesy4hbP8HXVU++aKllyXQhAzB 1nW8BWITPKG7kteflzDek4L0lfhxZ5NgwRZnVFX4Pgajh4dSiA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 15.08.2017 13:36, Petr Mladek wrote: > On Fri 2017-08-11 09:31:28, Helge Deller wrote: >> On 11.08.2017 02:15, Sergey Senozhatsky wrote: >>> On (08/10/17 19:35), Helge Deller wrote: >>>> Sometimes people seems unclear when to use the %pS or %pF printk format. >>>> Adding some examples may help to avoid such mistakes. >>>> >>>> See for example commit 51d96dc2e2dc ("random: fix warning message on ia64 and >>>> parisc") which fixed such a wrong format string. >>>> >>>> Signed-off-by: Helge Deller >>>> >>>> diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt >>>> index 65ea591..be8c05b 100644 >>>> --- a/Documentation/printk-formats.txt >>>> +++ b/Documentation/printk-formats.txt >>>> @@ -73,6 +73,12 @@ actually function descriptors which must first be resolved. The ``F`` and >>>> ``f`` specifiers perform this resolution and then provide the same >>>> functionality as the ``S`` and ``s`` specifiers. >>>> >>>> +Examples:: >>>> + >>>> + printk("Called from %pS.\n", __builtin_return_address(0)); >>>> + printk("Called from %pS.\n", (void *)regs->ip); >>>> + printk("Called from %pF.\n", &gettimeofday); >>> >>> there is this paragraph >>> >>> : On ia64, ppc64 and parisc64 architectures function pointers are >>> : actually function descriptors which must first be resolved. The ``F`` and >>> : ``f`` specifiers perform this resolution and then provide the same >>> : functionality as the ``S`` and ``s`` specifiers. >>> >>> which supposed to explain everything in details. the examples >>> don't make it any `clearer', IMHO. >> >> Experts surely do know what function descriptors are. >> Nevertheless even those often get it wrong as can be seen in >> various commits. > > It seems that these specifiers are used the wrong way on many > locations. Yes. %pF usage in mm/memblock.c is just one example. > They might be worth fixing but I cannot test it easily. I can check and send patches at some point. > Hmm, using %pF might actually cause a crash when used > on direct function address. Probably won't happen on parisc, but basically you are right. >> The hope with this patch is to show widely-used examples >> and avoid additional commits afterwards to fix it up. > > IMHO, one problem is that the meaning of ''F'' and ''f'' > is hidden at the end of the section. Also the first line > > 'For printing symbols and function pointers. The ``S`` and ``s`` ' > > kind of invites to use ``S`` and ``s`` even for function pointers. > I suggest to switch the order, slightly retranslate, add the > examples, see below. > > >> This patch was meant to be RFC. >> If you decide not to take it, I'm fine as well. >> >>> *may be* on "ia64, ppc64 and parisc64" we can somehow check >>> that the pointer, which we pass as %pS, belongs to .text and >>> print some build-time warnings. well, if it's actually a >>> problem. dunno. > > I think that it would need to be a runtime check because many/most > printed addresses are not statically defined. > > >> I think it's not needed. Those bugs will be seen and fixed. > > I am not sure how many people are familiar with this problem. > I might help to avoid some headaches when debugging. > > If we add the warning, it should be ratelimited to reduce messing > of the original message. > > I do not have strong opinion about it. > > > Here is the updated patch with my proposed changes. > Feel free to update it: Much better! Thanks a lot. Maybe we should mention usage of __func__ with '%s' (see other thread). And _RET_IP_ is worth mentioning beside __builtin_return_address(0) too, because it's used quite often wrongly. Helge > From ef983c65095cada994c1fe531e2b98e936c943bf Mon Sep 17 00:00:00 2001 > From: Helge Deller > Date: Tue, 15 Aug 2017 11:34:19 +0200 > Subject: [PATCH] printk-formats.txt: Better describe the difference between > %pS and %pF > > Sometimes people seems unclear when to use the %pS or %pF printk format. > For example, see commit 51d96dc2e2dc ("random: fix warning message on ia64 > and parisc") which fixed such a wrong format string. > > The documentation should be more clear about the difference. > > Signed-off-by: Helge Deller > [pmladek@suse.com: Restructure the entire section] > Signed-off-by: Petr Mladek > --- > Documentation/printk-formats.txt | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/Documentation/printk-formats.txt b/Documentation/printk-formats.txt > index 65ea5915178b..074670b98bac 100644 > --- a/Documentation/printk-formats.txt > +++ b/Documentation/printk-formats.txt > @@ -58,20 +58,23 @@ Symbols/Function Pointers > %ps versatile_init > %pB prev_fn_of_versatile_init+0x88/0x88 > > -For printing symbols and function pointers. The ``S`` and ``s`` specifiers > -result in the symbol name with (``S``) or without (``s``) offsets. Where > -this is used on a kernel without KALLSYMS - the symbol address is > -printed instead. > +The ``F`` and ``f`` specifiers are for printing function pointers, > +for example, f->func, &gettimeofday. They have the same result as > +``S`` and ``s`` specifiers. But they do an extra conversion on > +ia64, ppc64 and parisc64 architectures where the function pointers > +are actually function descriptors. > + > +The ``S`` and ``s`` specifiers can be used for printing symbols > +from direct addresses, for example, __builtin_return_address(0), > +(void *)regs->ip. They result in the symbol name with (``S``) or > +without (``s``) offsets. If KALLSYMS are disabled then the symbol > +address is printed instead. > > The ``B`` specifier results in the symbol name with offsets and should be > used when printing stack backtraces. The specifier takes into > consideration the effect of compiler optimisations which may occur > when tail-call``s are used and marked with the noreturn GCC attribute. > > -On ia64, ppc64 and parisc64 architectures function pointers are > -actually function descriptors which must first be resolved. The ``F`` and > -``f`` specifiers perform this resolution and then provide the same > -functionality as the ``S`` and ``s`` specifiers. > > Kernel Pointers > =============== >