* [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.