From: "Luck, Tony" <tony.luck@intel.com>
To: linux-kernel@vger.kernel.org
Cc: "Don Zickus" <dzickus@redhat.com>, "Matthew Garrett" <mjg@redhat.com>
Subject: pstore: change mutex locking to spin_locks
Date: Fri, 12 Aug 2011 10:54:51 -0700 [thread overview]
Message-ID: <4e4568eb10165cbab6@agluck-desktop.sc.intel.com> (raw)
From: Don Zickus <dzickus@redhat.com>
pstore was using mutex locking to protect read/write access to the
backend plug-ins. This causes problems when pstore is executed in
an NMI context through panic() -> kmsg_dump().
This patch changes the mutex to a spin_lock_irqsave then also checks to
see if we are in an NMI context. If we are in an NMI and can't get the
lock, just print a message stating that and blow by the locking.
All this is probably a hack around the bigger locking problem but it
solves my current situation of trying to sleep in an NMI context.
Tested by loading the lkdtm module and executing a HARDLOCKUP which
will cause the machine to panic inside the nmi handler.
Signed-off-by: Don Zickus <dzickus@redhat.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
Essentially the same patch that Don posted on July 20th. I just added the
efivars.c bit, and did some minor formatting changes to make it fit on
top of the "defer inserting OOPs entries into pstore" patch that I posted
yesterday. I plan to add both patches to linux-next soon, and will send
to Linus in the 3.2 merge window.
diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c
index 2ca59dc..5e820ea 100644
--- a/drivers/acpi/apei/erst.c
+++ b/drivers/acpi/apei/erst.c
@@ -1165,7 +1165,7 @@ static int __init erst_init(void)
goto err_release_erange;
buf = kmalloc(erst_erange.size, GFP_KERNEL);
- mutex_init(&erst_info.buf_mutex);
+ spin_lock_init(&erst_info.buf_lock);
if (buf) {
erst_info.buf = buf + sizeof(struct cper_pstore_record);
erst_info.bufsize = erst_erange.size -
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index eb80b54..be8bcb0 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -978,7 +978,7 @@ int register_efivars(struct efivars *efivars,
if (efivars->efi_pstore_info.buf) {
efivars->efi_pstore_info.bufsize = 1024;
efivars->efi_pstore_info.data = efivars;
- mutex_init(&efivars->efi_pstore_info.buf_mutex);
+ spin_lock_init(&efivars->efi_pstore_info.buf_lock);
pstore_register(&efivars->efi_pstore_info);
}
diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c
index ca60ebc..0472924 100644
--- a/fs/pstore/platform.c
+++ b/fs/pstore/platform.c
@@ -28,6 +28,7 @@
#include <linux/timer.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
+#include <linux/hardirq.h>
#include <linux/workqueue.h>
#include "internal.h"
@@ -88,13 +89,20 @@ static void pstore_dump(struct kmsg_dumper *dumper,
u64 id;
int hsize;
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";
- mutex_lock(&psinfo->buf_mutex);
+ 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
+ spin_lock_irqsave(&psinfo->buf_lock, flags);
oopscount++;
while (total < kmsg_bytes) {
dst = psinfo->buf;
@@ -123,7 +131,11 @@ static void pstore_dump(struct kmsg_dumper *dumper,
total += l1_cpy + l2_cpy;
part++;
}
- mutex_unlock(&psinfo->buf_mutex);
+ if (in_nmi()) {
+ if (is_locked)
+ spin_unlock(&psinfo->buf_lock);
+ } else
+ spin_unlock_irqrestore(&psinfo->buf_lock, flags);
}
static struct kmsg_dumper pstore_dumper = {
@@ -188,11 +200,12 @@ void pstore_get_records(int quiet)
enum pstore_type_id type;
struct timespec time;
int failed = 0, rc;
+ unsigned long flags;
if (!psi)
return;
- mutex_lock(&psinfo->buf_mutex);
+ spin_lock_irqsave(&psinfo->buf_lock, flags);
rc = psi->open(psi);
if (rc)
goto out;
@@ -205,7 +218,7 @@ void pstore_get_records(int quiet)
}
psi->close(psi);
out:
- mutex_unlock(&psinfo->buf_mutex);
+ spin_unlock_irqrestore(&psinfo->buf_lock, flags);
if (failed)
printk(KERN_WARNING "pstore: failed to load %d record(s) from '%s'\n",
@@ -233,7 +246,8 @@ static void pstore_timefunc(unsigned long dummy)
*/
int pstore_write(enum pstore_type_id type, char *buf, size_t size)
{
- u64 id;
+ u64 id;
+ unsigned long flags;
if (!psinfo)
return -ENODEV;
@@ -241,13 +255,13 @@ int pstore_write(enum pstore_type_id type, char *buf, size_t size)
if (size > psinfo->bufsize)
return -EFBIG;
- mutex_lock(&psinfo->buf_mutex);
+ spin_lock_irqsave(&psinfo->buf_lock, flags);
memcpy(psinfo->buf, buf, size);
id = psinfo->write(type, 0, size, psinfo);
if (pstore_is_mounted())
pstore_mkfile(PSTORE_TYPE_DMESG, psinfo->name, id, psinfo->buf,
size, CURRENT_TIME, psinfo);
- mutex_unlock(&psinfo->buf_mutex);
+ spin_unlock_irqrestore(&psinfo->buf_lock, flags);
return 0;
}
diff --git a/include/linux/pstore.h b/include/linux/pstore.h
index cc03bbf..b91440e 100644
--- a/include/linux/pstore.h
+++ b/include/linux/pstore.h
@@ -32,7 +32,7 @@ enum pstore_type_id {
struct pstore_info {
struct module *owner;
char *name;
- struct mutex buf_mutex; /* serialize access to 'buf' */
+ spinlock_t buf_lock; /* serialize access to 'buf' */
char *buf;
size_t bufsize;
int (*open)(struct pstore_info *psi);
next reply other threads:[~2011-08-12 17:54 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-08-12 17:54 Luck, Tony [this message]
2011-08-12 17:59 ` pstore: change mutex locking to spin_locks Matthew Garrett
2011-08-17 21:22 ` Andrew Morton
2011-08-17 21:47 ` Peter Zijlstra
2011-08-18 13:04 ` Don Zickus
2011-08-18 15:26 ` Peter Zijlstra
2011-08-18 12:58 ` Don Zickus
2011-08-18 14:37 ` Peter Zijlstra
2011-08-18 16:33 ` Luck, Tony
2011-08-18 17:25 ` Don Zickus
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4e4568eb10165cbab6@agluck-desktop.sc.intel.com \
--to=tony.luck@intel.com \
--cc=dzickus@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mjg@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox