Messages in this thread Patch in this message | | | From | Stephen Brennan <> | Subject | Re: [PATCH v3 4/4] printk: Drop console_sem during panic | Date | Fri, 04 Feb 2022 10:53:48 -0800 |
| |
Sergey Senozhatsky <senozhatsky@chromium.org> writes: > On (22/02/01 10:58), Stephen Brennan wrote: >> +/* >> + * Return true when this CPU should unlock console_sem without pushing all >> + * messages to the console. This reduces the chance that the console is >> + * locked when the panic CPU tries to use it. >> + */ >> +static bool abandon_console_lock_in_panic(void) >> +{ >> + if (!panic_in_progress()) >> + return false; >> + >> + /* >> + * We can use raw_smp_processor_id() here because it is impossible for >> + * the task to be migrated to the panic_cpu, or away from it. If >> + * panic_cpu has already been set, and we're not currently executing on >> + * that CPU, then we never will be. >> + */ >> + return atomic_read(&panic_cpu) != raw_smp_processor_id(); >> +} >> + >> /* >> * Can we actually use the console at this time on this cpu? >> * >> @@ -2746,6 +2765,10 @@ void console_unlock(void) >> if (handover) >> return; >> >> + /* Allow panic_cpu to take over the consoles safely */ >> + if (abandon_console_lock_in_panic()) >> + break; > > Sorry, why not just `return` like in handover case?
We need to drop console_sem before returning, since the whole benefit here is to increase the chance that console_sem is unlocked when the panic_cpu halts this CPU.
in the handover case, there's another cpu waiting, and we're essentially transferring the console_sem ownership to that cpu, so we explicitly return and skip the unlocking portion.
Does this need some comments to clarify it?
Admittedly if I had a few more lines of context in the diff, you would see the console unlock directly after the loop; it's a bit clearer when you're looking at the function as whole:
2768 /* Allow panic_cpu to take over the consoles safely */ 2769 if (abandon_console_lock_in_panic()) 2770 break; 2771 2772 if (do_cond_resched) 2773 cond_resched(); 2774 } 2775 2776 /* Get consistent value of the next-to-be-used sequence number. */ 2777 next_seq = console_seq; 2778 2779 console_locked = 0; 2780 up_console_sem();
Stephen
> >> + >> if (do_cond_resched) >> cond_resched(); >> } >> @@ -2763,7 +2786,7 @@ void console_unlock(void) >> * flush, no worries. >> */ >> retry = prb_read_valid(prb, next_seq, NULL); >> - if (retry && console_trylock()) >> + if (retry && !abandon_console_lock_in_panic() && console_trylock()) >> goto again; >> } >> EXPORT_SYMBOL(console_unlock);
| |