lkml.org 
[lkml]   [2011]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[RFC][PATCH v3 2/3] Skip spin_locks in panic case and add WARN_ON()
    Patch Description:
    - Skip spin_locks in panic case in both kmsg_dump() and pstore_dump() because they are
    serialized via smp_send_stop

    - Add WARN_ON() in "in_nmi() and !panic" case into kmsg_dump(). Currently, this case never
    happens because only kmsg_dump(KMSG_DUMP_PANIC) is called in NMI case.
    But if someone adds new kmsg_dump() into NMI path in the future, kmsg_dump() may deadlock.
    We can trap it and complain with this WARN_ON().


    With this patch, kmsg_dump()/pstore_dump() work as follows.
    panic case (KMSG_DUMP_PANIC):
    - don't take lock because they are serialized.

    not panic case (KMSG_DUMP_OOPS/KMSG_DUMP_EMERG/KMSG_DUMP_RESTART/KMSG_DUMP_HALT):
    - take locks normally

    Regarding as NMI case,
    - kmsg_dump()/pstore_dump() don't take locks, so deadlock issue will not happen
    because kmsg_dump() is called in just panic case with current implementation.
    - If someone adds new kmsg_dump() into NMI path, WARN_ON() is called.
    So we can trap it and ask to fix it.

    Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>

    ---
    fs/pstore/platform.c | 16 ++++++----------
    kernel/printk.c | 13 +++++++++++--
    2 files changed, 17 insertions(+), 12 deletions(-)

    diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
    index 57bbf90..823669e 100644
    --- a/fs/pstore/platform.c
    +++ b/fs/pstore/platform.c
    @@ -90,18 +90,17 @@ static void pstore_dump(struct kmsg_dumper *dumper,
    int hsize, ret;
    unsigned int part = 1;
    unsigned long flags = 0;
    - int is_locked = 0;

    if (reason < ARRAY_SIZE(reason_str))
    why = reason_str[reason];
    else
    why = "Unknown";

    - if (in_nmi()) {
    - is_locked = spin_trylock(&psinfo->buf_lock);
    - if (!is_locked)
    - pr_err("pstore dump routine blocked in NMI, may corrupt error record\n");
    - } else
    + /*
    + * pstore_dump() is serialized in panic path.
    + * So, we don't need to take any locks.
    + */
    + if (reason != KMSG_DUMP_PANIC)
    spin_lock_irqsave(&psinfo->buf_lock, flags);
    oopscount++;
    while (total < kmsg_bytes) {
    @@ -131,10 +130,7 @@ static void pstore_dump(struct kmsg_dumper *dumper,
    total += l1_cpy + l2_cpy;
    part++;
    }
    - if (in_nmi()) {
    - if (is_locked)
    - spin_unlock(&psinfo->buf_lock);
    - } else
    + if (reason != KMSG_DUMP_PANIC)
    spin_unlock_irqrestore(&psinfo->buf_lock, flags);
    }

    diff --git a/kernel/printk.c b/kernel/printk.c
    index 1455a0d..bc5ac61 100644
    --- a/kernel/printk.c
    +++ b/kernel/printk.c
    @@ -1732,13 +1732,22 @@ void kmsg_dump(enum kmsg_dump_reason reason)
    unsigned long l1, l2;
    unsigned long flags;

    + WARN_ON(in_nmi() && reason != KMSG_DUMP_PANIC);
    +
    /* Theoretically, the log could move on after we do this, but
    there's not a lot we can do about that. The new messages
    will overwrite the start of what we dump. */
    - raw_spin_lock_irqsave(&logbuf_lock, flags);
    +
    + /*
    + * kmsg_dump(KMSG_DUMP_PANIC) is serialized.
    + * So, we don't need to take any locks.
    + */
    + if (reason != KMSG_DUMP_PANIC)
    + raw_spin_lock_irqsave(&logbuf_lock, flags);
    end = log_end & LOG_BUF_MASK;
    chars = logged_chars;
    - raw_spin_unlock_irqrestore(&logbuf_lock, flags);
    + if (reason != KMSG_DUMP_PANIC)
    + raw_spin_unlock_irqrestore(&logbuf_lock, flags);

    if (chars > end) {
    s1 = log_buf + log_buf_len - chars + end;
    --
    1.7.1




    \
     
     \ /
      Last update: 2011-12-02 23:13    [W:0.026 / U:33.240 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site