[2/2] bcachefs: only console_trylock in bch2_print_string_as_lines

Commit Message

Daniel Vetter July 10, 2024, 9:31 a.m. UTC

console_lock is the outermost subsystem lock for a lot of subsystems,
which means get/put_user must nest within. Which means it cannot be
acquired somewhere deeply nested in other locks, and most definitely
not while holding fs locks potentially needed to resolve faults.

console_trylock is the best we can do here.

Including printk folks since even trylock feels realyl iffy here to
me.

Reported-by: [e-mail]
References: https://lore.kernel.org/dri-devel/[e-mail]/
Signed-off-by: Daniel Vetter <[e-mail]>
Fixes: a8f354284304 ("bcachefs: bch2_print_string_as_lines()")
Cc: <[e-mail]> # v6.7+
Cc: Kent Overstreet <[e-mail]>
Cc: Brian Foster <[e-mail]>
Cc: [e-mail]
Cc: Petr Mladek <[e-mail]>
Cc: Steven Rostedt <[e-mail]>
Cc: John Ogness <[e-mail]>
Cc: Sergey Senozhatsky <[e-mail]>
Signed-off-by: Daniel Vetter <[e-mail]>
---
 fs/bcachefs/util.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

John Ogness July 10, 2024, 11:02 a.m. UTC | #1

On 2024-07-10, Daniel Vetter <[e-mail]> wrote:
> console_lock is the outermost subsystem lock for a lot of subsystems,
> which means get/put_user must nest within. Which means it cannot be
> acquired somewhere deeply nested in other locks, and most definitely
> not while holding fs locks potentially needed to resolve faults.
>
> console_trylock is the best we can do here.
>
> Including printk folks since even trylock feels realyl iffy here to
> me.

Using the console lock here at all is wrong. The console lock does not
prevent other CPUs from calling printk() and inserting lines in between.

There is no way to guarantee a contiguous ringbuffer block using
multiple printk() calls.

The console_lock usage should be removed.

John Ogness

Patch

diff --git a/fs/bcachefs/util.c b/fs/bcachefs/util.c
index de331dec2a99..02381c653603 100644
--- a/fs/bcachefs/util.c
+++ b/fs/bcachefs/util.c
@@ -255,13 +255,14 @@  void bch2_prt_u64_base2(struct printbuf *out, u64 v)
 void bch2_print_string_as_lines(const char *prefix, const char *lines)
 {
 	const char *p;
+	int locked;
 
 	if (!lines) {
 		printk("%s (null)\n", prefix);
 		return;
 	}
 
-	console_lock();
+	locked = console_trylock();
 	while (1) {
 		p = strchrnul(lines, '\n');
 		printk("%s%.*s\n", prefix, (int) (p - lines), lines);
@@ -269,7 +270,8 @@  void bch2_print_string_as_lines(const char *prefix, const char *lines)
 			break;
 		lines = p + 1;
 	}
-	console_unlock();
+	if (locked)
+		console_unlock();
 }
 
 int bch2_save_backtrace(bch_stacktrace *stack, struct task_struct *task, unsigned skipnr,