All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Petr Mladek <pmladek@suse.com>
Cc: linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	John Ogness <john.ogness@linutronix.de>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	kernel-team@fb.com
Subject: Re: [PATCH v5 2/2] printk: console: Support console-specific loglevels
Date: Tue, 2 May 2023 15:26:37 +0100	[thread overview]
Message-ID: <ZFEdnY27pZlBvlih@chrisdown.name> (raw)
In-Reply-To: <ZEp9dXwHCYNPidjC@alley>

Thanks for the detailed feedback! All comments not explicitly replied to are 
acked and will be updated in v6.

Petr Mladek writes:
>> @@ -2293,7 +2429,14 @@ asmlinkage int vprintk_emit(int facility, int level,
>>  		in_sched = true;
>>  	}
>>
>> -	printk_delay(level);
>> +	cookie = console_srcu_read_lock();
>> +	for_each_console_srcu(con) {
>> +		if (!suppress_message_printing(level, con)) {
>> +			printk_delay();
>
>This would repeat the delay for each registered and non-supressed console.
>But it should be called only once when there is at least one
>non-suppressed console.
>
>> +			break;
>> +		}
>> +	}
>> +	console_srcu_read_unlock(cookie);

Doesn't the `break' below avoid that already? Maybe I'm missing something?

[...]

>> +static int printk_sysctl_deprecated(struct ctl_table *table, int write,
>> +				    void *buffer, size_t *lenp, loff_t *ppos)
>> +{
>> +	int res = proc_dointvec(table, write, buffer, lenp, ppos);
>> +
>> +	if (write)
>> +		pr_warn_once(
>> +			"printk: The kernel.printk sysctl is deprecated. Consider using kernel.console_loglevel or kernel.default_message_loglevel instead.\n");
>
>Nit: Please, put it on the same line as pr_warn_once("printk: The kernel....
>     The small shift left did not help much ;-)

I think this is done by clang-format with kernel default config, since my 
preference is single line as well. Maybe worth discussing changing the config?

>How does this sounds, please?

Sounds great! Thank you again for the very detailed feedback :-)

  parent reply	other threads:[~2023-05-02 14:26 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-25 13:06 [PATCH v5 0/2] printk: console: Per-console loglevels Chris Down
2023-04-25 13:06 ` [PATCH v5 1/2] printk: Do not delay messages which aren't solicited by any console Chris Down
2023-04-26  8:55   ` Petr Mladek
2023-04-25 13:06 ` [PATCH v5 2/2] printk: console: Support console-specific loglevels Chris Down
2023-04-27 13:49   ` Petr Mladek
2023-04-27 15:41     ` John Ogness
2023-05-02 14:28       ` Chris Down
2023-04-30 17:00     ` [PATCH v5 2/2] printk: console: Remove sysrq exception calumlikesapplepie
2023-05-02  9:23       ` Chris Down
2023-05-02 10:27       ` Petr Mladek
2023-05-02 14:26     ` Chris Down [this message]
2023-04-28 16:19   ` [PATCH v5 2/2] printk: console: Support console-specific loglevels kernel test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZFEdnY27pZlBvlih@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=geert@linux-m68k.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=john.ogness@linutronix.de \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.