Messages in this thread Patch in this message | | | From | Seiji Aguchi <> | Date | Fri, 2 Dec 2011 17:10:26 -0500 | 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
| |