lkml.org 
[lkml]   [2024]   [Apr]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [PATCH printk v4 20/27] printk: Avoid console_lock dance if no legacy or boot consoles
    Date
    On 2024-04-03, John Ogness <john.ogness@linutronix.de> wrote:
    > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
    > index df84c6bfbb2d..4ff3800e8e8e 100644
    > --- a/kernel/printk/printk.c
    > +++ b/kernel/printk/printk.c
    > @@ -3810,6 +3833,7 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
    > u64 last_diff = 0;
    > u64 printk_seq;
    > short flags;
    > + bool locked;
    > int cookie;
    > u64 diff;
    > u64 seq;
    > @@ -3819,22 +3843,35 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
    > seq = prb_next_reserve_seq(prb);
    >
    > /* Flush the consoles so that records up to @seq are printed. */
    > - console_lock();
    > - console_unlock();
    > + if (printing_via_unlock) {
    > + console_lock();
    > + console_unlock();
    > + }
    >
    > for (;;) {
    > unsigned long begin_jiffies;
    > unsigned long slept_jiffies;
    >
    > + locked = false;
    > diff = 0;
    >
    > + if (printing_via_unlock) {
    > + /*
    > + * Hold the console_lock to guarantee safe access to
    > + * console->seq. Releasing console_lock flushes more
    > + * records in case @seq is still not printed on all
    > + * usable consoles.
    > + */
    > + console_lock();
    > + locked = true;
    > + }
    > +
    > /*
    > - * Hold the console_lock to guarantee safe access to
    > - * console->seq. Releasing console_lock flushes more
    > - * records in case @seq is still not printed on all
    > - * usable consoles.
    > + * Ensure the compiler does not optimize @locked to be
    > + * @printing_via_unlock since the latter can change at any
    > + * time.
    > */
    > - console_lock();
    > + barrier();

    When I originally implemented this, my objective was to force the
    compiler to use a local variable. But to be fully paranoid (which we
    should) we must also forbid the compiler from being able to do this
    nonsense:

    if (printing_via_unlock) {
    console_lock();
    locked = printing_via_unlock;
    }

    I suggest replacing the __pr_flush() hunks to be as follows instead
    (i.e. ensure all conditional console lock usage within the loop is using
    the local variable):

    diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
    index df84c6bfbb2d..1dbd2a837b67 100644
    --- a/kernel/printk/printk.c
    +++ b/kernel/printk/printk.c
    @@ -3819,22 +3842,34 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
    seq = prb_next_reserve_seq(prb);

    /* Flush the consoles so that records up to @seq are printed. */
    - console_lock();
    - console_unlock();
    + if (printing_via_unlock) {
    + console_lock();
    + console_unlock();
    + }

    for (;;) {
    unsigned long begin_jiffies;
    unsigned long slept_jiffies;
    -
    - diff = 0;
    + bool use_console_lock = printing_via_unlock;

    /*
    - * Hold the console_lock to guarantee safe access to
    - * console->seq. Releasing console_lock flushes more
    - * records in case @seq is still not printed on all
    - * usable consoles.
    + * Ensure the compiler does not optimize @use_console_lock to
    + * be @printing_via_unlock since the latter can change at any
    + * time.
    */
    - console_lock();
    + barrier();
    +
    + diff = 0;
    +
    + if (use_console_lock) {
    + /*
    + * Hold the console_lock to guarantee safe access to
    + * console->seq. Releasing console_lock flushes more
    + * records in case @seq is still not printed on all
    + * usable consoles.
    + */
    + console_lock();
    + }

    cookie = console_srcu_read_lock();
    for_each_console_srcu(c) {
    @@ -3854,6 +3889,7 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
    if (flags & CON_NBCON) {
    printk_seq = nbcon_seq_read(c);
    } else {
    + WARN_ON_ONCE(!use_console_lock);
    printk_seq = c->seq;
    }

    @@ -3865,7 +3901,8 @@ static bool __pr_flush(struct console *con, int timeout_ms, bool reset_on_progre
    if (diff != last_diff && reset_on_progress)
    remaining_jiffies = timeout_jiffies;

    - console_unlock();
    + if (use_console_lock)
    + console_unlock();

    /* Note: @diff is 0 if there are no usable consoles. */
    if (diff == 0 || remaining_jiffies == 0)
    \
     
     \ /
      Last update: 2024-05-27 16:20    [W:4.847 / U:0.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site