* [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device
@ 2026-08-15 7:24 Andreas Schwab
2026-08-17 9:26 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Andreas Schwab @ 2026-08-15 7:24 UTC (permalink / raw)
To: linux-m68k
Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list
synchronization") show_cons_active calls the device method under the
console_list_lock, but console_is_registered tries to acquire
console_list_lock as well, causing a deadlock. It should not be necessary
to check console_is_registered here since the function should not be
called in the fist place when the console is not registered.
Fixes: 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")
Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
---
arch/m68k/emu/nfcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/m68k/emu/nfcon.c b/arch/m68k/emu/nfcon.c
index d41260672e24..f4657d9f4f15 100644
--- a/arch/m68k/emu/nfcon.c
+++ b/arch/m68k/emu/nfcon.c
@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
static struct tty_driver *nfcon_device(struct console *con, int *index)
{
*index = 0;
- return console_is_registered(con) ? nfcon_tty_driver : NULL;
+ return nfcon_tty_driver;
}
static struct console nf_console = {
--
2.55.0
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device
2026-08-15 7:24 [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device Andreas Schwab
@ 2026-08-17 9:26 ` Geert Uytterhoeven
2026-08-20 10:16 ` John Ogness
2026-08-20 12:57 ` Petr Mladek
0 siblings, 2 replies; 6+ messages in thread
From: Geert Uytterhoeven @ 2026-08-17 9:26 UTC (permalink / raw)
To: Andreas Schwab
Cc: linux-m68k, John Ogness, Petr Mladek, open list:SERIAL DRIVERS
Hi Andreas,
CC john, petr, serial
Thanks for your patch!
On Sat, 15 Aug 2026 at 09:33, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list
> synchronization") show_cons_active calls the device method under the
> console_list_lock, but console_is_registered tries to acquire
> console_list_lock as well, causing a deadlock. It should not be necessary
> to check console_is_registered here since the function should not be
> called in the fist place when the console is not registered.
Nice catch!
> Fixes: 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")
> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
i.e. will queue in the m68k tree for v7.3.
> --- a/arch/m68k/emu/nfcon.c
> +++ b/arch/m68k/emu/nfcon.c
> @@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
> static struct tty_driver *nfcon_device(struct console *con, int *index)
> {
> *index = 0;
> - return console_is_registered(con) ? nfcon_tty_driver : NULL;
Interestingly, that call to console_is_registered() was added in
commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"),
i.e. in the same series that added the extra locking...
> + return nfcon_tty_driver;
> }
>
> static struct console nf_console = {
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device
2026-08-17 9:26 ` Geert Uytterhoeven
@ 2026-08-20 10:16 ` John Ogness
2026-08-20 12:57 ` Petr Mladek
1 sibling, 0 replies; 6+ messages in thread
From: John Ogness @ 2026-08-20 10:16 UTC (permalink / raw)
To: Geert Uytterhoeven, Andreas Schwab
Cc: linux-m68k, Petr Mladek, open list:SERIAL DRIVERS
On 2026-08-17, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Sat, 15 Aug 2026 at 09:33, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list
>> synchronization") show_cons_active calls the device method under the
>> console_list_lock, but console_is_registered tries to acquire
>> console_list_lock as well, causing a deadlock. It should not be necessary
>> to check console_is_registered here since the function should not be
>> called in the fist place when the console is not registered.
>
> Nice catch!
>
>> Fixes: 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")
>> Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
FWIW:
Reviewed-by: John Ogness <john.ogness@linutronix.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device
2026-08-17 9:26 ` Geert Uytterhoeven
2026-08-20 10:16 ` John Ogness
@ 2026-08-20 12:57 ` Petr Mladek
2026-08-20 13:38 ` John Ogness
1 sibling, 1 reply; 6+ messages in thread
From: Petr Mladek @ 2026-08-20 12:57 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Andreas Schwab, linux-m68k, John Ogness, open list:SERIAL DRIVERS
On Mon 2026-08-17 11:26:57, Geert Uytterhoeven wrote:
> Hi Andreas,
>
> CC john, petr, serial
>
> Thanks for your patch!
>
> On Sat, 15 Aug 2026 at 09:33, Andreas Schwab <schwab@linux-m68k.org> wrote:
> > Since 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list
> > synchronization") show_cons_active calls the device method under the
> > console_list_lock, but console_is_registered tries to acquire
> > console_list_lock as well, causing a deadlock. It should not be necessary
> > to check console_is_registered here since the function should not be
> > called in the fist place when the console is not registered.
>
> Nice catch!
+1
> > Fixes: 7c2af0f634f1 ("tty: tty_io: use console_list_lock for list synchronization")
> > Signed-off-by: Andreas Schwab <schwab@linux-m68k.org>
>
> Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org>
> i.e. will queue in the m68k tree for v7.3.
>
> > --- a/arch/m68k/emu/nfcon.c
> > +++ b/arch/m68k/emu/nfcon.c
> > @@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
> > static struct tty_driver *nfcon_device(struct console *con, int *index)
> > {
> > *index = 0;
> > - return console_is_registered(con) ? nfcon_tty_driver : NULL;
>
> Interestingly, that call to console_is_registered() was added in
> commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"),
> i.e. in the same series that added the extra locking...
Sigh, we clearly did not check the callers of this function properly
at that time :/
> > + return nfcon_tty_driver;
Honestly, I do not feel comfortable with removing the check
completely.
The commit de61a1a3a083071 ("tty: nfcon: use console_is_registered()"
used console_is_registered() instead of (con->flags & CON_ENABLED).
And CON_ENABLED is historically cleared when the console is suspended,
see console_suspend().
Is is OK to return a valid struct tty_driver for a non-registered
or suspended console for all con->device() callers?
I have found only 4 callers:
$> git grep "\->device("
drivers/tty/serial/kgdboc.c: if (cons->device && cons->device(cons, &idx) == p &&
drivers/tty/tty_io.c: struct tty_driver *drv = cs[i]->device(cs[i], &index);
fs/proc/consoles.c: driver = con->device(con, &index);
kernel/printk/printk.c: driver = c->device(c, index);
These are:
+ configure_kgdboc() in drivers/tty/serial/kgdboc.c
+ show_cons_active() in drivers/tty/tty_io.c
+ show_console_dev() in fs/proc/consoles.c
+ console_device() in kernel/printk/printk.c
, where
+ configure_kgdboc() is used during the system initialization
when the console should not be suspended
+ show_cons_active() and show_console_dev() are questionable.
Maybe, we should not advertise the device to the userspace
when it is suspended.
+ console_device() is used in tty_kopen() -> tty_lookup_driver().
Also here we likely should not allow to open a suspended device.
Let's take a step back:
We should define the locking context for con->device() callback.
It seems that it is called under console_lock() and
console_srcu_read_lock() in:
+ configure_kgdboc()
+ console_device()
and under console_list_lock() and console_lock() in
+ show_console_dev()
+ show_cons_active()
I would put aside the console_lock(). We are trying to obsolete it.
So it is down to
console_srcu_read_lock() vs console_list_lock()
Both of them look reasonable. So, I think about adding a variant
of console_is_registered_srcu_read_locked() which would work
with both locking. Something like:
diff --git a/arch/m68k/emu/nfcon.c b/arch/m68k/emu/nfcon.c
index d41260672e24..d211b6c4da26 100644
--- a/arch/m68k/emu/nfcon.c
+++ b/arch/m68k/emu/nfcon.c
@@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
static struct tty_driver *nfcon_device(struct console *con, int *index)
{
*index = 0;
- return console_is_registered(con) ? nfcon_tty_driver : NULL;
+ return console_is_registered_srcu_read_locked(con) ? nfcon_tty_driver : NULL;
}
static struct console nf_console = {
diff --git a/include/linux/console.h b/include/linux/console.h
index d624200cfc17..b90527db4eca 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -311,7 +311,7 @@ struct nbcon_write_context {
* @name: The name of the console driver
* @write: Legacy write callback to output messages (Optional)
* @read: Read callback for console input (Optional)
- * @device: The underlying TTY device driver (Optional)
+ * @device: The underlying TTY device driver (Optional) [1]
* @unblank: Callback to unblank the console (Optional)
* @setup: Callback for initializing the console (Optional)
* @exit: Callback for teardown of the console (Optional)
@@ -334,6 +334,9 @@ struct nbcon_write_context {
* @kthread: Printer kthread for this console
* @rcuwait: RCU-safe wait object for @kthread waking
* @irq_work: Defer @kthread waking to IRQ work context
+ *
+ * [1] The @device callback must be called under either console_list_lock()
+ * or console_srcu_read_lock().
*/
struct console {
char name[16];
@@ -473,8 +476,13 @@ struct console {
};
#ifdef CONFIG_LOCKDEP
+extern bool console_list_lock_is_held(void);
extern void lockdep_assert_console_list_lock_held(void);
#else
+static inline bool console_list_lock_is_held(void)
+{
+ return 1;
+}
static inline void lockdep_assert_console_list_lock_held(void)
{
}
@@ -555,6 +563,16 @@ static inline bool console_is_registered_locked(const struct console *con)
return !hlist_unhashed(&con->node);
}
+/*
+ * Variant of console_is_registered() when either the console_srcu_read_lock
+ * or console_list_lock is held.
+*/
+static inline bool console_is_registered_srcu_read_locked(const struct console *con)
+{
+ lockdep_assert(console_srcu_read_lock_is_held() || console_list_lock_is_held());
+ return !hlist_unhashed(&con->node);
+}
+
/*
* console_is_registered - Check if the console is registered
* @con: struct console pointer of console to check
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2fe9a963c823..9b632b91f096 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -109,6 +109,12 @@ static struct lockdep_map console_lock_dep_map = {
.name = "console_lock"
};
+bool console_list_lock_is_held(void)
+{
+ return lockdep_is_held(&console_mutex);
+}
+EXPORT_SYMBOL(console_list_lock_is_held);
+
void lockdep_assert_console_list_lock_held(void)
{
lockdep_assert_held(&console_mutex);
Alternative solution would be to require the srcu locking.
IMHO, it should be perfectly fine to use the srcu_read_lock in
both show_console_dev() and show_cons_active().
What do you think, please?
John?
Best Regards,
Petr
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device
2026-08-20 12:57 ` Petr Mladek
@ 2026-08-20 13:38 ` John Ogness
2026-08-20 15:57 ` Petr Mladek
0 siblings, 1 reply; 6+ messages in thread
From: John Ogness @ 2026-08-20 13:38 UTC (permalink / raw)
To: Petr Mladek, Geert Uytterhoeven
Cc: Andreas Schwab, linux-m68k, open list:SERIAL DRIVERS
On 2026-08-20, Petr Mladek <pmladek@suse.com> wrote:
>> > --- a/arch/m68k/emu/nfcon.c
>> > +++ b/arch/m68k/emu/nfcon.c
>> > @@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
>> > static struct tty_driver *nfcon_device(struct console *con, int *index)
>> > {
>> > *index = 0;
>> > - return console_is_registered(con) ? nfcon_tty_driver : NULL;
>>
>> Interestingly, that call to console_is_registered() was added in
>> commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"),
>> i.e. in the same series that added the extra locking...
>
> Sigh, we clearly did not check the callers of this function properly
> at that time :/
>
>> > + return nfcon_tty_driver;
>
> Honestly, I do not feel comfortable with removing the check
> completely.
>
> The commit de61a1a3a083071 ("tty: nfcon: use console_is_registered()"
> used console_is_registered() instead of (con->flags & CON_ENABLED).
> And CON_ENABLED is historically cleared when the console is suspended,
> see console_suspend().
Since the NatFeat console driver never implemented suspend/resume
callbacks, I assume it was intended to identify if the console was
registered.
> Is is OK to return a valid struct tty_driver for a non-registered
> or suspended console for all con->device() callers?
>
> I have found only 4 callers:
>
> $> git grep "\->device("
> drivers/tty/serial/kgdboc.c: if (cons->device && cons->device(cons, &idx) == p &&
> drivers/tty/tty_io.c: struct tty_driver *drv = cs[i]->device(cs[i], &index);
> fs/proc/consoles.c: driver = con->device(con, &index);
> kernel/printk/printk.c: driver = c->device(c, index);
>
> These are:
>
> + configure_kgdboc() in drivers/tty/serial/kgdboc.c
> + show_cons_active() in drivers/tty/tty_io.c
> + show_console_dev() in fs/proc/consoles.c
> + console_device() in kernel/printk/printk.c
>
> , where
>
> + configure_kgdboc() is used during the system initialization
> when the console should not be suspended
>
> + show_cons_active() and show_console_dev() are questionable.
> Maybe, we should not advertise the device to the userspace
> when it is suspended.
>
> + console_device() is used in tty_kopen() -> tty_lookup_driver().
> Also here we likely should not allow to open a suspended device.
All of these callbacks are done when the console is guaranteed to be
registered (or not yet fully unregistered).
> Let's take a step back:
>
> We should define the locking context for con->device() callback.
> It seems that it is called under console_lock() and
> console_srcu_read_lock() in:
>
> + configure_kgdboc()
> + console_device()
>
> and under console_list_lock() and console_lock() in
>
> + show_console_dev()
> + show_cons_active()
>
> I would put aside the console_lock(). We are trying to obsolete it.
> So it is down to
>
> console_srcu_read_lock() vs console_list_lock()
>
> Both of them look reasonable. So, I think about adding a variant
> of console_is_registered_srcu_read_locked() which would work
> with both locking. Something like:
>
> diff --git a/arch/m68k/emu/nfcon.c b/arch/m68k/emu/nfcon.c
> index d41260672e24..d211b6c4da26 100644
> --- a/arch/m68k/emu/nfcon.c
> +++ b/arch/m68k/emu/nfcon.c
> @@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
> static struct tty_driver *nfcon_device(struct console *con, int *index)
> {
> *index = 0;
> - return console_is_registered(con) ? nfcon_tty_driver : NULL;
> + return console_is_registered_srcu_read_locked(con) ? nfcon_tty_driver : NULL;
> }
>
> static struct console nf_console = {
> diff --git a/include/linux/console.h b/include/linux/console.h
> index d624200cfc17..b90527db4eca 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -311,7 +311,7 @@ struct nbcon_write_context {
> * @name: The name of the console driver
> * @write: Legacy write callback to output messages (Optional)
> * @read: Read callback for console input (Optional)
> - * @device: The underlying TTY device driver (Optional)
> + * @device: The underlying TTY device driver (Optional) [1]
> * @unblank: Callback to unblank the console (Optional)
> * @setup: Callback for initializing the console (Optional)
> * @exit: Callback for teardown of the console (Optional)
> @@ -334,6 +334,9 @@ struct nbcon_write_context {
> * @kthread: Printer kthread for this console
> * @rcuwait: RCU-safe wait object for @kthread waking
> * @irq_work: Defer @kthread waking to IRQ work context
> + *
> + * [1] The @device callback must be called under either console_list_lock()
> + * or console_srcu_read_lock().
> */
> struct console {
> char name[16];
> @@ -473,8 +476,13 @@ struct console {
> };
>
> #ifdef CONFIG_LOCKDEP
> +extern bool console_list_lock_is_held(void);
> extern void lockdep_assert_console_list_lock_held(void);
> #else
> +static inline bool console_list_lock_is_held(void)
> +{
> + return 1;
> +}
> static inline void lockdep_assert_console_list_lock_held(void)
> {
> }
> @@ -555,6 +563,16 @@ static inline bool console_is_registered_locked(const struct console *con)
> return !hlist_unhashed(&con->node);
> }
>
> +/*
> + * Variant of console_is_registered() when either the console_srcu_read_lock
> + * or console_list_lock is held.
> +*/
> +static inline bool console_is_registered_srcu_read_locked(const struct console *con)
> +{
> + lockdep_assert(console_srcu_read_lock_is_held() || console_list_lock_is_held());
> + return !hlist_unhashed(&con->node);
> +}
> +
> /*
> * console_is_registered - Check if the console is registered
> * @con: struct console pointer of console to check
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2fe9a963c823..9b632b91f096 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -109,6 +109,12 @@ static struct lockdep_map console_lock_dep_map = {
> .name = "console_lock"
> };
>
> +bool console_list_lock_is_held(void)
> +{
> + return lockdep_is_held(&console_mutex);
> +}
> +EXPORT_SYMBOL(console_list_lock_is_held);
> +
> void lockdep_assert_console_list_lock_held(void)
> {
> lockdep_assert_held(&console_mutex);
>
>
> Alternative solution would be to require the srcu locking.
> IMHO, it should be perfectly fine to use the srcu_read_lock in
> both show_console_dev() and show_cons_active().
>
> What do you think, please?
> John?
I do not think any of that is necessary. I think it is enough to allow
the ->device() callback to assume it is registered. I see no reason why
the _callback_ should ever need to take the console_list_lock or the
console_srcu_read_lock.
John
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device
2026-08-20 13:38 ` John Ogness
@ 2026-08-20 15:57 ` Petr Mladek
0 siblings, 0 replies; 6+ messages in thread
From: Petr Mladek @ 2026-08-20 15:57 UTC (permalink / raw)
To: John Ogness
Cc: Geert Uytterhoeven, Andreas Schwab, linux-m68k,
open list:SERIAL DRIVERS
On Thu 2026-08-20 15:44:34, John Ogness wrote:
> On 2026-08-20, Petr Mladek <pmladek@suse.com> wrote:
> >> > --- a/arch/m68k/emu/nfcon.c
> >> > +++ b/arch/m68k/emu/nfcon.c
> >> > @@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
> >> > static struct tty_driver *nfcon_device(struct console *con, int *index)
> >> > {
> >> > *index = 0;
> >> > - return console_is_registered(con) ? nfcon_tty_driver : NULL;
> >>
> >> Interestingly, that call to console_is_registered() was added in
> >> commit de61a1a3a0830710 ("tty: nfcon: use console_is_registered()"),
> >> i.e. in the same series that added the extra locking...
> >
> > Sigh, we clearly did not check the callers of this function properly
> > at that time :/
> >
> >> > + return nfcon_tty_driver;
> >
> > Honestly, I do not feel comfortable with removing the check
> > completely.
> >
> > The commit de61a1a3a083071 ("tty: nfcon: use console_is_registered()"
> > used console_is_registered() instead of (con->flags & CON_ENABLED).
> > And CON_ENABLED is historically cleared when the console is suspended,
> > see console_suspend().
>
> Since the NatFeat console driver never implemented suspend/resume
> callbacks, I assume it was intended to identify if the console was
> registered.
Good point!
> > Is is OK to return a valid struct tty_driver for a non-registered
> > or suspended console for all con->device() callers?
> >
> > I have found only 4 callers:
> >
> > $> git grep "\->device("
> > drivers/tty/serial/kgdboc.c: if (cons->device && cons->device(cons, &idx) == p &&
> > drivers/tty/tty_io.c: struct tty_driver *drv = cs[i]->device(cs[i], &index);
> > fs/proc/consoles.c: driver = con->device(con, &index);
> > kernel/printk/printk.c: driver = c->device(c, index);
> >
> > These are:
> >
> > + configure_kgdboc() in drivers/tty/serial/kgdboc.c
> > + show_cons_active() in drivers/tty/tty_io.c
> > + show_console_dev() in fs/proc/consoles.c
> > + console_device() in kernel/printk/printk.c
> >
> > , where
> >
> > + configure_kgdboc() is used during the system initialization
> > when the console should not be suspended
> >
> > + show_cons_active() and show_console_dev() are questionable.
> > Maybe, we should not advertise the device to the userspace
> > when it is suspended.
> >
> > + console_device() is used in tty_kopen() -> tty_lookup_driver().
> > Also here we likely should not allow to open a suspended device.
>
> All of these callbacks are done when the console is guaranteed to be
> registered (or not yet fully unregistered).
Ditto.
> > Let's take a step back:
> >
> > We should define the locking context for con->device() callback.
> > It seems that it is called under console_lock() and
> > console_srcu_read_lock() in:
> >
> > + configure_kgdboc()
> > + console_device()
> >
> > and under console_list_lock() and console_lock() in
> >
> > + show_console_dev()
> > + show_cons_active()
> >
> > I would put aside the console_lock(). We are trying to obsolete it.
> > So it is down to
> >
> > console_srcu_read_lock() vs console_list_lock()
> >
> > Both of them look reasonable. So, I think about adding a variant
> > of console_is_registered_srcu_read_locked() which would work
> > with both locking. Something like:
> >
> > diff --git a/arch/m68k/emu/nfcon.c b/arch/m68k/emu/nfcon.c
> > index d41260672e24..d211b6c4da26 100644
> > --- a/arch/m68k/emu/nfcon.c
> > +++ b/arch/m68k/emu/nfcon.c
> > @@ -49,7 +49,7 @@ static void nfcon_write(struct console *con, const char *str,
> > static struct tty_driver *nfcon_device(struct console *con, int *index)
> > {
> > *index = 0;
> > - return console_is_registered(con) ? nfcon_tty_driver : NULL;
> > + return console_is_registered_srcu_read_locked(con) ? nfcon_tty_driver : NULL;
> > }
> >
> > static struct console nf_console = {
> > diff --git a/include/linux/console.h b/include/linux/console.h
> > index d624200cfc17..b90527db4eca 100644
> > --- a/include/linux/console.h
> > +++ b/include/linux/console.h
> > @@ -311,7 +311,7 @@ struct nbcon_write_context {
> > * @name: The name of the console driver
> > * @write: Legacy write callback to output messages (Optional)
> > * @read: Read callback for console input (Optional)
> > - * @device: The underlying TTY device driver (Optional)
> > + * @device: The underlying TTY device driver (Optional) [1]
> > * @unblank: Callback to unblank the console (Optional)
> > * @setup: Callback for initializing the console (Optional)
> > * @exit: Callback for teardown of the console (Optional)
> > @@ -334,6 +334,9 @@ struct nbcon_write_context {
> > * @kthread: Printer kthread for this console
> > * @rcuwait: RCU-safe wait object for @kthread waking
> > * @irq_work: Defer @kthread waking to IRQ work context
> > + *
> > + * [1] The @device callback must be called under either console_list_lock()
> > + * or console_srcu_read_lock().
> > */
> > struct console {
> > char name[16];
> > @@ -473,8 +476,13 @@ struct console {
> > };
> >
> > #ifdef CONFIG_LOCKDEP
> > +extern bool console_list_lock_is_held(void);
> > extern void lockdep_assert_console_list_lock_held(void);
> > #else
> > +static inline bool console_list_lock_is_held(void)
> > +{
> > + return 1;
> > +}
> > static inline void lockdep_assert_console_list_lock_held(void)
> > {
> > }
> > @@ -555,6 +563,16 @@ static inline bool console_is_registered_locked(const struct console *con)
> > return !hlist_unhashed(&con->node);
> > }
> >
> > +/*
> > + * Variant of console_is_registered() when either the console_srcu_read_lock
> > + * or console_list_lock is held.
> > +*/
> > +static inline bool console_is_registered_srcu_read_locked(const struct console *con)
> > +{
> > + lockdep_assert(console_srcu_read_lock_is_held() || console_list_lock_is_held());
> > + return !hlist_unhashed(&con->node);
> > +}
> > +
> > /*
> > * console_is_registered - Check if the console is registered
> > * @con: struct console pointer of console to check
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 2fe9a963c823..9b632b91f096 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -109,6 +109,12 @@ static struct lockdep_map console_lock_dep_map = {
> > .name = "console_lock"
> > };
> >
> > +bool console_list_lock_is_held(void)
> > +{
> > + return lockdep_is_held(&console_mutex);
> > +}
> > +EXPORT_SYMBOL(console_list_lock_is_held);
> > +
> > void lockdep_assert_console_list_lock_held(void)
> > {
> > lockdep_assert_held(&console_mutex);
> >
> >
> > Alternative solution would be to require the srcu locking.
> > IMHO, it should be perfectly fine to use the srcu_read_lock in
> > both show_console_dev() and show_cons_active().
> >
> > What do you think, please?
> > John?
>
> I do not think any of that is necessary. I think it is enough to allow
> the ->device() callback to assume it is registered. I see no reason why
> the _callback_ should ever need to take the console_list_lock or the
> console_srcu_read_lock.
Fair enough. I see that other con->device() callbacks just return
the pointer without checking anything.
So, the fix looks good after all.
Reviewed-by: Petr Mladek <pmladek@suse.com>
Best Regards,
Petr
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-20 15:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-15 7:24 [PATCH] m68k: nfcon: don't call console_is_registered in nfcon_device Andreas Schwab
2026-08-17 9:26 ` Geert Uytterhoeven
2026-08-20 10:16 ` John Ogness
2026-08-20 12:57 ` Petr Mladek
2026-08-20 13:38 ` John Ogness
2026-08-20 15:57 ` Petr Mladek
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.