[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eeada499-1624-6a6c-783a-d017301bbd86@redhat.com>
Date: Wed, 5 Sep 2018 17:20:53 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Petr Mladek <pmladek@...e.com>,
Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H . Peter Anvin" <hpa@...or.com>,
Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
x86@...nel.org, linux-kernel@...r.kernel.org,
Steven Rostedt <rostedt@...dmis.org>,
Maninder Singh <maninder1.s@...sung.com>
Subject: Re: [PATCH 4.19 regression fix] printk: For early boot messages check
loglevel when flushing the buffer
HI,
On 05-09-18 13:02, Petr Mladek wrote:
> On Wed 2018-09-05 17:33:26, Sergey Senozhatsky wrote:
>> On (09/05/18 14:36), Sergey Senozhatsky wrote:
>>>
>>> Just a demonstration of the idea. It does not look very good, tho.
>>> I'd rather have just one suppress_message_printing() in printk code.
>>>
>>> // This is not a proposed patch, hence the 80-cols violation.
>>>
>>> ---
>>>
>>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>>> index c036f128cdc3..231ac18423e1 100644
>>> --- a/kernel/printk/printk.c
>>> +++ b/kernel/printk/printk.c
>>> @@ -2416,7 +2416,7 @@ void console_unlock(void)
>>> break;
>>>
>>> msg = log_from_idx(console_idx);
>>> - if (msg->flags & LOG_NOCONS) {
>>> + if (msg->flags & LOG_NOCONS || (exclusive_console && suppress_message_printing(msg->level))) {
>>> /*
>>> * Skip record if !ignore_loglevel, and
>>> * record has level above the console loglevel.
>>
>> D'oh... Sorry about that, but, believe it or not, this is completely
>> not what I had in my mind. What I had, was something like this:
>>
>> ---
>>
>> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
>> index c036f128cdc3..dadb8c11b0d6 100644
>> --- a/kernel/printk/printk.c
>> +++ b/kernel/printk/printk.c
>> @@ -2416,6 +2416,11 @@ void console_unlock(void)
>> break;
>>
>> msg = log_from_idx(console_idx);
>> +
>> + if (exclusive_console &&
>> + !suppress_message_printing(msg->level))
>> + msg->flags &= ~LOG_NOCONS;
>
> Hmm, this does not help with consoles without CON_PRINTBUFFER
> flag. Note that the first registered console prints all messages
> even without this flag.
>
> Also there is "debug" earlyparam where we need the opposite. I mean
> that we want to show messages that were suppressed by default.
>
> I played with another solution, see the patch below. It defines
> which messages have a valid NOCONS flag according to the msg_seq
> number. IMHO, it is a bit more straightforward but it is still
> a hack. I am not super happy about it.
>
>
> Hmm, I seriously think about reverting the commit 375899cddcbb
> ("printk: make sure to print log on console.") and solving it
> another way.
>
> For example, the commit was primary about locations that
> wanted to make some messages always visible or always
> suppressed. We could create LOG_FORCE_NOCONS and
> LOG_FORCE_CONS for these two special cases.
>
>
>
> Possible solution:
So do you want me to give this solution a try or was this mainly for
discussion purposes? If you've a fix which you think you are
happy with and plan to merge I would be happy to try it.
Regards,
Hans
>
>
> From c0fb2d83ca3ca0bab5e1de5a6e29a1b96756a530 Mon Sep 17 00:00:00 2001
> From: Petr Mladek <pmladek@...e.com>
> Date: Wed, 5 Sep 2018 12:57:18 +0200
> Subject: [RFC PATCH] printk: Possible solution for loglevel manipulation by
> earlyparams
>
> Early params that manipulate console_loglevel should invalidate
> LOG_NOCONS flag for all the existing messages.
> ---
> include/linux/console.h | 2 ++
> init/main.c | 6 +++---
> kernel/printk/printk.c | 27 +++++++++++++++++++++++----
> 3 files changed, 28 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/console.h b/include/linux/console.h
> index ec9bdb3d7bab..3d3f7ab8f82e 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -195,6 +195,8 @@ extern bool console_suspend_enabled;
> extern void suspend_console(void);
> extern void resume_console(void);
>
> +extern void console_set_default_loglevel(int loglevel);
> +
> int mda_console_init(void);
> void prom_con_init(void);
>
> diff --git a/init/main.c b/init/main.c
> index 18f8f0140fa0..936466209494 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -213,13 +213,13 @@ EXPORT_SYMBOL(loops_per_jiffy);
>
> static int __init debug_kernel(char *str)
> {
> - console_loglevel = CONSOLE_LOGLEVEL_DEBUG;
> + console_set_default_loglevel(CONSOLE_LOGLEVEL_DEBUG);
> return 0;
> }
>
> static int __init quiet_kernel(char *str)
> {
> - console_loglevel = CONSOLE_LOGLEVEL_QUIET;
> + console_set_default_loglevel(CONSOLE_LOGLEVEL_QUIET);
> return 0;
> }
>
> @@ -236,7 +236,7 @@ static int __init loglevel(char *str)
> * are quite hard to debug
> */
> if (get_option(&str, &newlevel)) {
> - console_loglevel = newlevel;
> + console_set_default_loglevel(newlevel);
> return 0;
> }
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 924e37fb1620..b5a0074302e9 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -430,6 +430,9 @@ static u32 console_idx;
> static u64 clear_seq;
> static u32 clear_idx;
>
> +/* Invalidate nocons flag setting before early param are proceed */
> +static u64 console_valid_nocons_seq;
> +
> #define PREFIX_MAX 32
> #define LOG_LINE_MAX (1024 - PREFIX_MAX)
>
> @@ -1148,11 +1151,19 @@ module_param(ignore_loglevel, bool, S_IRUGO | S_IWUSR);
> MODULE_PARM_DESC(ignore_loglevel,
> "ignore loglevel setting (prints all kernel messages to the console)");
>
> -static bool suppress_message_printing(int level)
> +static bool nocons_loglevel(int level)
> {
> return (level >= console_loglevel && !ignore_loglevel);
> }
>
> +static bool nocons_msg(struct printk_log *msg)
> +{
> + if (console_seq >= console_valid_nocons_seq)
> + return msg->flags & LOG_NOCONS;
> +
> + return nocons_loglevel(msg->level);
> +}
> +
> #ifdef CONFIG_BOOT_PRINTK_DELAY
>
> static int boot_delay; /* msecs delay after each printk during bootup */
> @@ -1182,7 +1193,7 @@ static void boot_delay_msec(int level)
> unsigned long timeout;
>
> if ((boot_delay == 0 || system_state >= SYSTEM_RUNNING)
> - || suppress_message_printing(level)) {
> + || nocons_loglevel(level)) {
> return;
> }
>
> @@ -1882,7 +1893,7 @@ int vprintk_store(int facility, int level,
> if (dict)
> lflags |= LOG_PREFIX|LOG_NEWLINE;
>
> - if (suppress_message_printing(level))
> + if (nocons_loglevel(level))
> lflags |= LOG_NOCONS;
>
> return log_output(facility, level, lflags,
> @@ -2031,6 +2042,7 @@ static void console_lock_spinning_enable(void) { }
> static int console_lock_spinning_disable_and_check(void) { return 0; }
> static void call_console_drivers(const char *ext_text, size_t ext_len,
> const char *text, size_t len) {}
> +static bool nocons_msg(struct printk_log *msg) { return false; }
> static size_t msg_print_text(const struct printk_log *msg,
> bool syslog, char *buf, size_t size) { return 0; }
>
> @@ -2197,6 +2209,13 @@ void resume_console(void)
> console_unlock();
> }
>
> +void console_set_default_loglevel(int loglevel)
> +{
> + console_loglevel = loglevel;
> + /* Invalidate nocons flag for earlier messages. */
> + console_valid_nocons_seq = log_next_seq;
> +}
> +
> /**
> * console_cpu_notify - print deferred console messages after CPU hotplug
> * @cpu: unused
> @@ -2369,7 +2388,7 @@ void console_unlock(void)
> break;
>
> msg = log_from_idx(console_idx);
> - if (msg->flags & LOG_NOCONS) {
> + if (nocons_msg(msg)) {
> /*
> * Skip record if !ignore_loglevel, and
> * record has level above the console loglevel.
>
Powered by blists - more mailing lists