All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-kernel@vger.kernel.org, Helge Deller <deller@gmx.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Juergen Gross <jgross@suse.com>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Tom Rix <trix@redhat.com>,
	linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH printk v3 33/40] printk, xen: fbfront: create/use safe function for forcing preferred
Date: Thu, 10 Nov 2022 17:09:12 +0106	[thread overview]
Message-ID: <877d026blr.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <Y20aBwNWT19YDeib@alley>

On 2022-11-10, Petr Mladek <pmladek@suse.com> wrote:
>> +void console_force_preferred_locked(struct console *con)
>> +{
>> +	struct console *cur_pref_con;
>> +
>> +	if (!console_is_registered_locked(con))
>> +		return;
>> +
>> +	cur_pref_con = console_first();
>> +
>> +	/* Already preferred? */
>> +	if (cur_pref_con == con)
>> +		return;
>> +
>> +	hlist_del_init_rcu(&con->node);
>
> We actually should re-initialize the node only after all existing
> console list walks are finished. Se we should use here:
>
> 	hlist_del_rcu(&con->node);

hlist_del_init_rcu() only re-initializes @pprev pointer. But maybe you
are concerned that there is a window where list_unhashed() becomes true?
I agree that it should be changed to hlist_del_rcu() because there
should not be a window where this console appears unregistered.

>> +	/* Only the new head can have CON_CONSDEV set. */
>> +	WRITE_ONCE(cur_pref_con->flags, cur_pref_con->flags & ~CON_CONSDEV);
>
> As mentioned in the reply for 7th patch, I would prefer to hide this
> WRITE_ONCE into a wrapper, e.g. console_set_flag(). It might also
> check that the console_list_lock is taken...

Agreed. For v4 it will become:

console_srcu_write_flags(cur_pref_con->flags & ~CON_CONSDEV);

John

WARNING: multiple messages have this Message-ID (diff)
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Juergen Gross <jgross@suse.com>,
	linux-fbdev@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Helge Deller <deller@gmx.de>,
	linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Javier Martinez Canillas <javierm@redhat.com>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	dri-devel@lists.freedesktop.org,
	Thomas Zimmermann <tzimmermann@suse.de>,
	Tom Rix <trix@redhat.com>, Thomas Gleixner <tglx@linutronix.de>,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH printk v3 33/40] printk, xen: fbfront: create/use safe function for forcing preferred
Date: Thu, 10 Nov 2022 17:09:12 +0106	[thread overview]
Message-ID: <877d026blr.fsf@jogness.linutronix.de> (raw)
In-Reply-To: <Y20aBwNWT19YDeib@alley>

On 2022-11-10, Petr Mladek <pmladek@suse.com> wrote:
>> +void console_force_preferred_locked(struct console *con)
>> +{
>> +	struct console *cur_pref_con;
>> +
>> +	if (!console_is_registered_locked(con))
>> +		return;
>> +
>> +	cur_pref_con = console_first();
>> +
>> +	/* Already preferred? */
>> +	if (cur_pref_con == con)
>> +		return;
>> +
>> +	hlist_del_init_rcu(&con->node);
>
> We actually should re-initialize the node only after all existing
> console list walks are finished. Se we should use here:
>
> 	hlist_del_rcu(&con->node);

hlist_del_init_rcu() only re-initializes @pprev pointer. But maybe you
are concerned that there is a window where list_unhashed() becomes true?
I agree that it should be changed to hlist_del_rcu() because there
should not be a window where this console appears unregistered.

>> +	/* Only the new head can have CON_CONSDEV set. */
>> +	WRITE_ONCE(cur_pref_con->flags, cur_pref_con->flags & ~CON_CONSDEV);
>
> As mentioned in the reply for 7th patch, I would prefer to hide this
> WRITE_ONCE into a wrapper, e.g. console_set_flag(). It might also
> check that the console_list_lock is taken...

Agreed. For v4 it will become:

console_srcu_write_flags(cur_pref_con->flags & ~CON_CONSDEV);

John

  reply	other threads:[~2022-11-10 16:03 UTC|newest]

Thread overview: 129+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-07 14:15 [PATCH printk v3 00/40] reduce console_lock scope John Ogness
2022-11-07 14:15 ` John Ogness
2022-11-07 14:15 ` John Ogness
2022-11-07 14:15 ` John Ogness
2022-11-07 14:15 ` [PATCH printk v3 01/40] rcu: implement lockdep_rcu_enabled for !CONFIG_DEBUG_LOCK_ALLOC John Ogness
2022-11-07 18:01   ` Paul E. McKenney
2022-11-07 19:23     ` John Ogness
2022-11-08 19:27       ` Paul E. McKenney
2022-11-09 17:49         ` Paul E. McKenney
2022-11-08 10:29   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 02/40] serial: kgdboc: Lock console list in probe function John Ogness
2022-11-09  8:20   ` Daniel Thompson
2022-11-07 14:16 ` [PATCH printk v3 03/40] printk: Convert console_drivers list to hlist John Ogness
2022-11-07 14:16 ` [PATCH printk v3 04/40] printk: Prepare for SRCU console list protection John Ogness
2022-11-08 12:14   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 05/40] printk: fix setting first seq for consoles John Ogness
2022-11-08 12:20   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 06/40] um: kmsg_dump: only dump when no output console available John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-08 12:41   ` Petr Mladek
2022-11-08 12:41     ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 07/40] console: introduce console_is_enabled() wrapper John Ogness
2022-11-08 16:18   ` Petr Mladek
2022-11-10 15:05     ` John Ogness
2022-11-11 13:32       ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 08/40] printk: use console_is_enabled() John Ogness
2022-11-08 17:40   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 09/40] um: kmsg_dump: " John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-09 14:52   ` Petr Mladek
2022-11-09 14:52     ` Petr Mladek
2022-11-09 14:56   ` Petr Mladek
2022-11-09 14:56     ` Petr Mladek
2022-11-09 14:57     ` Petr Mladek
2022-11-09 14:57       ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 10/40] kdb: kdb_io: " John Ogness
2022-11-09  8:22   ` Daniel Thompson
2022-11-09 14:59   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 11/40] um: kmsg_dumper: use srcu console list iterator John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-09 15:05   ` Petr Mladek
2022-11-09 15:05     ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 12/40] tty: serial: kgdboc: document console_lock usage John Ogness
2022-11-09  8:23   ` Daniel Thompson
2022-11-09 15:19   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 13/40] tty: tty_io: " John Ogness
2022-11-09 15:20   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 14/40] proc: consoles: " John Ogness
2022-11-09 15:22   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 15/40] kdb: use srcu console list iterator John Ogness
2022-11-09  8:53   ` Daniel Thompson
2022-11-09  9:27     ` John Ogness
2022-11-09 15:27       ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 16/40] printk: console_flush_all: " John Ogness
2022-11-07 14:16 ` [PATCH printk v3 17/40] printk: console_unblank: " John Ogness
2022-11-07 14:16 ` [PATCH printk v3 18/40] printk: console_flush_on_panic: " John Ogness
2022-11-07 14:16 ` [PATCH printk v3 19/40] printk: console_device: " John Ogness
2022-11-09 15:58   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 20/40] printk: __pr_flush: " John Ogness
2022-11-07 14:16 ` [PATCH printk v3 21/40] printk: introduce console_list_lock John Ogness
2022-11-10 12:58   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 22/40] console: introduce console_is_registered() John Ogness
2022-11-10 13:00   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 23/40] serial_core: replace uart_console_enabled() with uart_console_registered() John Ogness
2022-11-08  8:46   ` Geert Uytterhoeven
2022-11-10 13:24     ` Petr Mladek
2022-11-10 13:46       ` John Ogness
2022-11-07 14:16 ` [PATCH printk v3 24/40] tty: nfcon: use console_is_registered() John Ogness
2022-11-08  8:39   ` Geert Uytterhoeven
2022-11-10 13:58   ` Petr Mladek
2022-11-10 14:19     ` John Ogness
2022-11-10 17:50       ` Eero Tamminen
2022-11-07 14:16 ` [PATCH printk v3 25/40] efi: earlycon: " John Ogness
2022-11-10 14:28   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 26/40] tty: hvc: " John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-10 14:52   ` Petr Mladek
2022-11-10 14:52     ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 27/40] tty: serial: earlycon: " John Ogness
2022-11-10 15:00   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 28/40] tty: serial: pic32_uart: " John Ogness
2022-11-10 15:01   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 29/40] tty: serial: samsung_tty: " John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-10 15:01   ` Petr Mladek
2022-11-10 15:01     ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 30/40] tty: serial: xilinx_uartps: " John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-10 15:04   ` Petr Mladek
2022-11-10 15:04     ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 31/40] usb: early: xhci-dbc: " John Ogness
2022-11-10 15:05   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 32/40] netconsole: avoid CON_ENABLED misuse to track registration John Ogness
2022-11-10 15:17   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 33/40] printk, xen: fbfront: create/use safe function for forcing preferred John Ogness
2022-11-07 14:16   ` John Ogness
2022-11-10 15:34   ` Petr Mladek
2022-11-10 15:34     ` Petr Mladek
2022-11-10 16:03     ` John Ogness [this message]
2022-11-10 16:03       ` John Ogness
2022-11-10 17:26       ` Petr Mladek
2022-11-10 17:26         ` Petr Mladek
2022-11-10 22:37         ` John Ogness
2022-11-10 22:37           ` John Ogness
2022-11-11 10:48   ` Petr Mladek
2022-11-11 10:48     ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 34/40] tty: tty_io: use console_list_lock for list synchronization John Ogness
2022-11-07 14:16 ` [PATCH printk v3 35/40] proc: consoles: use console_list_lock for list iteration John Ogness
2022-11-07 14:16 ` [PATCH printk v3 36/40] tty: serial: kgdboc: use console_list_lock for list traversal John Ogness
2022-11-09  9:06   ` Daniel Thompson
2022-11-09  9:44     ` John Ogness
2022-11-10 18:00       ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 37/40] tty: serial: kgdboc: synchronize tty_find_polling_driver() and register_console() John Ogness
2022-11-10 15:13   ` Daniel Thompson
2022-11-10 18:03   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 38/40] tty: serial: kgdboc: use console_list_lock to trap exit John Ogness
2022-11-10 15:18   ` Daniel Thompson
2022-11-11  9:59   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 39/40] printk: relieve console_lock of list synchronization duties John Ogness
2022-11-07 16:30   ` John Ogness
2022-11-11 10:27     ` Petr Mladek
2022-11-11 13:06   ` Petr Mladek
2022-11-07 14:16 ` [PATCH printk v3 40/40] tty: serial: sh-sci: use setup() callback for early console John Ogness
2022-11-08  8:53   ` Geert Uytterhoeven
2022-11-11 17:15   ` Petr Mladek
2022-11-11 14:43 ` [PATCH printk v3 00/40] reduce console_lock scope Mathieu Desnoyers
2022-11-11 14:43   ` Mathieu Desnoyers
2022-11-11 14:43   ` Mathieu Desnoyers
2022-11-11 14:43   ` Mathieu Desnoyers

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=877d026blr.fsf@jogness.linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=boris.ostrovsky@oracle.com \
    --cc=deller@gmx.de \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=javierm@redhat.com \
    --cc=jgross@suse.com \
    --cc=linux-fbdev@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=senozhatsky@chromium.org \
    --cc=tglx@linutronix.de \
    --cc=trix@redhat.com \
    --cc=tzimmermann@suse.de \
    /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.