Messages in this thread Patch in this message | | | From | John Ogness <> | Subject | Re: [patch RFC 28/29] printk: Provide functions for atomic write enforcement | Date | Tue, 27 Sep 2022 16:46:46 +0206 |
| |
Below is a fix that was used for the LPC2022 demo so that an interrupting context does not clobber the console state when the interrupted context was the console owner.
On 2022-09-11, Thomas Gleixner <tglx@linutronix.de> wrote: > --- a/kernel/printk/printk_nobkl.c > +++ b/kernel/printk/printk_nobkl.c > +/** > + * cons_atomic_flush_one - Flush one console in atomic mode > + * @con: The console to flush > + * @prio: The priority of the current context > + */ > +static void cons_atomic_flush_one(struct console *con, enum cons_prio prio) > +{ > + struct cons_write_context *wctxt = cons_get_wctxt(con, prio); > + struct cons_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt);
@ctxt is per-console, per-cpu, and per-prio. But it is *not* per-context. If a CPU is in EMERGENCY priority and is interrupted by another context that calls into printk() while the first context had the console locked, the following cons_atomic_try_acquire() will use the same ctxt and clobber the values used by the first context. This corrupts the state information for the first context and results in situations where the console is never unlocked because the owner's state was corrupt.
> + if (!cons_atomic_try_acquire(con, ctxt, prio)) > + return; > + > + do { > + /* > + * cons_emit_record() returns false when the console was > + * handed over or taken over. In both cases the context is > + * not longer valid. > + */ > + if (!cons_emit_record(wctxt)) > + return; > + } while (ctxt->backlog); > + > + cons_release(ctxt); > +}
Since it is not desirable to actively print from nested contexts of the same priority, add an @in_use flag to wctxt, allowing to detect the situation and avoid active printing and corrupting the state.
Applying the following patch on top does just that:
diff --git a/include/linux/console.h b/include/linux/console.h index e6bde0e879fc..1b3028cce3f3 100644 --- a/include/linux/console.h +++ b/include/linux/console.h @@ -317,6 +317,7 @@ struct cons_context { * @len: Length to write * @pos: Current write position in @outbuf * @unsafe: Invoked in unsafe state due to force takeover + * @in_use: The context is in use */ struct cons_write_context { struct cons_context __private ctxt; @@ -324,6 +325,7 @@ struct cons_write_context { unsigned int len; unsigned int pos; bool unsafe; + bool in_use; }; #define CONS_MAX_NEST_LVL 8 diff --git a/kernel/printk/printk_nobkl.c b/kernel/printk/printk_nobkl.c index 736f71361799..b513b2fb2c2d 100644 --- a/kernel/printk/printk_nobkl.c +++ b/kernel/printk/printk_nobkl.c @@ -1338,8 +1338,12 @@ static void cons_atomic_flush_one(struct console *con, enum cons_prio prio) struct cons_write_context *wctxt = cons_get_wctxt(con, prio); struct cons_context *ctxt = &ACCESS_PRIVATE(wctxt, ctxt); - if (!cons_atomic_try_acquire(con, ctxt, prio)) + if (wctxt->in_use) return; + wctxt->in_use = true; + + if (!cons_atomic_try_acquire(con, ctxt, prio)) + goto out; do { /* @@ -1348,10 +1352,12 @@ static void cons_atomic_flush_one(struct console *con, enum cons_prio prio) * not longer valid. */ if (!cons_emit_record(wctxt)) - return; + goto out; } while (ctxt->backlog); cons_release(ctxt); +out: + wctxt->in_use = false; } /** John Ogness
| |