lkml.org 
[lkml]   [2022]   [Nov]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH printk v2 27/38] printk: console_flush_all: use srcu console list iterator
    Date
    On 2022-10-25, Petr Mladek <pmladek@suse.com> wrote:
    >> 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
    >>
    >> @@ -2819,12 +2827,17 @@ static bool console_flush_all(bool do_cond_resched, u64 *next_seq, bool *handove
    >> /* 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;
    >
    > Please, release the SRCU read lock here:
    >
    > if (*handover) {
    > console_srcu_read_unlock(cookie);
    > return false;
    > }
    >
    > The lock should be released in the same function where it was taken.
    > It does not require passing the cookie and looks more straightforward.

    It looks more straight forward, but it is incorrect from a locking
    perspective.

    The locking order was:

    console_lock()
    console_srcu_read_lock()

    But for a handover at this point in code, console_emit_next_record() has
    already released the console_lock (to the spinning context). The
    console_srcu_read_lock should have been released first.

    > We actually do the same when abandon_console_lock_in_panic()
    > returns true. We could share the code:
    >
    > handover_abandon:
    > console_srcu_read_unlock(cookie);
    > return false;

    This case is different. Here the console_lock was not released yet so it
    is fine to perform the console_srcu_read_unlock() here.

    John Ogness

    \
     
     \ /
      Last update: 2022-11-07 01:00    [W:5.087 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site