On Wed, Sep 02, 2009 at 02:22:52PM -0400, H Hartley Sweeten wrote:
> On Wednesday, September 02, 2009 10:05 AM, Tim Bird wrote:
> > Marc Andre Tanner wrote:
> >> On Tue, Sep 01, 2009 at 07:32:25PM -0400, H Hartley Sweeten wrote:
> >>> On Tuesday, September 01, 2009 4:24 PM, Tim Bird wrote:
> >>>> Some places in the kernel break the message into pieces, like so:
> >>>>
> >>>> printk(KERN_ERR, "Error: first part ");
> >>>> ...
> >>>> printk(" more info for error.\n");
> >>> Technically, shouldn't the second part of the message actually be:
> >>>
> >>> printk(KERN_CONT " more info for error.\n");
> >>>
> >>> Maybe some mechanism could be created to handle the continued message
> >>> if they have the KERN_CONT?
> >>
> >> Yes it's true that KERN_CONT isn't handled correctly, but I don't see a way
> >> to change that.
> >>
> >>>> These parts would not be handled consistently under certain
> >>>> conditions.
> >>>>
> >>>> It would be confusing to see only part of the message,
> >>>> but I don't know how often this construct is used.
> >>
> >> $ grep -R KERN_CONT linux-2.6 | wc -l
> >> 373
> >>
> >>>> Maybe
> >>>> another mechanism is needed to ensure that continuation
> >>>> printk lines have the same log level as their start strings.
> >>
> >> I currently don't see a way to achieve this with the CPP.
> >
> > If it's that few, then maybe it's OK to actually change
> > the code for those printk statements. (Heck, these locations
> > were all changed in the last 2 years anyway.)
> >
> > I'm just brainstorming here, but how about changing them from:
> > printk(KERN_INFO "foo");
> > printk(KERN_CONT "bar\n");
> > to:
> > printk(KERN_INFO "foo");
> > printk_cont(KERN_INFO "bar\n");
>
> Unfortunately not all the continued printk statements in the kernel are
> properly tagged with KERN_CONT (or pr_cont, etc.).
Yes and a quick search revealed that there are actually quite a lot of
those untagged messages.
As I needed some distraction from some other work I played around a bit
more with a solution which wouldn't require to change existing kernel
code. It uses a variable which would have to be available in every
function to store the loglevel of messages which are split over multiple
printk calls. Here is what I came up with so far:
#define PRINTK_IS_LINE(fmt) ( \
((const char*)(fmt))[sizeof((fmt)) - 2] == '\n' \
)
#define PRINTK_LEVEL(fmt) ( \
(((const char *)(fmt))[0] == '<' && \
((const char *)(fmt))[1] >= '0' && \
((const char *)(fmt))[1] <= '9' \
) ? ((const char *)(fmt))[1] - '0' : 4 \
)
#define printk(fmt, ...) ({ \
int __printk_ret = 0; \
int __printk_level = __printk_cont_level; \
\
if (__builtin_constant_p((fmt)) && ((fmt))) { \
/* check if we are dealing with a line ending */ \
if (PRINTK_IS_LINE((fmt))) { \
/* check if it was a whole line */ \
if (__printk_cont_level == -1) \
__printk_level = PRINTK_LEVEL((fmt)); \
__printk_cont_level = -1; \
} else if (__printk_cont_level == -1) /* first part of a line? */ \
__printk_level = __printk_cont_level = PRINTK_LEVEL((fmt)); \
} \
\
if (!__builtin_constant_p((fmt)) || __printk_level <= CONFIG_PRINTK_VERBOSITY) \
__printk_ret = printk((fmt), ##__VA_ARGS__); \
\
__printk_ret; \
})
Apart from the fact that it's getting uglier it obviously depends on
the availability of __printk_cont_level (which would get compiled out,
at least I hope so) in every function.
So
void demo() {
...
}
would have to become:
void demo() {
int __printk_cont_level = -1;
...
}
I don't know whether this is possible at all through some kind of gcc
magic. There is also the problem of introducing warnings when
__prink_cont_level isn't used in a function but I guess this could be
dealt with some __attribute__ setting.
Anyway just wanted to share the results of a brainstorming session.
Marc
--
Marc Andre Tanner >< http://www.brain-dump.org/ >< GPG key: CF7D56C0
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
