headers
Matthew Wilcox | 4 Jul 22:36

Re: the printk problem

On Fri, Jul 04, 2008 at 01:02:05PM -0700, Linus Torvalds wrote:
> On Fri, 4 Jul 2008, Linus Torvalds wrote:
> > 
> > so I think we could easily just say that we extend %p in various ways:
> > 
> >  - %pS - print pointer as a symbol
> > 
> > and leave tons of room for future extensions for different kinds of 
> > pointers. 
> 
> So here's a totally untested example patch of this, which could probably 
> easily be extended to to other things.
> 
> I actually made it '%pF' and '%pS' for a Function descriptor pointer and 
> normal Symbolic pointer respectively, because of the stupid things ia64 
> and PPC64 do with the pointer indirection through function descriptors. 

It's also true for parisc, fwiw.  Added a cc to them.

> That function descriptor indirection is totally untested, and I did it 
> with a
> 
> 	pagefault_disable();
> 	__get_user(..)
> 	pagefault_enable();
> 
> thing because I thought it would be nice if printk() was always safe, and 
> passing bogus function pointers to '%pF' should try to work, but quite 
> frankly, I didn't even check that that part compiles, much less works.
>
(Continue reading)

Permalink | Reply | 2 comments |
headers
Linus Torvalds | 5 Jul 01:25
Gravatar

Re: the printk problem


On Sat, 5 Jul 2008, Benjamin Herrenschmidt wrote:
> 
> I'll give it a try using probe_kernel_address() instead on monday.

Here's the updated patch which uses probe_kernel_address() instead (and 
moves the whole #ifdef mess out of the code that wants it and into a 
helper function - and maybe we should then put that helper into 
kallsyms.h, but that's a different issue).

Still all happily untested, of course. And still with no actual users 
converted.

			Linus

---
 lib/vsprintf.c |  108 +++++++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 80 insertions(+), 28 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6021757..1fbb1c0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -22,6 +22,8 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
 #include <linux/kernel.h>
+#include <linux/kallsyms.h>
+#include <linux/uaccess.h>
(Continue reading)

Permalink | Reply | 17 comments |
headers
Linus Torvalds | 6 Jul 00:32
Gravatar

Re: the printk problem


On Fri, 4 Jul 2008, Linus Torvalds wrote:
> 
> Still all happily untested, of course. And still with no actual users 
> converted.

Ok, it's tested, and here's an example usage conversion.

The diffstat pretty much says it all. It _does_ change the format of the 
stack trace entry a bit, but I don't think it's for the worse (unless it 
breaks things like the oops tracker - Arjan?)

It changes the symbol-in-module format from

	:ext3:add_dirent_to_buf+0x6c/0x26c

to

	add_dirent_to_buf+0x6c/0x26c [ext3]

but quite frankly, the latter was the standard format anyway (it's what 
"sprint_symbol()" gives you), and traps_64.c was the odd man out.

In fact, traps_32.c already uses the standard print_symbol() format, so it 
really was an issue of the 64-bit code being odd (and I assume that this 
also means that it cannot break the oops tracker, since it already had to 
be able to handle both formats).

I also removed the KALLSYMS dependency, so if KALLSYMS isn't enabled it 
will now give the same hex format twice, but I doubt we really care (such
(Continue reading)

Permalink | Reply | 7 comments |
headers
Arjan van de Ven | 6 Jul 00:57
Picon

Re: the printk problem

Linus Torvalds wrote:
> 
> On Fri, 4 Jul 2008, Linus Torvalds wrote:
>> Still all happily untested, of course. And still with no actual users 
>> converted.
> 
> Ok, it's tested, and here's an example usage conversion.
> 
> The diffstat pretty much says it all. It _does_ change the format of the 
> stack trace entry a bit, but I don't think it's for the worse (unless it 
> breaks things like the oops tracker - Arjan?)
> 
> It changes the symbol-in-module format from
> 
> 	:ext3:add_dirent_to_buf+0x6c/0x26c
> 
> to
> 
> 	add_dirent_to_buf+0x6c/0x26c [ext3]
> 
> but quite frankly, the latter was the standard format anyway (it's what 
> "sprint_symbol()" gives you), and traps_64.c was the odd man out.
> 

This won't break anything for me actually; I already deal with either case.

$ cat oopsparse.pl | wc -l
1252

the kernel is so inconsistent with oops formats (over time/across architectures)
(Continue reading)

Permalink | Reply | 0 comments |
headers
Ingo Molnar | 6 Jul 07:27
Picon
Picon
Favicon

Re: the printk problem


* Linus Torvalds <torvalds <at> linux-foundation.org> wrote:

> > Still all happily untested, of course. And still with no actual 
> > users converted.
> 
> Ok, it's tested, and here's an example usage conversion.
> 
> The diffstat pretty much says it all. It _does_ change the format of 
> the stack trace entry a bit, but I don't think it's for the worse 
> (unless it breaks things like the oops tracker - Arjan?)
> 
> It changes the symbol-in-module format from
> 
> 	:ext3:add_dirent_to_buf+0x6c/0x26c
> 
> to
> 
> 	add_dirent_to_buf+0x6c/0x26c [ext3]
> 
> but quite frankly, the latter was the standard format anyway (it's 
> what "sprint_symbol()" gives you), and traps_64.c was the odd man out.
> 
> In fact, traps_32.c already uses the standard print_symbol() format, 
> so it really was an issue of the 64-bit code being odd (and I assume 
> that this also means that it cannot break the oops tracker, since it 
> already had to be able to handle both formats).
> 
> I also removed the KALLSYMS dependency, so if KALLSYMS isn't enabled 
> it will now give the same hex format twice, but I doubt we really care
(Continue reading)

Permalink | Reply | 2 comments |
headers
Linus Torvalds | 6 Jul 07:37
Gravatar

Re: the printk problem


On Sun, 6 Jul 2008, Ingo Molnar wrote:
> 
> applied (with the commit message below) to tip/x86/debug for v2.6.27 
> merging, thanks Linus. Can i add your SOB too?

Sure, add my S-O-B. But I hope/assuem that you also added my earlier patch 
that added the support for '%pS' too? I'm not entirely sure that should go 
in an x86-specific branch, since it has nothing x86-specific in it.

Also, I've cleaned up the lib/vsnprintf.c patch to fix up some minor 
details. For example, it should not default to a field width of 
"2*sizeof(unsigned long)" - it should do that only if it ends up printing 
the pointer as a hex number.

So this is my current version.. (the "precision" thing was moved into the 
'pointer()' function, and to after the special case handling).

		Linus
---
 lib/vsprintf.c |  118 ++++++++++++++++++++++++++++++++++++++++----------------
 1 files changed, 85 insertions(+), 33 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 6021757..f60c7c0 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -22,6 +22,8 @@
 #include <linux/string.h>
 #include <linux/ctype.h>
(Continue reading)

Permalink | Reply | 2 comments |
headers
Ingo Molnar | 6 Jul 07:53
Picon
Picon
Favicon

Re: the printk problem


* Linus Torvalds <torvalds <at> linux-foundation.org> wrote:

> On Sun, 6 Jul 2008, Ingo Molnar wrote:
> > 
> > applied (with the commit message below) to tip/x86/debug for v2.6.27 
> > merging, thanks Linus. Can i add your SOB too?
> 
> Sure, add my S-O-B. But I hope/assuem that you also added my earlier 
> patch that added the support for '%pS' too? I'm not entirely sure that 
> should go in an x86-specific branch, since it has nothing x86-specific 
> in it.

yeah, agreed, combined it's not an x86 topic anymore.

[ There's some lkml trouble so i've missed the earlier patch. I'm not 
  sure the email problem is on my side, see how incomplete the 
  discussion is on lkml.org as well:

     http://lkml.org/lkml/2008/6/25/170   ]

Anyway, i have have added this second patch of yours to tip/core/printk 
and moved your first patch over to that topic (which relies on it).

That topic has a few other (smaller) printk enhancements queued for 
v2.6.27 already:

   git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git core/printk

[find other stats below]
(Continue reading)

Permalink | Reply | 1 comment |
headers
Ingo Molnar | 6 Jul 08:13
Picon
Picon
Favicon

Re: the printk problem


* Ingo Molnar <mingo <at> elte.hu> wrote:

> yeah, agreed, combined it's not an x86 topic anymore.
> 
> [ There's some lkml trouble so i've missed the earlier patch. I'm not 
>   sure the email problem is on my side, see how incomplete the 
>   discussion is on lkml.org as well:
> 
>      http://lkml.org/lkml/2008/6/25/170   ]
> 
> Anyway, i have have added this second patch of yours to 
> tip/core/printk and moved your first patch over to that topic (which 
> relies on it).

ah, i found the reason - it started out on linux-ia64 originally then 
moved over to lkml - so only half of the discussion was visible there. 
And linux-ia64 is one of the few vger lists i'm not subscribed to 
apparently. (there's no vger-please-give-me-all-emails list - making the 
following of Linux development even harder)

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
Benjamin Herrenschmidt | 7 Jul 03:14

Re: the printk problem

On Fri, 2008-07-04 at 16:25 -0700, Linus Torvalds wrote:
> 
> On Sat, 5 Jul 2008, Benjamin Herrenschmidt wrote:
> > 
> > I'll give it a try using probe_kernel_address() instead on monday.
> 
> Here's the updated patch which uses probe_kernel_address() instead (and 
> moves the whole #ifdef mess out of the code that wants it and into a 
> helper function - and maybe we should then put that helper into 
> kallsyms.h, but that's a different issue).
> 
> Still all happily untested, of course. And still with no actual users 
> converted.

Did a few tests and it seems to work. I'll stick a patch converting
powerpc to use %pS for oops display in -next.

Thanks !

Cheers,
Ben.

_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev <at> ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Permalink | Reply | 0 comments |
headers
Kyle McMartin | 8 Jul 03:44
Picon

Re: the printk problem

On Fri, Jul 04, 2008 at 02:36:21PM -0600, Matthew Wilcox wrote:
> It's also true for parisc, fwiw.  Added a cc to them.
> 

I posted a patch months ago for kallsyms on parisc, but it looks like
nobody ever responded or cared. Nice.
--
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |

Gmane