From 7b0592a23e4f27196f4ea207a926838e7651c5b5 Mon Sep 17 00:00:00 2001 From: Wang Honghui Date: Tue, 22 Nov 2022 16:50:36 +0800 Subject: printk: fix a typo of comment Fix a typo of comment Signed-off-by: Wang Honghui Reviewed-by: John Ogness Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/77522C532189E547+Y3yG91g6XALbtdJr@TP-P15V.lan Link: https://lore.kernel.org/r/0C7C980DB815FAE1+Y3yNXJCqZ3Nzxa5V@TP-P15V.lan --- kernel/printk/printk_ringbuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c index 2b7b6ddab4f7..2dc4d5a1f1ff 100644 --- a/kernel/printk/printk_ringbuffer.c +++ b/kernel/printk/printk_ringbuffer.c @@ -203,7 +203,7 @@ * prb_rec_init_wr(&r, 5); * * // try to extend, but only if it does not exceed 32 bytes - * if (prb_reserve_in_last(&e, &test_rb, &r, printk_caller_id()), 32) { + * if (prb_reserve_in_last(&e, &test_rb, &r, printk_caller_id(), 32)) { * snprintf(&r.text_buf[r.info->text_len], * r.text_buf_size - r.info->text_len, "hello"); * -- cgit v1.2.3 From 0cd7e350abc40eed5d3b60292dc102f700c88388 Mon Sep 17 00:00:00 2001 From: "Paul E. McKenney" Date: Tue, 22 Nov 2022 13:53:57 -0800 Subject: rcu: Make SRCU mandatory Kernels configured with CONFIG_PRINTK=n and CONFIG_SRCU=n get build failures. This causes trouble for deep embedded systems. But given that there are more than 25 instances of "select SRCU" in the kernel, it is hard to believe that there are many kernels running in production without SRCU. This commit therefore makes SRCU mandatory. The SRCU Kconfig option remains for backwards compatibility, and will be removed when it is no longer used. [ paulmck: Update per kernel test robot feedback. ] Reported-by: John Ogness Reported-by: Petr Mladek Signed-off-by: Paul E. McKenney Cc: Acked-by: Randy Dunlap # build-tested Reviewed-by: John Ogness --- include/linux/rcutiny.h | 4 ---- include/linux/srcu.h | 9 +-------- kernel/rcu/Kconfig | 11 +++-------- kernel/rcu/Kconfig.debug | 3 --- kernel/rcu/rcu.h | 8 ++++++-- kernel/rcu/update.c | 18 +++++++----------- 6 files changed, 17 insertions(+), 36 deletions(-) (limited to 'kernel') diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h index 768196a5f39d..d40b21ec7e0d 100644 --- a/include/linux/rcutiny.h +++ b/include/linux/rcutiny.h @@ -154,11 +154,7 @@ static inline bool rcu_preempt_need_deferred_qs(struct task_struct *t) return false; } static inline void rcu_preempt_deferred_qs(struct task_struct *t) { } -#ifdef CONFIG_SRCU void rcu_scheduler_starting(void); -#else /* #ifndef CONFIG_SRCU */ -static inline void rcu_scheduler_starting(void) { } -#endif /* #else #ifndef CONFIG_SRCU */ static inline void rcu_end_inkernel_boot(void) { } static inline bool rcu_inkernel_boot_has_ended(void) { return true; } static inline bool rcu_is_watching(void) { return true; } diff --git a/include/linux/srcu.h b/include/linux/srcu.h index f0814ffca34b..9b9d0bbf1d3c 100644 --- a/include/linux/srcu.h +++ b/include/linux/srcu.h @@ -47,11 +47,8 @@ int init_srcu_struct(struct srcu_struct *ssp); #include #elif defined(CONFIG_TREE_SRCU) #include -#elif defined(CONFIG_SRCU) -#error "Unknown SRCU implementation specified to kernel configuration" #else -/* Dummy definition for things like notifiers. Actual use gets link error. */ -struct srcu_struct { }; +#error "Unknown SRCU implementation specified to kernel configuration" #endif void call_srcu(struct srcu_struct *ssp, struct rcu_head *head, @@ -78,11 +75,7 @@ static inline void __srcu_read_unlock_nmisafe(struct srcu_struct *ssp, int idx) } #endif /* CONFIG_NEED_SRCU_NMI_SAFE */ -#ifdef CONFIG_SRCU void srcu_init(void); -#else /* #ifdef CONFIG_SRCU */ -static inline void srcu_init(void) { } -#endif /* #else #ifdef CONFIG_SRCU */ #ifdef CONFIG_DEBUG_LOCK_ALLOC diff --git a/kernel/rcu/Kconfig b/kernel/rcu/Kconfig index f53ad63b2bc6..1b0d79b2f5fa 100644 --- a/kernel/rcu/Kconfig +++ b/kernel/rcu/Kconfig @@ -54,21 +54,17 @@ config RCU_EXPERT Say N if you are unsure. config SRCU - bool - help - This option selects the sleepable version of RCU. This version - permits arbitrary sleeping or blocking within RCU read-side critical - sections. + def_bool y config TINY_SRCU bool - default y if SRCU && TINY_RCU + default y if TINY_RCU help This option selects the single-CPU non-preemptible version of SRCU. config TREE_SRCU bool - default y if SRCU && !TINY_RCU + default y if !TINY_RCU help This option selects the full-fledged version of SRCU. @@ -77,7 +73,6 @@ config NEED_SRCU_NMI_SAFE config TASKS_RCU_GENERIC def_bool TASKS_RCU || TASKS_RUDE_RCU || TASKS_TRACE_RCU - select SRCU help This option enables generic infrastructure code supporting task-based RCU implementations. Not for manual selection. diff --git a/kernel/rcu/Kconfig.debug b/kernel/rcu/Kconfig.debug index 1b0c41d490f0..232e29fe3e5e 100644 --- a/kernel/rcu/Kconfig.debug +++ b/kernel/rcu/Kconfig.debug @@ -27,7 +27,6 @@ config RCU_SCALE_TEST tristate "performance tests for RCU" depends on DEBUG_KERNEL select TORTURE_TEST - select SRCU default n help This option provides a kernel module that runs performance @@ -43,7 +42,6 @@ config RCU_TORTURE_TEST tristate "torture tests for RCU" depends on DEBUG_KERNEL select TORTURE_TEST - select SRCU default n help This option provides a kernel module that runs torture tests @@ -59,7 +57,6 @@ config RCU_REF_SCALE_TEST tristate "Scalability tests for read-side synchronization (RCU and others)" depends on DEBUG_KERNEL select TORTURE_TEST - select SRCU default n help This option provides a kernel module that runs performance tests diff --git a/kernel/rcu/rcu.h b/kernel/rcu/rcu.h index be5979da07f5..1f7ca484483a 100644 --- a/kernel/rcu/rcu.h +++ b/kernel/rcu/rcu.h @@ -286,7 +286,7 @@ void rcu_test_sync_prims(void); */ extern void resched_cpu(int cpu); -#if defined(CONFIG_SRCU) || !defined(CONFIG_TINY_RCU) +#if !defined(CONFIG_TINY_RCU) #include @@ -375,6 +375,10 @@ extern void rcu_init_geometry(void); (cpu) <= rnp->grphi; \ (cpu) = rcu_find_next_bit((rnp), (cpu) + 1 - (rnp->grplo), (mask))) +#endif /* !defined(CONFIG_TINY_RCU) */ + +#if !defined(CONFIG_TINY_RCU) || defined(CONFIG_TASKS_RCU_GENERIC) + /* * Wrappers for the rcu_node::lock acquire and release. * @@ -437,7 +441,7 @@ do { \ #define raw_lockdep_assert_held_rcu_node(p) \ lockdep_assert_held(&ACCESS_PRIVATE(p, lock)) -#endif /* #if defined(CONFIG_SRCU) || !defined(CONFIG_TINY_RCU) */ +#endif // #if !defined(CONFIG_TINY_RCU) || defined(CONFIG_TASKS_RCU_GENERIC) #ifdef CONFIG_TINY_RCU /* Tiny RCU doesn't expedite, as its purpose in life is instead to be tiny. */ diff --git a/kernel/rcu/update.c b/kernel/rcu/update.c index 738842c4886b..f5e6a2f95a2a 100644 --- a/kernel/rcu/update.c +++ b/kernel/rcu/update.c @@ -224,7 +224,7 @@ void rcu_test_sync_prims(void) synchronize_rcu_expedited(); } -#if !defined(CONFIG_TINY_RCU) || defined(CONFIG_SRCU) +#if !defined(CONFIG_TINY_RCU) /* * Switch to run-time mode once RCU has fully initialized. @@ -239,7 +239,7 @@ static int __init rcu_set_runtime_mode(void) } core_initcall(rcu_set_runtime_mode); -#endif /* #if !defined(CONFIG_TINY_RCU) || defined(CONFIG_SRCU) */ +#endif /* #if !defined(CONFIG_TINY_RCU) */ #ifdef CONFIG_DEBUG_LOCK_ALLOC static struct lock_class_key rcu_lock_key; @@ -559,10 +559,8 @@ static void early_boot_test_call_rcu(void) struct early_boot_kfree_rcu *rhp; call_rcu(&head, test_callback); - if (IS_ENABLED(CONFIG_SRCU)) { - early_srcu_cookie = start_poll_synchronize_srcu(&early_srcu); - call_srcu(&early_srcu, &shead, test_callback); - } + early_srcu_cookie = start_poll_synchronize_srcu(&early_srcu); + call_srcu(&early_srcu, &shead, test_callback); rhp = kmalloc(sizeof(*rhp), GFP_KERNEL); if (!WARN_ON_ONCE(!rhp)) kfree_rcu(rhp, rh); @@ -585,11 +583,9 @@ static int rcu_verify_early_boot_tests(void) if (rcu_self_test) { early_boot_test_counter++; rcu_barrier(); - if (IS_ENABLED(CONFIG_SRCU)) { - early_boot_test_counter++; - srcu_barrier(&early_srcu); - WARN_ON_ONCE(!poll_state_synchronize_srcu(&early_srcu, early_srcu_cookie)); - } + early_boot_test_counter++; + srcu_barrier(&early_srcu); + WARN_ON_ONCE(!poll_state_synchronize_srcu(&early_srcu, early_srcu_cookie)); } if (rcu_self_test_counter != early_boot_test_counter) { WARN_ON(1); -- cgit v1.2.3 From 7365df19e8ff7a031e1557616fc0b3aa6d794d7e Mon Sep 17 00:00:00 2001 From: Xu Panda Date: Wed, 30 Nov 2022 16:01:41 +0800 Subject: printk: use strscpy() to instead of strlcpy() The implementation of strscpy() is more robust and safer. That's now the recommended way to copy NUL terminated strings. Signed-off-by: Xu Panda Signed-off-by: Yang Yang Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/202211301601416229001@zte.com.cn --- kernel/printk/printk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index e4f1e7478b52..eec87ddcaa45 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2391,7 +2391,7 @@ static int __add_preferred_console(char *name, int idx, char *options, return -E2BIG; if (!brl_options) preferred_console = i; - strlcpy(c->name, name, sizeof(c->name)); + strscpy(c->name, name, sizeof(c->name)); c->options = options; set_user_specified(c, user_specified); braille_set_options(c, brl_options); -- cgit v1.2.3 From 51f5f78a4f804aeb73cf12a7cbba6e5198abd908 Mon Sep 17 00:00:00 2001 From: Zqiang Date: Wed, 9 Nov 2022 15:36:38 +0800 Subject: srcu: Make Tiny synchronize_srcu() check for readers This commit adds lockdep checks for illegal use of synchronize_srcu() within same-type SRCU read-side critical sections and within normal RCU read-side critical sections. It also makes synchronize_srcu() be a no-op during early boot. These changes bring Tiny synchronize_srcu() into line with both Tree synchronize_srcu() and Tiny synchronize_rcu(). Signed-off-by: Zqiang Signed-off-by: Paul E. McKenney Tested-by: John Ogness --- kernel/rcu/srcutiny.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'kernel') diff --git a/kernel/rcu/srcutiny.c b/kernel/rcu/srcutiny.c index 33adafdad261..b12fb0cec44d 100644 --- a/kernel/rcu/srcutiny.c +++ b/kernel/rcu/srcutiny.c @@ -197,6 +197,16 @@ void synchronize_srcu(struct srcu_struct *ssp) { struct rcu_synchronize rs; + RCU_LOCKDEP_WARN(lockdep_is_held(ssp) || + lock_is_held(&rcu_bh_lock_map) || + lock_is_held(&rcu_lock_map) || + lock_is_held(&rcu_sched_lock_map), + "Illegal synchronize_srcu() in same-type SRCU (or in RCU) read-side critical section"); + + if (rcu_scheduler_active == RCU_SCHEDULER_INACTIVE) + return; + + might_sleep(); init_rcu_head_on_stack(&rs.head); init_completion(&rs.completion); call_srcu(ssp, &rs.head, wakeme_after_rcu); -- cgit v1.2.3 From d9a4af5690e26afa8a2eb83c575d3a9ef52cde1d Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Wed, 16 Nov 2022 17:27:14 +0106 Subject: printk: Convert console_drivers list to hlist Replace the open coded single linked list with a hlist so a conversion to SRCU protected list walks can reuse the existing primitives. Co-developed-by: John Ogness Signed-off-by: John Ogness Signed-off-by: Thomas Gleixner Reviewed-by: Greg Kroah-Hartman Reviewed-by: Sergey Senozhatsky Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-3-john.ogness@linutronix.de --- fs/proc/consoles.c | 3 +- include/linux/console.h | 8 ++-- kernel/printk/printk.c | 101 ++++++++++++++++++++++++++---------------------- 3 files changed, 62 insertions(+), 50 deletions(-) (limited to 'kernel') diff --git a/fs/proc/consoles.c b/fs/proc/consoles.c index dfe6ce3505ce..cf2e0788f9c7 100644 --- a/fs/proc/consoles.c +++ b/fs/proc/consoles.c @@ -74,8 +74,9 @@ static void *c_start(struct seq_file *m, loff_t *pos) static void *c_next(struct seq_file *m, void *v, loff_t *pos) { struct console *con = v; + ++*pos; - return con->next; + return hlist_entry_safe(con->node.next, struct console, node); } static void c_stop(struct seq_file *m, void *v) diff --git a/include/linux/console.h b/include/linux/console.h index 8c1686e2c233..7b5f21f9e469 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -15,6 +15,7 @@ #define _LINUX_CONSOLE_H_ 1 #include +#include #include struct vc_data; @@ -154,14 +155,16 @@ struct console { u64 seq; unsigned long dropped; void *data; - struct console *next; + struct hlist_node node; }; +extern struct hlist_head console_list; + /* * for_each_console() allows you to iterate on each console */ #define for_each_console(con) \ - for (con = console_drivers; con != NULL; con = con->next) + hlist_for_each_entry(con, &console_list, node) extern int console_set_on_cmdline; extern struct console *early_console; @@ -174,7 +177,6 @@ enum con_flush_mode { extern int add_preferred_console(char *name, int idx, char *options); extern void register_console(struct console *); extern int unregister_console(struct console *); -extern struct console *console_drivers; extern void console_lock(void); extern int console_trylock(void); extern void console_unlock(void); diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index e4f1e7478b52..e6f0832e71f0 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -79,13 +79,12 @@ int oops_in_progress; EXPORT_SYMBOL(oops_in_progress); /* - * console_sem protects the console_drivers list, and also - * provides serialisation for access to the entire console - * driver system. + * console_sem protects console_list and console->flags updates, and also + * provides serialization for access to the entire console driver system. */ static DEFINE_SEMAPHORE(console_sem); -struct console *console_drivers; -EXPORT_SYMBOL_GPL(console_drivers); +HLIST_HEAD(console_list); +EXPORT_SYMBOL_GPL(console_list); /* * System may need to suppress printk message under certain @@ -2556,7 +2555,7 @@ static int console_cpu_notify(unsigned int cpu) * console_lock - lock the console system for exclusive use. * * Acquires a lock which guarantees that the caller has - * exclusive access to the console system and the console_drivers list. + * exclusive access to the console system and console_list. * * Can sleep, returns nothing. */ @@ -2576,7 +2575,7 @@ EXPORT_SYMBOL(console_lock); * console_trylock - try to lock the console system for exclusive use. * * Try to acquire a lock which guarantees that the caller has exclusive - * access to the console system and the console_drivers list. + * access to the console system and console_list. * * returns 1 on success, and 0 on failure to acquire the lock. */ @@ -2940,11 +2939,20 @@ void console_flush_on_panic(enum con_flush_mode mode) console_may_schedule = 0; if (mode == CONSOLE_REPLAY_ALL) { + struct hlist_node *tmp; struct console *c; u64 seq; seq = prb_first_valid_seq(prb); - for_each_console(c) + /* + * This cannot use for_each_console() because it's not established + * that the current context has console locked and neither there is + * a guarantee that there is no concurrency in that case. + * + * Open code it for documentation purposes and pretend that + * it works. + */ + hlist_for_each_entry_safe(c, tmp, &console_list, node) c->seq = seq; } console_unlock(); @@ -3081,6 +3089,9 @@ static void try_enable_default_console(struct console *newcon) (con->flags & CON_BOOT) ? "boot" : "", \ con->name, con->index, ##__VA_ARGS__) +#define console_first() \ + hlist_entry(console_list.first, struct console, node) + /* * The console driver calls this routine during kernel initialization * to register the console printing procedure with printk() and to @@ -3140,8 +3151,8 @@ void register_console(struct console *newcon) * flag set and will be first in the list. */ if (preferred_console < 0) { - if (!console_drivers || !console_drivers->device || - console_drivers->flags & CON_BOOT) { + if (hlist_empty(&console_list) || !console_first()->device || + console_first()->flags & CON_BOOT) { try_enable_default_console(newcon); } } @@ -3169,20 +3180,22 @@ void register_console(struct console *newcon) } /* - * Put this console in the list - keep the - * preferred driver at the head of the list. + * Put this console in the list - keep the + * preferred driver at the head of the list. */ console_lock(); - if ((newcon->flags & CON_CONSDEV) || console_drivers == NULL) { - newcon->next = console_drivers; - console_drivers = newcon; - if (newcon->next) - newcon->next->flags &= ~CON_CONSDEV; - /* Ensure this flag is always set for the head of the list */ + if (hlist_empty(&console_list)) { + /* Ensure CON_CONSDEV is always set for the head. */ newcon->flags |= CON_CONSDEV; + hlist_add_head(&newcon->node, &console_list); + + } else if (newcon->flags & CON_CONSDEV) { + /* Only the new head can have CON_CONSDEV set. */ + console_first()->flags &= ~CON_CONSDEV; + hlist_add_head(&newcon->node, &console_list); + } else { - newcon->next = console_drivers->next; - console_drivers->next = newcon; + hlist_add_behind(&newcon->node, console_list.first); } newcon->dropped = 0; @@ -3209,16 +3222,18 @@ void register_console(struct console *newcon) if (bootcon_enabled && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) && !keep_bootcon) { - for_each_console(con) + struct hlist_node *tmp; + + hlist_for_each_entry_safe(con, tmp, &console_list, node) { if (con->flags & CON_BOOT) unregister_console(con); + } } } EXPORT_SYMBOL(register_console); int unregister_console(struct console *console) { - struct console *con; int res; con_printk(KERN_INFO, console, "disabled\n"); @@ -3229,32 +3244,30 @@ int unregister_console(struct console *console) if (res > 0) return 0; - res = -ENODEV; console_lock(); - if (console_drivers == console) { - console_drivers=console->next; - res = 0; - } else { - for_each_console(con) { - if (con->next == console) { - con->next = console->next; - res = 0; - break; - } - } + + /* Disable it unconditionally */ + console->flags &= ~CON_ENABLED; + + if (hlist_unhashed(&console->node)) { + console_unlock(); + return -ENODEV; } - if (res) - goto out_disable_unlock; + hlist_del_init(&console->node); /* + * * If this isn't the last console and it has CON_CONSDEV set, we * need to set it on the next preferred console. + * + * + * The above makes no sense as there is no guarantee that the next + * console has any device attached. Oh well.... */ - if (console_drivers != NULL && console->flags & CON_CONSDEV) - console_drivers->flags |= CON_CONSDEV; + if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV) + console_first()->flags |= CON_CONSDEV; - console->flags &= ~CON_ENABLED; console_unlock(); console_sysfs_notify(); @@ -3262,12 +3275,6 @@ int unregister_console(struct console *console) res = console->exit(console); return res; - -out_disable_unlock: - console->flags &= ~CON_ENABLED; - console_unlock(); - - return res; } EXPORT_SYMBOL(unregister_console); @@ -3317,10 +3324,11 @@ void __init console_init(void) */ static int __init printk_late_init(void) { + struct hlist_node *tmp; struct console *con; int ret; - for_each_console(con) { + hlist_for_each_entry_safe(con, tmp, &console_list, node) { if (!(con->flags & CON_BOOT)) continue; @@ -3340,6 +3348,7 @@ static int __init printk_late_init(void) unregister_console(con); } } + ret = cpuhp_setup_state_nocalls(CPUHP_PRINTK_DEAD, "printk:dead", NULL, console_cpu_notify); WARN_ON(ret < 0); -- cgit v1.2.3 From 6c4afa79147e4aa86665795695ac4a5f25e73176 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:15 +0106 Subject: printk: Prepare for SRCU console list protection Provide an NMI-safe SRCU protected variant to walk the console list. Note that all console fields are now set before adding the console to the list to avoid the console becoming visible by SCRU readers before being fully initialized. This is a preparatory change for a new console infrastructure which operates independent of the console BKL. Suggested-by: Thomas Gleixner Signed-off-by: John Ogness Acked-by: Miguel Ojeda Reviewed-by: Paul E. McKenney Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-4-john.ogness@linutronix.de --- .clang-format | 1 + include/linux/console.h | 28 +++++++++++++++- kernel/printk/printk.c | 87 ++++++++++++++++++++++++++++++++++++++++--------- 3 files changed, 100 insertions(+), 16 deletions(-) (limited to 'kernel') diff --git a/.clang-format b/.clang-format index 1247d54f9e49..04a675b56b57 100644 --- a/.clang-format +++ b/.clang-format @@ -222,6 +222,7 @@ ForEachMacros: - 'for_each_component_dais' - 'for_each_component_dais_safe' - 'for_each_console' + - 'for_each_console_srcu' - 'for_each_cpu' - 'for_each_cpu_and' - 'for_each_cpu_not' diff --git a/include/linux/console.h b/include/linux/console.h index 7b5f21f9e469..f4f0c9523835 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -15,7 +15,7 @@ #define _LINUX_CONSOLE_H_ 1 #include -#include +#include #include struct vc_data; @@ -158,8 +158,34 @@ struct console { struct hlist_node node; }; +#ifdef CONFIG_DEBUG_LOCK_ALLOC +extern bool console_srcu_read_lock_is_held(void); +#else +static inline bool console_srcu_read_lock_is_held(void) +{ + return 1; +} +#endif + +extern int console_srcu_read_lock(void); +extern void console_srcu_read_unlock(int cookie); + extern struct hlist_head console_list; +/** + * for_each_console_srcu() - Iterator over registered consoles + * @con: struct console pointer used as loop cursor + * + * Although SRCU guarantees the console list will be consistent, the + * struct console fields may be updated by other CPUs while iterating. + * + * Requires console_srcu_read_lock to be held. Can be invoked from + * any context. + */ +#define for_each_console_srcu(con) \ + hlist_for_each_entry_srcu(con, &console_list, node, \ + console_srcu_read_lock_is_held()) + /* * for_each_console() allows you to iterate on each console */ diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index e6f0832e71f0..173f46a29252 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -85,6 +85,7 @@ EXPORT_SYMBOL(oops_in_progress); static DEFINE_SEMAPHORE(console_sem); HLIST_HEAD(console_list); EXPORT_SYMBOL_GPL(console_list); +DEFINE_STATIC_SRCU(console_srcu); /* * System may need to suppress printk message under certain @@ -104,6 +105,13 @@ static struct lockdep_map console_lock_dep_map = { }; #endif +#ifdef CONFIG_DEBUG_LOCK_ALLOC +bool console_srcu_read_lock_is_held(void) +{ + return srcu_read_lock_held(&console_srcu); +} +#endif + enum devkmsg_log_bits { __DEVKMSG_LOG_BIT_ON = 0, __DEVKMSG_LOG_BIT_OFF, @@ -219,6 +227,32 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, } #endif /* CONFIG_PRINTK && CONFIG_SYSCTL */ +/** + * console_srcu_read_lock - Register a new reader for the + * SRCU-protected console list + * + * Use for_each_console_srcu() to iterate the console list + * + * Context: Any context. + */ +int console_srcu_read_lock(void) +{ + return srcu_read_lock_nmisafe(&console_srcu); +} +EXPORT_SYMBOL(console_srcu_read_lock); + +/** + * console_srcu_read_unlock - Unregister an old reader from + * the SRCU-protected console list + * + * Counterpart to console_srcu_read_lock() + */ +void console_srcu_read_unlock(int cookie) +{ + srcu_read_unlock_nmisafe(&console_srcu, cookie); +} +EXPORT_SYMBOL(console_srcu_read_unlock); + /* * Helper macros to handle lockdep when locking/unlocking console_sem. We use * macros instead of functions so that _RET_IP_ contains useful information. @@ -2989,6 +3023,14 @@ void console_stop(struct console *console) console_lock(); console->flags &= ~CON_ENABLED; console_unlock(); + + /* + * Ensure that all SRCU list walks have completed. All contexts must + * be able to see that this console is disabled so that (for example) + * the caller can suspend the port without risk of another context + * using the port. + */ + synchronize_srcu(&console_srcu); } EXPORT_SYMBOL(console_stop); @@ -3179,6 +3221,17 @@ void register_console(struct console *newcon) newcon->flags &= ~CON_PRINTBUFFER; } + newcon->dropped = 0; + if (newcon->flags & CON_PRINTBUFFER) { + /* Get a consistent copy of @syslog_seq. */ + mutex_lock(&syslog_lock); + newcon->seq = syslog_seq; + mutex_unlock(&syslog_lock); + } else { + /* Begin with next message. */ + newcon->seq = prb_next_seq(prb); + } + /* * Put this console in the list - keep the * preferred driver at the head of the list. @@ -3187,28 +3240,24 @@ void register_console(struct console *newcon) if (hlist_empty(&console_list)) { /* Ensure CON_CONSDEV is always set for the head. */ newcon->flags |= CON_CONSDEV; - hlist_add_head(&newcon->node, &console_list); + hlist_add_head_rcu(&newcon->node, &console_list); } else if (newcon->flags & CON_CONSDEV) { /* Only the new head can have CON_CONSDEV set. */ console_first()->flags &= ~CON_CONSDEV; - hlist_add_head(&newcon->node, &console_list); + hlist_add_head_rcu(&newcon->node, &console_list); } else { - hlist_add_behind(&newcon->node, console_list.first); - } - - newcon->dropped = 0; - if (newcon->flags & CON_PRINTBUFFER) { - /* Get a consistent copy of @syslog_seq. */ - mutex_lock(&syslog_lock); - newcon->seq = syslog_seq; - mutex_unlock(&syslog_lock); - } else { - /* Begin with next message. */ - newcon->seq = prb_next_seq(prb); + hlist_add_behind_rcu(&newcon->node, console_list.first); } console_unlock(); + + /* + * No need to synchronize SRCU here! The caller does not rely + * on all contexts being able to see the new console before + * register_console() completes. + */ + console_sysfs_notify(); /* @@ -3254,7 +3303,7 @@ int unregister_console(struct console *console) return -ENODEV; } - hlist_del_init(&console->node); + hlist_del_init_rcu(&console->node); /* * @@ -3269,6 +3318,14 @@ int unregister_console(struct console *console) console_first()->flags |= CON_CONSDEV; console_unlock(); + + /* + * Ensure that all SRCU list walks have completed. All contexts + * must not be able to see this console in the list so that any + * exit/cleanup routines can be performed safely. + */ + synchronize_srcu(&console_srcu); + console_sysfs_notify(); if (console->exit) -- cgit v1.2.3 From 1145703612462b9bab2a325c305408ebfb6de304 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:16 +0106 Subject: printk: register_console: use "registered" for variable names The @bootcon_enabled and @realcon_enabled local variables actually represent if such console types are registered. In general there has been a confusion about enabled vs. registered. Incorrectly naming such variables promotes such confusion. Rename the variables to _registered. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-5-john.ogness@linutronix.de --- kernel/printk/printk.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 173f46a29252..3d449dfb1ed6 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3156,8 +3156,8 @@ static void try_enable_default_console(struct console *newcon) void register_console(struct console *newcon) { struct console *con; - bool bootcon_enabled = false; - bool realcon_enabled = false; + bool bootcon_registered = false; + bool realcon_registered = false; int err; for_each_console(con) { @@ -3168,13 +3168,13 @@ void register_console(struct console *newcon) for_each_console(con) { if (con->flags & CON_BOOT) - bootcon_enabled = true; + bootcon_registered = true; else - realcon_enabled = true; + realcon_registered = true; } /* Do not register boot consoles when there already is a real one. */ - if (newcon->flags & CON_BOOT && realcon_enabled) { + if ((newcon->flags & CON_BOOT) && realcon_registered) { pr_info("Too late to register bootconsole %s%d\n", newcon->name, newcon->index); return; @@ -3216,7 +3216,7 @@ void register_console(struct console *newcon) * the real console are the same physical device, it's annoying to * see the beginning boot messages twice */ - if (bootcon_enabled && + if (bootcon_registered && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV)) { newcon->flags &= ~CON_PRINTBUFFER; } @@ -3268,7 +3268,7 @@ void register_console(struct console *newcon) * went to the bootconsole (that they do not see on the real console) */ con_printk(KERN_INFO, newcon, "enabled\n"); - if (bootcon_enabled && + if (bootcon_registered && ((newcon->flags & (CON_CONSDEV | CON_BOOT)) == CON_CONSDEV) && !keep_bootcon) { struct hlist_node *tmp; -- cgit v1.2.3 From b80ea0e81b3954e4c519e0c257b095e77cf94d86 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:17 +0106 Subject: printk: move @seq initialization to helper The code to initialize @seq for a new console needs to consider more factors when choosing an initial value. Move the code into a helper function console_init_seq() "as is" so this code can be expanded without causing register_console() to become too long. A later commit will implement the additional code. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-6-john.ogness@linutronix.de --- kernel/printk/printk.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 3d449dfb1ed6..31d9d1cf8682 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3131,6 +3131,19 @@ static void try_enable_default_console(struct console *newcon) (con->flags & CON_BOOT) ? "boot" : "", \ con->name, con->index, ##__VA_ARGS__) +static void console_init_seq(struct console *newcon) +{ + if (newcon->flags & CON_PRINTBUFFER) { + /* Get a consistent copy of @syslog_seq. */ + mutex_lock(&syslog_lock); + newcon->seq = syslog_seq; + mutex_unlock(&syslog_lock); + } else { + /* Begin with next message. */ + newcon->seq = prb_next_seq(prb); + } +} + #define console_first() \ hlist_entry(console_list.first, struct console, node) @@ -3222,15 +3235,7 @@ void register_console(struct console *newcon) } newcon->dropped = 0; - if (newcon->flags & CON_PRINTBUFFER) { - /* Get a consistent copy of @syslog_seq. */ - mutex_lock(&syslog_lock); - newcon->seq = syslog_seq; - mutex_unlock(&syslog_lock); - } else { - /* Begin with next message. */ - newcon->seq = prb_next_seq(prb); - } + console_init_seq(newcon); /* * Put this console in the list - keep the -- cgit v1.2.3 From a42427609368d773c4ab6b4c9686577817a4d219 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:18 +0106 Subject: printk: fix setting first seq for consoles It used to be that all consoles were synchronized with respect to which message they were printing. After commit a699449bb13b ("printk: refactor and rework printing logic"), all consoles have their own @seq for tracking which message they are on. That commit also changed how the initial sequence number was chosen. Instead of choosing the next non-printed message, it chose the sequence number of the next message that will be added to the ringbuffer. That change created a possibility that a non-boot console taking over for a boot console might skip messages if the boot console was behind and did not have a chance to catch up before being unregistered. Since it is not known which boot console is the same device, flush all consoles and, if necessary, start with the message of the enabled boot console that is the furthest behind. If no boot consoles are enabled, begin with the next message that will be added to the ringbuffer. Also, since boot consoles are meant to be used at boot time, handle them the same as CON_PRINTBUFFER to ensure that no initial messages are skipped. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-7-john.ogness@linutronix.de --- kernel/printk/printk.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 31d9d1cf8682..c84654444a02 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3131,16 +3131,56 @@ static void try_enable_default_console(struct console *newcon) (con->flags & CON_BOOT) ? "boot" : "", \ con->name, con->index, ##__VA_ARGS__) -static void console_init_seq(struct console *newcon) +static void console_init_seq(struct console *newcon, bool bootcon_registered) { - if (newcon->flags & CON_PRINTBUFFER) { + struct console *con; + bool handover; + + if (newcon->flags & (CON_PRINTBUFFER | CON_BOOT)) { /* Get a consistent copy of @syslog_seq. */ mutex_lock(&syslog_lock); newcon->seq = syslog_seq; mutex_unlock(&syslog_lock); } else { - /* Begin with next message. */ + /* Begin with next message added to ringbuffer. */ newcon->seq = prb_next_seq(prb); + + /* + * If any enabled boot consoles are due to be unregistered + * shortly, some may not be caught up and may be the same + * device as @newcon. Since it is not known which boot console + * is the same device, flush all consoles and, if necessary, + * start with the message of the enabled boot console that is + * the furthest behind. + */ + if (bootcon_registered && !keep_bootcon) { + /* + * Flush all consoles and set the console to start at + * the next unprinted sequence number. + */ + if (!console_flush_all(true, &newcon->seq, &handover)) { + /* + * Flushing failed. Just choose the lowest + * sequence of the enabled boot consoles. + */ + + /* + * If there was a handover, this context no + * longer holds the console_lock. + */ + if (handover) + console_lock(); + + newcon->seq = prb_next_seq(prb); + for_each_console(con) { + if ((con->flags & CON_BOOT) && + (con->flags & CON_ENABLED) && + con->seq < newcon->seq) { + newcon->seq = con->seq; + } + } + } + } } } @@ -3235,13 +3275,13 @@ void register_console(struct console *newcon) } newcon->dropped = 0; - console_init_seq(newcon); + console_lock(); + console_init_seq(newcon, bootcon_registered); /* * Put this console in the list - keep the * preferred driver at the head of the list. */ - console_lock(); if (hlist_empty(&console_list)) { /* Ensure CON_CONSDEV is always set for the head. */ newcon->flags |= CON_CONSDEV; -- cgit v1.2.3 From 4dc64682ad37abb02c54ca598430ac27de60c21a Mon Sep 17 00:00:00 2001 From: John Ogness Date: Mon, 21 Nov 2022 12:16:12 +0106 Subject: printk: introduce console_list_lock Currently there exist races in register_console(), where the types of registered consoles are checked (without holding the console_lock) and then after acquiring the console_lock, it is assumed that the list has not changed. Also, some code that performs console_unregister() make similar assumptions. It might be possible to fix these races using the console_lock. But it would require a complex analysis of all console drivers to make sure that the console_lock is not taken in match() and setup() callbacks. And we really prefer to split up and reduce the responsibilities of console_lock rather than expand its complexity. Therefore, introduce a new console_list_lock to provide full synchronization for any console list changes. In addition, also use console_list_lock for synchronization of console->flags updates. All flags are either static or modified only during the console registration. There are only two exceptions. The first exception is CON_ENABLED, which is also modified by console_start()/console_stop(). Therefore, these functions must also take the console_list_lock. The second exception is when the flags are modified by the console driver init code before the console is registered. These will be ignored because they are not visible to the rest of the system via the console_drivers list. Note that one of the various responsibilities of the console_lock is also intended to provide console list and console->flags synchronization. Later changes will update call sites relying on the console_lock for these purposes. Once all call sites have been updated, the console_lock will be relieved of synchronizing console_list and console->flags updates. Suggested-by: Thomas Gleixner Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/87sficwokr.fsf@jogness.linutronix.de --- include/linux/console.h | 23 +++++++++++-- kernel/printk/printk.c | 89 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 100 insertions(+), 12 deletions(-) (limited to 'kernel') diff --git a/include/linux/console.h b/include/linux/console.h index f4f0c9523835..24d83e24840b 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -158,6 +158,14 @@ struct console { struct hlist_node node; }; +#ifdef CONFIG_LOCKDEP +extern void lockdep_assert_console_list_lock_held(void); +#else +static inline void lockdep_assert_console_list_lock_held(void) +{ +} +#endif + #ifdef CONFIG_DEBUG_LOCK_ALLOC extern bool console_srcu_read_lock_is_held(void); #else @@ -170,6 +178,9 @@ static inline bool console_srcu_read_lock_is_held(void) extern int console_srcu_read_lock(void); extern void console_srcu_read_unlock(int cookie); +extern void console_list_lock(void) __acquires(console_mutex); +extern void console_list_unlock(void) __releases(console_mutex); + extern struct hlist_head console_list; /** @@ -186,10 +197,16 @@ extern struct hlist_head console_list; hlist_for_each_entry_srcu(con, &console_list, node, \ console_srcu_read_lock_is_held()) -/* - * for_each_console() allows you to iterate on each console +/** + * for_each_console() - Iterator over registered consoles + * @con: struct console pointer used as loop cursor + * + * The console list and the console->flags are immutable while iterating. + * + * Requires console_list_lock to be held. */ -#define for_each_console(con) \ +#define for_each_console(con) \ + lockdep_assert_console_list_lock_held(); \ hlist_for_each_entry(con, &console_list, node) extern int console_set_on_cmdline; diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index c84654444a02..2b4506673a86 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -78,6 +78,13 @@ EXPORT_SYMBOL(ignore_console_lock_warning); int oops_in_progress; EXPORT_SYMBOL(oops_in_progress); +/* + * console_mutex protects console_list updates and console->flags updates. + * The flags are synchronized only for consoles that are registered, i.e. + * accessible via the console list. + */ +static DEFINE_MUTEX(console_mutex); + /* * console_sem protects console_list and console->flags updates, and also * provides serialization for access to the entire console driver system. @@ -103,6 +110,12 @@ static int __read_mostly suppress_panic_printk; static struct lockdep_map console_lock_dep_map = { .name = "console_lock" }; + +void lockdep_assert_console_list_lock_held(void) +{ + lockdep_assert_held(&console_mutex); +} +EXPORT_SYMBOL(lockdep_assert_console_list_lock_held); #endif #ifdef CONFIG_DEBUG_LOCK_ALLOC @@ -227,6 +240,40 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, } #endif /* CONFIG_PRINTK && CONFIG_SYSCTL */ +/** + * console_list_lock - Lock the console list + * + * For console list or console->flags updates + */ +void console_list_lock(void) +{ + /* + * In unregister_console(), synchronize_srcu() is called with the + * console_list_lock held. Therefore it is not allowed that the + * console_list_lock is taken with the srcu_lock held. + * + * Detecting if this context is really in the read-side critical + * section is only possible if the appropriate debug options are + * enabled. + */ + WARN_ON_ONCE(debug_lockdep_rcu_enabled() && + srcu_read_lock_held(&console_srcu)); + + mutex_lock(&console_mutex); +} +EXPORT_SYMBOL(console_list_lock); + +/** + * console_list_unlock - Unlock the console list + * + * Counterpart to console_list_lock() + */ +void console_list_unlock(void) +{ + mutex_unlock(&console_mutex); +} +EXPORT_SYMBOL(console_list_unlock); + /** * console_srcu_read_lock - Register a new reader for the * SRCU-protected console list @@ -3020,9 +3067,11 @@ struct tty_driver *console_device(int *index) void console_stop(struct console *console) { __pr_flush(console, 1000, true); + console_list_lock(); console_lock(); console->flags &= ~CON_ENABLED; console_unlock(); + console_list_unlock(); /* * Ensure that all SRCU list walks have completed. All contexts must @@ -3036,9 +3085,11 @@ EXPORT_SYMBOL(console_stop); void console_start(struct console *console) { + console_list_lock(); console_lock(); console->flags |= CON_ENABLED; console_unlock(); + console_list_unlock(); __pr_flush(console, 1000, true); } EXPORT_SYMBOL(console_start); @@ -3187,6 +3238,8 @@ static void console_init_seq(struct console *newcon, bool bootcon_registered) #define console_first() \ hlist_entry(console_list.first, struct console, node) +static int unregister_console_locked(struct console *console); + /* * The console driver calls this routine during kernel initialization * to register the console printing procedure with printk() and to @@ -3213,13 +3266,14 @@ void register_console(struct console *newcon) bool realcon_registered = false; int err; + console_list_lock(); + for_each_console(con) { if (WARN(con == newcon, "console '%s%d' already registered\n", - con->name, con->index)) - return; - } + con->name, con->index)) { + goto unlock; + } - for_each_console(con) { if (con->flags & CON_BOOT) bootcon_registered = true; else @@ -3230,7 +3284,7 @@ void register_console(struct console *newcon) if ((newcon->flags & CON_BOOT) && realcon_registered) { pr_info("Too late to register bootconsole %s%d\n", newcon->name, newcon->index); - return; + goto unlock; } /* @@ -3261,7 +3315,7 @@ void register_console(struct console *newcon) /* printk() messages are not printed to the Braille console. */ if (err || newcon->flags & CON_BRL) - return; + goto unlock; /* * If we have a bootconsole, and are switching to a real console, @@ -3320,16 +3374,21 @@ void register_console(struct console *newcon) hlist_for_each_entry_safe(con, tmp, &console_list, node) { if (con->flags & CON_BOOT) - unregister_console(con); + unregister_console_locked(con); } } +unlock: + console_list_unlock(); } EXPORT_SYMBOL(register_console); -int unregister_console(struct console *console) +/* Must be called under console_list_lock(). */ +static int unregister_console_locked(struct console *console) { int res; + lockdep_assert_console_list_lock_held(); + con_printk(KERN_INFO, console, "disabled\n"); res = _braille_unregister_console(console); @@ -3378,6 +3437,16 @@ int unregister_console(struct console *console) return res; } + +int unregister_console(struct console *console) +{ + int res; + + console_list_lock(); + res = unregister_console_locked(console); + console_list_unlock(); + return res; +} EXPORT_SYMBOL(unregister_console); /* @@ -3430,6 +3499,7 @@ static int __init printk_late_init(void) struct console *con; int ret; + console_list_lock(); hlist_for_each_entry_safe(con, tmp, &console_list, node) { if (!(con->flags & CON_BOOT)) continue; @@ -3447,9 +3517,10 @@ static int __init printk_late_init(void) */ pr_warn("bootconsole [%s%d] uses init memory and must be disabled even before the real one is ready\n", con->name, con->index); - unregister_console(con); + unregister_console_locked(con); } } + console_list_unlock(); ret = cpuhp_setup_state_nocalls(CPUHP_PRINTK_DEAD, "printk:dead", NULL, console_cpu_notify); -- cgit v1.2.3 From 100bdef2c198b8dc64df011dc4a2152db913c8ba Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:24 +0106 Subject: console: introduce wrappers to read/write console flags After switching to SRCU for console list iteration, some readers will begin readings console->flags as a data race. Locklessly reading console->flags provides a consistent value because there is at most one CPU modifying console->flags and that CPU is using only read-modify-write operations. Introduce a wrapper for SRCU iterators to read console flags. Introduce a matching wrapper to write to flags of registered consoles. Writing to flags of registered consoles is synchronized by the console_list_lock. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-13-john.ogness@linutronix.de --- include/linux/console.h | 45 +++++++++++++++++++++++++++++++++++++++++++++ kernel/printk/printk.c | 10 +++++----- 2 files changed, 50 insertions(+), 5 deletions(-) (limited to 'kernel') diff --git a/include/linux/console.h b/include/linux/console.h index 24d83e24840b..c1ca461d088a 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -183,6 +183,51 @@ extern void console_list_unlock(void) __releases(console_mutex); extern struct hlist_head console_list; +/** + * console_srcu_read_flags - Locklessly read the console flags + * @con: struct console pointer of console to read flags from + * + * This function provides the necessary READ_ONCE() and data_race() + * notation for locklessly reading the console flags. The READ_ONCE() + * in this function matches the WRITE_ONCE() when @flags are modified + * for registered consoles with console_srcu_write_flags(). + * + * Only use this function to read console flags when locklessly + * iterating the console list via srcu. + * + * Context: Any context. + */ +static inline short console_srcu_read_flags(const struct console *con) +{ + WARN_ON_ONCE(!console_srcu_read_lock_is_held()); + + /* + * Locklessly reading console->flags provides a consistent + * read value because there is at most one CPU modifying + * console->flags and that CPU is using only read-modify-write + * operations to do so. + */ + return data_race(READ_ONCE(con->flags)); +} + +/** + * console_srcu_write_flags - Write flags for a registered console + * @con: struct console pointer of console to write flags to + * @flags: new flags value to write + * + * Only use this function to write flags for registered consoles. It + * requires holding the console_list_lock. + * + * Context: Any context. + */ +static inline void console_srcu_write_flags(struct console *con, short flags) +{ + lockdep_assert_console_list_lock_held(); + + /* This matches the READ_ONCE() in console_srcu_read_flags(). */ + WRITE_ONCE(con->flags, flags); +} + /** * for_each_console_srcu() - Iterator over registered consoles * @con: struct console pointer used as loop cursor diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 2b4506673a86..cad65d34c96d 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3069,7 +3069,7 @@ void console_stop(struct console *console) __pr_flush(console, 1000, true); console_list_lock(); console_lock(); - console->flags &= ~CON_ENABLED; + console_srcu_write_flags(console, console->flags & ~CON_ENABLED); console_unlock(); console_list_unlock(); @@ -3087,7 +3087,7 @@ void console_start(struct console *console) { console_list_lock(); console_lock(); - console->flags |= CON_ENABLED; + console_srcu_write_flags(console, console->flags | CON_ENABLED); console_unlock(); console_list_unlock(); __pr_flush(console, 1000, true); @@ -3343,7 +3343,7 @@ void register_console(struct console *newcon) } else if (newcon->flags & CON_CONSDEV) { /* Only the new head can have CON_CONSDEV set. */ - console_first()->flags &= ~CON_CONSDEV; + console_srcu_write_flags(console_first(), console_first()->flags & ~CON_CONSDEV); hlist_add_head_rcu(&newcon->node, &console_list); } else { @@ -3400,7 +3400,7 @@ static int unregister_console_locked(struct console *console) console_lock(); /* Disable it unconditionally */ - console->flags &= ~CON_ENABLED; + console_srcu_write_flags(console, console->flags & ~CON_ENABLED); if (hlist_unhashed(&console->node)) { console_unlock(); @@ -3419,7 +3419,7 @@ static int unregister_console_locked(struct console *console) * console has any device attached. Oh well.... */ if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV) - console_first()->flags |= CON_CONSDEV; + console_srcu_write_flags(console_first(), console_first()->flags | CON_CONSDEV); console_unlock(); -- cgit v1.2.3 From b8ef04be6e1935748ef98af98d81b0744cf225d0 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:26 +0106 Subject: kdb: use srcu console list iterator Guarantee safe iteration of the console list by using SRCU. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Reviewed-by: Aaron Tomlin Reviewed-by: Douglas Anderson Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-15-john.ogness@linutronix.de --- kernel/debug/kdb/kdb_io.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'kernel') diff --git a/kernel/debug/kdb/kdb_io.c b/kernel/debug/kdb/kdb_io.c index 67d3c48a1522..5c7e9ba7cd6b 100644 --- a/kernel/debug/kdb/kdb_io.c +++ b/kernel/debug/kdb/kdb_io.c @@ -545,6 +545,7 @@ static void kdb_msg_write(const char *msg, int msg_len) { struct console *c; const char *cp; + int cookie; int len; if (msg_len == 0) @@ -558,8 +559,20 @@ static void kdb_msg_write(const char *msg, int msg_len) cp++; } - for_each_console(c) { - if (!(c->flags & CON_ENABLED)) + /* + * The console_srcu_read_lock() only provides safe console list + * traversal. The use of the ->write() callback relies on all other + * CPUs being stopped at the moment and console drivers being able to + * handle reentrance when @oops_in_progress is set. + * + * There is no guarantee that every console driver can handle + * reentrance in this way; the developer deploying the debugger + * is responsible for ensuring that the console drivers they + * have selected handle reentrance appropriately. + */ + cookie = console_srcu_read_lock(); + for_each_console_srcu(c) { + if (!(console_srcu_read_flags(c) & CON_ENABLED)) continue; if (c == dbg_io_ops->cons) continue; @@ -577,6 +590,7 @@ static void kdb_msg_write(const char *msg, int msg_len) --oops_in_progress; touch_nmi_watchdog(); } + console_srcu_read_unlock(cookie); } int vkdb_printf(enum kdb_msgsrc src, const char *fmt, va_list ap) -- cgit v1.2.3 From fc956ae0de7fa25f99114609f21b76e7d38dc25c Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:27 +0106 Subject: printk: console_flush_all: use srcu console list iterator Guarantee safe iteration of the console list by using SRCU. Note that in the case of a handover, the SRCU read lock is also released. This is documented in the function description and as comments in the code. It is a bit tricky, but this preserves the lockdep lock ordering for the context handing over the console_lock: console_lock() | mutex_acquire(&console_lock_dep_map) <-- console lock | console_unlock() | console_flush_all() | | srcu_read_lock(&console_srcu) <-- srcu lock | | console_emit_next_record() | | | console_lock_spinning_disable_and_check() | | | | srcu_read_unlock(&console_srcu) <-- srcu unlock | | | | mutex_release(&console_lock_dep_map) <-- console unlock Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-16-john.ogness@linutronix.de --- kernel/printk/printk.c | 50 +++++++++++++++++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 15 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index cad65d34c96d..13bc960c7aa5 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -1894,13 +1894,13 @@ static void console_lock_spinning_enable(void) * safe to start busy waiting for the lock. Second, it checks if * there is a busy waiter and passes the lock rights to her. * - * Important: Callers lose the lock if there was a busy waiter. - * They must not touch items synchronized by console_lock - * in this case. + * Important: Callers lose both the console_lock and the SRCU read lock if + * there was a busy waiter. They must not touch items synchronized by + * console_lock or SRCU read lock in this case. * * Return: 1 if the lock rights were passed, 0 otherwise. */ -static int console_lock_spinning_disable_and_check(void) +static int console_lock_spinning_disable_and_check(int cookie) { int waiter; @@ -1919,6 +1919,12 @@ static int console_lock_spinning_disable_and_check(void) spin_release(&console_owner_dep_map, _THIS_IP_); + /* + * Preserve lockdep lock ordering. Release the SRCU read lock before + * releasing the console_lock. + */ + console_srcu_read_unlock(cookie); + /* * Hand off console_lock to waiter. The waiter will perform * the up(). After this, the waiter is the console_lock owner. @@ -2402,7 +2408,7 @@ static ssize_t msg_print_ext_body(char *buf, size_t size, char *text, size_t text_len, struct dev_printk_info *dev_info) { return 0; } static void console_lock_spinning_enable(void) { } -static int console_lock_spinning_disable_and_check(void) { return 0; } +static int console_lock_spinning_disable_and_check(int cookie) { return 0; } static void call_console_driver(struct console *con, const char *text, size_t len, char *dropped_text) { @@ -2744,16 +2750,18 @@ static void __console_unlock(void) * DROPPED_TEXT_MAX. Otherwise @dropped_text must be NULL. * * @handover will be set to true if a printk waiter has taken over the - * console_lock, in which case the caller is no longer holding the - * console_lock. Otherwise it is set to false. + * console_lock, in which case the caller is no longer holding both the + * console_lock and the SRCU read lock. Otherwise it is set to false. + * + * @cookie is the cookie from the SRCU read lock. * * Returns false if the given console has no next record to print, otherwise * true. * - * Requires the console_lock. + * Requires the console_lock and the SRCU read lock. */ static bool console_emit_next_record(struct console *con, char *text, char *ext_text, - char *dropped_text, bool *handover) + char *dropped_text, bool *handover, int cookie) { static int panic_console_dropped; struct printk_info info; @@ -2813,7 +2821,7 @@ static bool console_emit_next_record(struct console *con, char *text, char *ext_ con->seq++; - *handover = console_lock_spinning_disable_and_check(); + *handover = console_lock_spinning_disable_and_check(cookie); printk_safe_exit_irqrestore(flags); skip: return true; @@ -2850,6 +2858,7 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove bool any_usable = false; struct console *con; bool any_progress; + int cookie; *next_seq = 0; *handover = false; @@ -2857,23 +2866,29 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove do { any_progress = false; - for_each_console(con) { + cookie = console_srcu_read_lock(); + for_each_console_srcu(con) { bool progress; if (!console_is_usable(con)) continue; any_usable = true; - if (con->flags & CON_EXTENDED) { + if (console_srcu_read_flags(con) & CON_EXTENDED) { /* Extended consoles do not print "dropped messages". */ progress = console_emit_next_record(con, &text[0], &ext_text[0], NULL, - handover); + handover, cookie); } else { progress = console_emit_next_record(con, &text[0], NULL, &dropped_text[0], - handover); + handover, cookie); } + + /* + * If a handover has occurred, the SRCU read lock + * is already released. + */ if (*handover) return false; @@ -2887,14 +2902,19 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove /* Allow panic_cpu to take over the consoles safely. */ if (abandon_console_lock_in_panic()) - return false; + goto abandon; if (do_cond_resched) cond_resched(); } + console_srcu_read_unlock(cookie); } while (any_progress); return any_usable; + +abandon: + console_srcu_read_unlock(cookie); + return false; } /** -- cgit v1.2.3 From eb7f1ed2509c6223d960a7d69644a8eb506b13d0 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:28 +0106 Subject: printk: __pr_flush: use srcu console list iterator Use srcu console list iteration for console list traversal. Document why the console_lock is still necessary. Note that this is a preparatory change for when console_lock no longer provides synchronization for the console list. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-17-john.ogness@linutronix.de --- kernel/printk/printk.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 13bc960c7aa5..abdbd3ffb068 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3561,6 +3561,7 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre struct console *c; u64 last_diff = 0; u64 printk_seq; + int cookie; u64 diff; u64 seq; @@ -3571,9 +3572,15 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre for (;;) { diff = 0; + /* + * Hold the console_lock to guarantee safe access to + * console->seq and to prevent changes to @console_suspended + * until all consoles have been processed. + */ console_lock(); - for_each_console(c) { + cookie = console_srcu_read_lock(); + for_each_console_srcu(c) { if (con && con != c) continue; if (!console_is_usable(c)) @@ -3582,6 +3589,7 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre if (printk_seq < seq) diff += seq - printk_seq; } + console_srcu_read_unlock(cookie); /* * If consoles are suspended, it cannot be expected that they -- cgit v1.2.3 From 12f1da5fc4c728dc690a2fe3f4197d62f3a6fa7b Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:29 +0106 Subject: printk: console_is_usable: use console_srcu_read_flags All users of console_is_usable() are SRCU iterators. Use the appropriate wrapper function to locklessly read the flags. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-18-john.ogness@linutronix.de --- kernel/printk/printk.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index abdbd3ffb068..853a86c94ee0 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -2709,11 +2709,13 @@ static bool abandon_console_lock_in_panic(void) * Check if the given console is currently capable and allowed to print * records. * - * Requires the console_lock. + * Requires the console_srcu_read_lock. */ static inline bool console_is_usable(struct console *con) { - if (!(con->flags & CON_ENABLED)) + short flags = console_srcu_read_flags(con); + + if (!(flags & CON_ENABLED)) return false; if (!con->write) @@ -2724,8 +2726,7 @@ static inline bool console_is_usable(struct console *con) * allocated. So unless they're explicitly marked as being able to * cope (CON_ANYTIME) don't call them until this CPU is officially up. */ - if (!cpu_online(raw_smp_processor_id()) && - !(con->flags & CON_ANYTIME)) + if (!cpu_online(raw_smp_processor_id()) && !(flags & CON_ANYTIME)) return false; return true; -- cgit v1.2.3 From d792db6f6b902dad6b751dc2ef226fb4463efe62 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:30 +0106 Subject: printk: console_unblank: use srcu console list iterator Use srcu console list iteration for console list traversal. Document why the console_lock is still necessary. Note that this is a preparatory change for when console_lock no longer provides synchronization for the console list. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-19-john.ogness@linutronix.de --- kernel/printk/printk.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 853a86c94ee0..8e37a35764f2 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3000,10 +3000,14 @@ EXPORT_SYMBOL(console_conditional_schedule); void console_unblank(void) { struct console *c; + int cookie; /* - * console_unblank can no longer be called in interrupt context unless - * oops_in_progress is set to 1.. + * Stop console printing because the unblank() callback may + * assume the console is not within its write() callback. + * + * If @oops_in_progress is set, this may be an atomic context. + * In that case, attempt a trylock as best-effort. */ if (oops_in_progress) { if (down_trylock_console_sem() != 0) @@ -3013,9 +3017,14 @@ void console_unblank(void) console_locked = 1; console_may_schedule = 0; - for_each_console(c) - if ((c->flags & CON_ENABLED) && c->unblank) + + cookie = console_srcu_read_lock(); + for_each_console_srcu(c) { + if ((console_srcu_read_flags(c) & CON_ENABLED) && c->unblank) c->unblank(); + } + console_srcu_read_unlock(cookie); + console_unlock(); if (!oops_in_progress) -- cgit v1.2.3 From 87f2e4b7d9e5c50756184b4f8d0b92680de61496 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:31 +0106 Subject: printk: console_flush_on_panic: use srcu console list iterator With SRCU it is now safe to traverse the console list, even if the console_trylock() failed. However, overwriting console->seq when console_trylock() failed is still an issue. Switch to SRCU iteration and document remaining issue with console->seq. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-20-john.ogness@linutronix.de --- kernel/printk/printk.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 8e37a35764f2..b78947077659 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3050,21 +3050,22 @@ void console_flush_on_panic(enum con_flush_mode mode) console_may_schedule = 0; if (mode == CONSOLE_REPLAY_ALL) { - struct hlist_node *tmp; struct console *c; + int cookie; u64 seq; seq = prb_first_valid_seq(prb); - /* - * This cannot use for_each_console() because it's not established - * that the current context has console locked and neither there is - * a guarantee that there is no concurrency in that case. - * - * Open code it for documentation purposes and pretend that - * it works. - */ - hlist_for_each_entry_safe(c, tmp, &console_list, node) + + cookie = console_srcu_read_lock(); + for_each_console_srcu(c) { + /* + * If the above console_trylock() failed, this is an + * unsynchronized assignment. But in that case, the + * kernel is in "hope and pray" mode anyway. + */ c->seq = seq; + } + console_srcu_read_unlock(cookie); } console_unlock(); } -- cgit v1.2.3 From 8cb15f7f492f85d695a6f4d12044c47230f69d96 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:32 +0106 Subject: printk: console_device: use srcu console list iterator Use srcu console list iteration for console list traversal. It is acceptable because the consoles might come and go at any time. Strict synchronizing with console registration code would not bring any advantage over srcu. Document why the console_lock is still necessary. Note that this is a preparatory change for when console_lock no longer provides synchronization for the console list. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-21-john.ogness@linutronix.de --- kernel/printk/printk.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index b78947077659..101d08d9dc88 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3077,15 +3077,25 @@ struct tty_driver *console_device(int *index) { struct console *c; struct tty_driver *driver = NULL; + int cookie; + /* + * Take console_lock to serialize device() callback with + * other console operations. For example, fg_console is + * modified under console_lock when switching vt. + */ console_lock(); - for_each_console(c) { + + cookie = console_srcu_read_lock(); + for_each_console_srcu(c) { if (!c->device) continue; driver = c->device(c, index); if (driver) break; } + console_srcu_read_unlock(cookie); + console_unlock(); return driver; } -- cgit v1.2.3 From 1fd4224a6b641f1949e27bf350b5b1c1e47e2ccc Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:33 +0106 Subject: console: introduce console_is_registered() Currently it is not possible for drivers to detect if they have already successfully registered their console. Several drivers have multiple paths that lead to console registration. To avoid attempting a 2nd registration (which leads to a WARN), drivers are implementing their own solution. Introduce console_is_registered() so drivers can easily identify if their console is currently registered. A _locked() variant is also provided if the caller is already holding the console_list_lock. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-22-john.ogness@linutronix.de --- include/linux/console.h | 28 ++++++++++++++++++++++++++++ kernel/printk/printk.c | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) (limited to 'kernel') diff --git a/include/linux/console.h b/include/linux/console.h index c1ca461d088a..f716e1dd9eaf 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -228,6 +228,34 @@ static inline void console_srcu_write_flags(struct console *con, short flags) WRITE_ONCE(con->flags, flags); } +/* Variant of console_is_registered() when the console_list_lock is held. */ +static inline bool console_is_registered_locked(const struct console *con) +{ + lockdep_assert_console_list_lock_held(); + return !hlist_unhashed(&con->node); +} + +/* + * console_is_registered - Check if the console is registered + * @con: struct console pointer of console to check + * + * Context: Process context. May sleep while acquiring console list lock. + * Return: true if the console is in the console list, otherwise false. + * + * If false is returned for a console that was previously registered, it + * can be assumed that the console's unregistration is fully completed, + * including the exit() callback after console list removal. + */ +static inline bool console_is_registered(const struct console *con) +{ + bool ret; + + console_list_lock(); + ret = console_is_registered_locked(con); + console_list_unlock(); + return ret; +} + /** * for_each_console_srcu() - Iterator over registered consoles * @con: struct console pointer used as loop cursor diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 101d08d9dc88..e58a6ccb945c 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -3443,7 +3443,7 @@ static int unregister_console_locked(struct console *console) /* Disable it unconditionally */ console_srcu_write_flags(console, console->flags & ~CON_ENABLED); - if (hlist_unhashed(&console->node)) { + if (!console_is_registered_locked(console)) { console_unlock(); return -ENODEV; } -- cgit v1.2.3 From 6f8836756f3cbb02bfd1f0e033516585250318a9 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:44 +0106 Subject: printk, xen: fbfront: create/use safe function for forcing preferred With commit 9e124fe16ff2("xen: Enable console tty by default in domU if it's not a dummy") a hack was implemented to make sure that the tty console remains the console behind the /dev/console device. The main problem with the hack is that, after getting the console pointer to the tty console, it is assumed the pointer is still valid after releasing the console_sem. This assumption is incorrect and unsafe. Make the hack safe by introducing a new function console_force_preferred_locked() and perform the full operation under the console_list_lock. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-33-john.ogness@linutronix.de --- drivers/video/fbdev/xen-fbfront.c | 12 ++++------ include/linux/console.h | 1 + kernel/printk/printk.c | 49 ++++++++++++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 11 deletions(-) (limited to 'kernel') diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c index 4d2694d904aa..8752d389e382 100644 --- a/drivers/video/fbdev/xen-fbfront.c +++ b/drivers/video/fbdev/xen-fbfront.c @@ -504,18 +504,14 @@ static void xenfb_make_preferred_console(void) if (console_set_on_cmdline) return; - console_lock(); + console_list_lock(); for_each_console(c) { if (!strcmp(c->name, "tty") && c->index == 0) break; } - console_unlock(); - if (c) { - unregister_console(c); - c->flags |= CON_CONSDEV; - c->flags &= ~CON_PRINTBUFFER; /* don't print again */ - register_console(c); - } + if (c) + console_force_preferred_locked(c); + console_list_unlock(); } static int xenfb_resume(struct xenbus_device *dev) diff --git a/include/linux/console.h b/include/linux/console.h index f716e1dd9eaf..9cea254b34b8 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -291,6 +291,7 @@ enum con_flush_mode { }; extern int add_preferred_console(char *name, int idx, char *options); +extern void console_force_preferred_locked(struct console *con); extern void register_console(struct console *); extern int unregister_console(struct console *); extern void console_lock(void); diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index e58a6ccb945c..bda9e8ec6817 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -248,9 +248,10 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write, void console_list_lock(void) { /* - * In unregister_console(), synchronize_srcu() is called with the - * console_list_lock held. Therefore it is not allowed that the - * console_list_lock is taken with the srcu_lock held. + * In unregister_console() and console_force_preferred_locked(), + * synchronize_srcu() is called with the console_list_lock held. + * Therefore it is not allowed that the console_list_lock is taken + * with the srcu_lock held. * * Detecting if this context is really in the read-side critical * section is only possible if the appropriate debug options are @@ -3490,6 +3491,48 @@ int unregister_console(struct console *console) } EXPORT_SYMBOL(unregister_console); +/** + * console_force_preferred_locked - force a registered console preferred + * @con: The registered console to force preferred. + * + * Must be called under console_list_lock(). + */ +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; + + /* + * Delete, but do not re-initialize the entry. This allows the console + * to continue to appear registered (via any hlist_unhashed_lockless() + * checks), even though it was briefly removed from the console list. + */ + hlist_del_rcu(&con->node); + + /* + * Ensure that all SRCU list walks have completed so that the console + * can be added to the beginning of the console list and its forward + * list pointer can be re-initialized. + */ + synchronize_srcu(&console_srcu); + + con->flags |= CON_CONSDEV; + WARN_ON(!con->device); + + /* Only the new head can have CON_CONSDEV set. */ + console_srcu_write_flags(cur_pref_con, cur_pref_con->flags & ~CON_CONSDEV); + hlist_add_head_rcu(&con->node, &console_list); +} +EXPORT_SYMBOL(console_force_preferred_locked); + /* * Initialize the console device. This is called *early*, so * we can't necessarily depend on lots of kernel help here. -- cgit v1.2.3 From 848a9c106625b10fa74022ea853845a2b7a834f0 Mon Sep 17 00:00:00 2001 From: John Ogness Date: Wed, 16 Nov 2022 17:27:51 +0106 Subject: printk: relieve console_lock of list synchronization duties The console_list_lock provides synchronization for console list and console->flags updates. All call sites that were using the console_lock for this synchronization have either switched to use the console_list_lock or the SRCU list iterator. Remove console_lock usage for console list updates and console->flags updates. Signed-off-by: John Ogness Reviewed-by: Petr Mladek Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/20221116162152.193147-40-john.ogness@linutronix.de --- kernel/printk/printk.c | 44 ++++++++++++++++++++------------------------ 1 file changed, 20 insertions(+), 24 deletions(-) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index bda9e8ec6817..503d55ccc835 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -86,8 +86,8 @@ EXPORT_SYMBOL(oops_in_progress); static DEFINE_MUTEX(console_mutex); /* - * console_sem protects console_list and console->flags updates, and also - * provides serialization for access to the entire console driver system. + * console_sem protects updates to console->seq and console_suspended, + * and also provides serialization for console printing. */ static DEFINE_SEMAPHORE(console_sem); HLIST_HEAD(console_list); @@ -2640,10 +2640,10 @@ static int console_cpu_notify(unsigned int cpu) } /** - * console_lock - lock the console system for exclusive use. + * console_lock - block the console subsystem from printing * - * Acquires a lock which guarantees that the caller has - * exclusive access to the console system and console_list. + * Acquires a lock which guarantees that no consoles will + * be in or enter their write() callback. * * Can sleep, returns nothing. */ @@ -2660,10 +2660,10 @@ void console_lock(void) EXPORT_SYMBOL(console_lock); /** - * console_trylock - try to lock the console system for exclusive use. + * console_trylock - try to block the console subsystem from printing * - * Try to acquire a lock which guarantees that the caller has exclusive - * access to the console system and console_list. + * Try to acquire a lock which guarantees that no consoles will + * be in or enter their write() callback. * * returns 1 on success, and 0 on failure to acquire the lock. */ @@ -2920,10 +2920,10 @@ abandon: } /** - * console_unlock - unlock the console system + * console_unlock - unblock the console subsystem from printing * - * Releases the console_lock which the caller holds on the console system - * and the console driver list. + * Releases the console_lock which the caller holds to block printing of + * the console subsystem. * * While the console_lock was held, console output may have been buffered * by printk(). If this is the case, console_unlock(); emits @@ -3110,9 +3110,7 @@ void console_stop(struct console *console) { __pr_flush(console, 1000, true); console_list_lock(); - console_lock(); console_srcu_write_flags(console, console->flags & ~CON_ENABLED); - console_unlock(); console_list_unlock(); /* @@ -3128,9 +3126,7 @@ EXPORT_SYMBOL(console_stop); void console_start(struct console *console) { console_list_lock(); - console_lock(); console_srcu_write_flags(console, console->flags | CON_ENABLED); - console_unlock(); console_list_unlock(); __pr_flush(console, 1000, true); } @@ -3247,6 +3243,12 @@ static void console_init_seq(struct console *newcon, bool bootcon_registered) * the furthest behind. */ if (bootcon_registered && !keep_bootcon) { + /* + * Hold the console_lock to stop console printing and + * guarantee safe access to console->seq. + */ + console_lock(); + /* * Flush all consoles and set the console to start at * the next unprinted sequence number. @@ -3273,6 +3275,8 @@ static void console_init_seq(struct console *newcon, bool bootcon_registered) } } } + + console_unlock(); } } } @@ -3371,7 +3375,6 @@ void register_console(struct console *newcon) } newcon->dropped = 0; - console_lock(); console_init_seq(newcon, bootcon_registered); /* @@ -3391,7 +3394,6 @@ void register_console(struct console *newcon) } else { hlist_add_behind_rcu(&newcon->node, console_list.first); } - console_unlock(); /* * No need to synchronize SRCU here! The caller does not rely @@ -3439,15 +3441,11 @@ static int unregister_console_locked(struct console *console) if (res > 0) return 0; - console_lock(); - /* Disable it unconditionally */ console_srcu_write_flags(console, console->flags & ~CON_ENABLED); - if (!console_is_registered_locked(console)) { - console_unlock(); + if (!console_is_registered_locked(console)) return -ENODEV; - } hlist_del_init_rcu(&console->node); @@ -3463,8 +3461,6 @@ static int unregister_console_locked(struct console *console) if (!hlist_empty(&console_list) && console->flags & CON_CONSDEV) console_srcu_write_flags(console_first(), console_first()->flags | CON_CONSDEV); - console_unlock(); - /* * Ensure that all SRCU list walks have completed. All contexts * must not be able to see this console in the list so that any -- cgit v1.2.3 From 5074ffbec67ac592614901771d3a15e1198d759d Mon Sep 17 00:00:00 2001 From: John Ogness Date: Tue, 22 Nov 2022 10:01:23 +0106 Subject: printk: htmldocs: add missing description Variable and return descriptions were missing from the SRCU read lock functions. Add them. Signed-off-by: John Ogness Signed-off-by: Petr Mladek Link: https://lore.kernel.org/r/87zgcjpdvo.fsf@jogness.linutronix.de --- kernel/printk/printk.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'kernel') diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c index 503d55ccc835..9ec101766471 100644 --- a/kernel/printk/printk.c +++ b/kernel/printk/printk.c @@ -282,6 +282,7 @@ EXPORT_SYMBOL(console_list_unlock); * Use for_each_console_srcu() to iterate the console list * * Context: Any context. + * Return: A cookie to pass to console_srcu_read_unlock(). */ int console_srcu_read_lock(void) { @@ -292,6 +293,7 @@ EXPORT_SYMBOL(console_srcu_read_lock); /** * console_srcu_read_unlock - Unregister an old reader from * the SRCU-protected console list + * @cookie: cookie returned from console_srcu_read_lock() * * Counterpart to console_srcu_read_lock() */ -- cgit v1.2.3