* [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses [not found] <1504729681-3504-1-git-send-email-deller@gmx.de> @ 2017-09-06 20:27 ` Helge Deller 2017-09-07 4:50 ` Coly Li 0 siblings, 1 reply; 5+ messages in thread From: Helge Deller @ 2017-09-06 20:27 UTC (permalink / raw) To: linux-kernel Cc: Sergey Senozhatsky, Petr Mladek, Andrew Morton, linux-bcache, linux-raid Use the %pS instead of the %pF printk format specifier for printing symbols from direct addresses. This is needed for the ia64, ppc64 and parisc64 architectures. Signed-off-by: Helge Deller <deller@gmx.de> Cc: linux-bcache@vger.kernel.org Cc: linux-raid@vger.kernel.org --- drivers/md/bcache/closure.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c index 864e673..0b0c9bc 100644 --- a/drivers/md/bcache/closure.c +++ b/drivers/md/bcache/closure.c @@ -175,7 +175,7 @@ static int debug_seq_show(struct seq_file *f, void *data) list_for_each_entry(cl, &closure_list, all) { int r = atomic_read(&cl->remaining); - seq_printf(f, "%p: %pF -> %pf p %p r %i ", + seq_printf(f, "%p: %pS -> %pf p %p r %i ", cl, (void *) cl->ip, cl->fn, cl->parent, r & CLOSURE_REMAINING_MASK); @@ -187,7 +187,7 @@ static int debug_seq_show(struct seq_file *f, void *data) r & CLOSURE_SLEEPING ? "Sl" : ""); if (r & CLOSURE_WAITING) - seq_printf(f, " W %pF\n", + seq_printf(f, " W %pS\n", (void *) cl->waiting_on); seq_printf(f, "\n"); -- 2.1.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses 2017-09-06 20:27 ` [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses Helge Deller @ 2017-09-07 4:50 ` Coly Li 2017-09-07 7:42 ` Helge Deller 0 siblings, 1 reply; 5+ messages in thread From: Coly Li @ 2017-09-07 4:50 UTC (permalink / raw) To: Helge Deller Cc: linux-kernel, Sergey Senozhatsky, Petr Mladek, Andrew Morton, linux-bcache, linux-raid On 2017/9/7 上午4:27, Helge Deller wrote: > Use the %pS instead of the %pF printk format specifier for printing symbols > from direct addresses. This is needed for the ia64, ppc64 and parisc64 > architectures. > > Signed-off-by: Helge Deller <deller@gmx.de> > Cc: linux-bcache@vger.kernel.org > Cc: linux-raid@vger.kernel.org > --- > drivers/md/bcache/closure.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c > index 864e673..0b0c9bc 100644 > --- a/drivers/md/bcache/closure.c > +++ b/drivers/md/bcache/closure.c > @@ -175,7 +175,7 @@ static int debug_seq_show(struct seq_file *f, void *data) > list_for_each_entry(cl, &closure_list, all) { > int r = atomic_read(&cl->remaining); > > - seq_printf(f, "%p: %pF -> %pf p %p r %i ", > + seq_printf(f, "%p: %pS -> %pf p %p r %i ", > cl, (void *) cl->ip, cl->fn, cl->parent, > r & CLOSURE_REMAINING_MASK); > > @@ -187,7 +187,7 @@ static int debug_seq_show(struct seq_file *f, void *data) > r & CLOSURE_SLEEPING ? "Sl" : ""); > > if (r & CLOSURE_WAITING) > - seq_printf(f, " W %pF\n", > + seq_printf(f, " W %pS\n", > (void *) cl->waiting_on); > > seq_printf(f, "\n"); > It is unclear to me, that if %pF is used, on ia64/ppc64/parisc64 a function descriptor conversion happens, what negative effect exactly takes place ? Or you just want to unify the out put format, and get rid of extra conversion information ? Thanks. -- Coly Li ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses 2017-09-07 4:50 ` Coly Li @ 2017-09-07 7:42 ` Helge Deller 2017-09-07 7:49 ` Coly Li 2017-09-07 8:05 ` Sergey Senozhatsky 0 siblings, 2 replies; 5+ messages in thread From: Helge Deller @ 2017-09-07 7:42 UTC (permalink / raw) To: Coly Li Cc: linux-kernel, Sergey Senozhatsky, Petr Mladek, Andrew Morton, linux-bcache, linux-raid On 07.09.2017 06:50, Coly Li wrote: > On 2017/9/7 上午4:27, Helge Deller wrote: >> Use the %pS instead of the %pF printk format specifier for printing symbols >> from direct addresses. This is needed for the ia64, ppc64 and parisc64 >> architectures. >> >> Signed-off-by: Helge Deller <deller@gmx.de> >> Cc: linux-bcache@vger.kernel.org >> Cc: linux-raid@vger.kernel.org >> --- >> drivers/md/bcache/closure.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c >> index 864e673..0b0c9bc 100644 >> --- a/drivers/md/bcache/closure.c >> +++ b/drivers/md/bcache/closure.c >> @@ -175,7 +175,7 @@ static int debug_seq_show(struct seq_file *f, void *data) >> list_for_each_entry(cl, &closure_list, all) { >> int r = atomic_read(&cl->remaining); >> >> - seq_printf(f, "%p: %pF -> %pf p %p r %i ", >> + seq_printf(f, "%p: %pS -> %pf p %p r %i ", >> cl, (void *) cl->ip, cl->fn, cl->parent, >> r & CLOSURE_REMAINING_MASK); >> >> @@ -187,7 +187,7 @@ static int debug_seq_show(struct seq_file *f, void *data) >> r & CLOSURE_SLEEPING ? "Sl" : ""); >> >> if (r & CLOSURE_WAITING) >> - seq_printf(f, " W %pF\n", >> + seq_printf(f, " W %pS\n", >> (void *) cl->waiting_on); >> >> seq_printf(f, "\n"); >> > > It is unclear to me, that if %pF is used, on ia64/ppc64/parisc64 a > function descriptor conversion happens, what negative effect exactly > takes place ? On ia64/ppc64/parisc64 the kernel will crash here in the worst case, because vsprintf() will try to read a pointer from that address and resolve it. But you hand over a return address (_RET_IP_) which should be resolved directly to a symbol. For that you need to use the %pS specifier, which is what my patch does. The patch has no negative effect on any platform. Output will still be the same on x86/mips/arm/... as it has been before with %pF. The only positive effect of the patch is that the seq_printf will now show correct output on ia64/ppc64/parisc64 too. Helge ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses 2017-09-07 7:42 ` Helge Deller @ 2017-09-07 7:49 ` Coly Li 2017-09-07 8:05 ` Sergey Senozhatsky 1 sibling, 0 replies; 5+ messages in thread From: Coly Li @ 2017-09-07 7:49 UTC (permalink / raw) To: Helge Deller Cc: linux-kernel, Sergey Senozhatsky, Petr Mladek, Andrew Morton, linux-bcache, linux-raid On 2017/9/7 下午3:42, Helge Deller wrote: > On 07.09.2017 06:50, Coly Li wrote: >> On 2017/9/7 上午4:27, Helge Deller wrote: >>> Use the %pS instead of the %pF printk format specifier for printing symbols >>> from direct addresses. This is needed for the ia64, ppc64 and parisc64 >>> architectures. >>> >>> Signed-off-by: Helge Deller <deller@gmx.de> >>> Cc: linux-bcache@vger.kernel.org >>> Cc: linux-raid@vger.kernel.org >>> --- >>> drivers/md/bcache/closure.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/md/bcache/closure.c b/drivers/md/bcache/closure.c >>> index 864e673..0b0c9bc 100644 >>> --- a/drivers/md/bcache/closure.c >>> +++ b/drivers/md/bcache/closure.c >>> @@ -175,7 +175,7 @@ static int debug_seq_show(struct seq_file *f, void *data) >>> list_for_each_entry(cl, &closure_list, all) { >>> int r = atomic_read(&cl->remaining); >>> >>> - seq_printf(f, "%p: %pF -> %pf p %p r %i ", >>> + seq_printf(f, "%p: %pS -> %pf p %p r %i ", >>> cl, (void *) cl->ip, cl->fn, cl->parent, >>> r & CLOSURE_REMAINING_MASK); >>> >>> @@ -187,7 +187,7 @@ static int debug_seq_show(struct seq_file *f, void *data) >>> r & CLOSURE_SLEEPING ? "Sl" : ""); >>> >>> if (r & CLOSURE_WAITING) >>> - seq_printf(f, " W %pF\n", >>> + seq_printf(f, " W %pS\n", >>> (void *) cl->waiting_on); >>> >>> seq_printf(f, "\n"); >>> >> >> It is unclear to me, that if %pF is used, on ia64/ppc64/parisc64 a >> function descriptor conversion happens, what negative effect exactly >> takes place ? > > On ia64/ppc64/parisc64 the kernel will crash here in the worst case, because > vsprintf() will try to read a pointer from that address and resolve it. > But you hand over a return address (_RET_IP_) which should be > resolved directly to a symbol. For that you need to use the %pS specifier, > which is what my patch does. > > The patch has no negative effect on any platform. Output will still be the > same on x86/mips/arm/... as it has been before with %pF. The only positive > effect of the patch is that the seq_printf will now show correct output on > ia64/ppc64/parisc64 too. Oh I see. The patch is OK to me, but could you please add the above information in the commit log, it will help other bcache developers to understand the patch. Thanks in advance for doing this. BTW, I also suggest to fix vsprintf() for this issue. For most of developers, we don't have sense for such issue on ia64/ppc64/parisc64, it is still very probably someone else makes similar mistake in future. If vsprintf() can be fixed, the potential risk can be safe. -- Coly Li ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses 2017-09-07 7:42 ` Helge Deller 2017-09-07 7:49 ` Coly Li @ 2017-09-07 8:05 ` Sergey Senozhatsky 1 sibling, 0 replies; 5+ messages in thread From: Sergey Senozhatsky @ 2017-09-07 8:05 UTC (permalink / raw) To: Helge Deller Cc: Coly Li, linux-kernel, Sergey Senozhatsky, Petr Mladek, Andrew Morton, linux-bcache, linux-raid On (09/07/17 09:42), Helge Deller wrote: > >> - seq_printf(f, "%p: %pF -> %pf p %p r %i ", > >> + seq_printf(f, "%p: %pS -> %pf p %p r %i ", > >> cl, (void *) cl->ip, cl->fn, cl->parent, > >> r & CLOSURE_REMAINING_MASK); > >> > >> @@ -187,7 +187,7 @@ static int debug_seq_show(struct seq_file *f, void *data) > >> r & CLOSURE_SLEEPING ? "Sl" : ""); > >> > >> if (r & CLOSURE_WAITING) > >> - seq_printf(f, " W %pF\n", > >> + seq_printf(f, " W %pS\n", > >> (void *) cl->waiting_on); > >> > >> seq_printf(f, "\n"); > >> > > > > It is unclear to me, that if %pF is used, on ia64/ppc64/parisc64 a > > function descriptor conversion happens, what negative effect exactly > > takes place ? > > On ia64/ppc64/parisc64 the kernel will crash here in the worst case, because > vsprintf() will try to read a pointer from that address and resolve it. probe_kernel_address() handles the page fault and returns -EFAULT if you give it bad pointer. module_address_lookup() and get_symbol_pos() seems to be smart enough not to crash on bad pointer as well. what am I missing? could you please explain where we will crash? -ss ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-09-07 8:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1504729681-3504-1-git-send-email-deller@gmx.de>
2017-09-06 20:27 ` [PATCH 06/14] md/bcache: Use %pS printk format for direct addresses Helge Deller
2017-09-07 4:50 ` Coly Li
2017-09-07 7:42 ` Helge Deller
2017-09-07 7:49 ` Coly Li
2017-09-07 8:05 ` Sergey Senozhatsky
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).