Messages in this thread | | | Date | Fri, 2 Dec 2022 13:21:34 +0100 | From | Petr Mladek <> | Subject | Re: [PATCH v2 1/3] printk: introduce new macros pr_<level>_cont() |
| |
On Wed 2022-11-30 15:50:55, Thomas Weißschuh wrote: > On 2022-11-30 14:59+0100, Petr Mladek wrote: > > On Fri 2022-11-25 21:33:40, Thomas Weißschuh wrote: > >> On 2022-11-25 12:18-0800, Joe Perches wrote: > >>> On Fri, 2022-11-25 at 20:09 +0100, Thomas Weißschuh wrote: > >>>> These macros emit continuation messages with explicit levels. > >>>> In case the continuation is logged separately from the original message > >>>> it will retain its level instead of falling back to KERN_DEFAULT. > >>>> > >>>> This remedies the issue that logs filtered by level contain stray > >>>> continuation messages without context. > >>>> > >>>> --- a/include/linux/printk.h > >>>> +++ b/include/linux/printk.h > >>>> @@ -701,6 +703,27 @@ do { \ > >>>> no_printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__) > >>>> #endif > >>>> > >>>> +/* > >>>> + * Print a continuation message with level. In case the continuation is split > >>>> + * from the main message it preserves the level. > >>>> + */ > >>>> + > >>>> +#define pr_emerg_cont(fmt, ...) \ > >>>> + printk(KERN_EMERG KERN_CONT pr_fmt(fmt), ##__VA_ARGS__) > >>> > >>> Aren't this rather backwards? > >>> KERN_CONT KERN_<LEVEL> seems to make more sense to me. > >> > >> If nobody else disagrees I'll do this for v3. > > > > I slightly prefer the way how it is now. IMHO, it makes it easier > > to check the related levels in /sys/kernel/debug/printk/index/vmlinux [*]: > > > > <6> kernel/power/process.c:227 thaw_kernel_threads "Restarting kernel threads ... " > > <6,c> kernel/power/process.c:218 thaw_processes "done.\n" > > <6> kernel/power/process.c:197 thaw_processes "Restarting tasks ... " > > <6,c> kernel/power/process.c:176 freeze_kernel_threads "\n" > > <6,c> kernel/power/process.c:174 freeze_kernel_threads "done." > > <6> kernel/power/process.c:169 freeze_kernel_threads "Freezing remaining freezable tasks ... " > > <6,c> kernel/power/process.c:140 freeze_processes "\n" > > <6,c> kernel/power/process.c:138 freeze_processes "done." > > <6> kernel/power/process.c:133 freeze_processes "Freezing user space processes ... " > > <6,c> kernel/power/process.c:105 try_to_freeze_tasks "(elapsed %d.%03d seconds) " > > I did not test it (will do so later) but it seems to me that the code in > kernel/printk/index.c should do this correctly in either case. At least it > tries to: > > if (flags & LOG_CONT) { > /* > * LOGLEVEL_DEFAULT here means "use the same level as the > * message we're continuing from", not the default message > * loglevel, so don't display it as such. > */ > if (level == LOGLEVEL_DEFAULT) > seq_puts(s, "<c>"); > else > seq_printf(s, "<%d,c>", level); > } else > seq_printf(s, "<%d>", level); > }
Great. It makes the index consistent. I should have checked the code ;-)
I do not mind then about the ordering in the macro definitions. It really seems to be only an implementation detail.
Best Regards, Petr
| |