[PATCH 1/1] drivers/acpi: acpi_ipmi.c replace mutex with spin_lock_irqsave
From: tcam...@redhat.com
From: Tony Camuso
We were getting occasional "Scheduling while atomic" call traces
during boot on some systems. Problem was first seen on a Cisco C210
but we were able to reproduce it on a Cisco c220m3. Setting
CONFIG_LOCKDEP and LOCKDEP_SUPPORT to 'y' exposed a lockdep around
tx_msg_lock in acpi_ipmi.c struct acpi_ipmi_device.
=
[ INFO: inconsistent lock state ]
2.6.32-415.el6.x86_64-debug-splck #1
-
inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage.
ksoftirqd/3/17 [HC0[0]:SC1[1]:HE1:SE0] takes:
(_device->tx_msg_lock){+.?...}, at: []
ipmi_msg_handler+0x71/0x126
{SOFTIRQ-ON-W} state was registered at:
[] __lock_acquire+0x63c/0x1570
[] lock_acquire+0xa4/0x120
[] __mutex_lock_common+0x4c/0x400
[] mutex_lock_nested+0x4a/0x60
[] acpi_ipmi_space_handler+0x11b/0x234
[] acpi_ev_address_space_dispatch+0x170/0x1be
The ipmi_msg_handler() function in drivers/acpi/acpi_ipmi was taking
a mutex_lock after ultimately being called from a call chain initiated
by function smi_rcv_tasklet() in drivers/char/ipmi/ipmi_msghandler.c.
Documentation/mutex-design.txt on lines 93 and 94 says, "mutexes may
not be used in hardware or software interrupt contexts such as
tasklets and timers."
The patch changes the mutex member tx_msg_lock in acpi_ipmi.c struct
acpi_ipmi_device to a spinlock_t and replaces mutex_lock/unlock pairs
with spin_lock_irqsave/unlock_irqrestore around tx_msg_lock for
accesses to that struct.
Tested the patch in a boot loop with lockdep debug enabled and never
saw the problem in over 400 reboots.
Signed-off-by: Tony Camuso
---
drivers/acpi/acpi_ipmi.c | 24 ++--
1 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index f40acef..a4b428d 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -57,7 +57,7 @@ struct acpi_ipmi_device {
struct list_head head;
/* the IPMI request message list */
struct list_head tx_msg_list;
- struct mutextx_msg_lock;
+ spinlock_t tx_msg_lock;
acpi_handle handle;
struct pnp_dev *pnp_dev;
ipmi_user_t user_interface;
@@ -147,6 +147,7 @@ static void acpi_format_ipmi_msg(struct acpi_ipmi_msg
*tx_msg,
struct kernel_ipmi_msg *msg;
struct acpi_ipmi_buffer *buffer;
struct acpi_ipmi_device *device;
+ unsigned long flags;
msg = _msg->tx_message;
/*
@@ -177,10 +178,10 @@ static void acpi_format_ipmi_msg(struct acpi_ipmi_msg
*tx_msg,
/* Get the msgid */
device = tx_msg->device;
- mutex_lock(>tx_msg_lock);
+ spin_lock_irqsave(>tx_msg_lock, flags);
device->curr_msgid++;
tx_msg->tx_msgid = device->curr_msgid;
- mutex_unlock(>tx_msg_lock);
+ spin_unlock_irqrestore(>tx_msg_lock, flags);
}
static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
@@ -242,6 +243,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
int msg_found = 0;
struct acpi_ipmi_msg *tx_msg;
struct pnp_dev *pnp_dev = ipmi_device->pnp_dev;
+ unsigned long flags;
if (msg->user != ipmi_device->user_interface) {
dev_warn(_dev->dev, "Unexpected response is returned. "
@@ -250,7 +252,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
ipmi_free_recv_msg(msg);
return;
}
- mutex_lock(_device->tx_msg_lock);
+ spin_lock_irqsave(_device->tx_msg_lock, flags);
list_for_each_entry(tx_msg, _device->tx_msg_list, head) {
if (msg->msgid == tx_msg->tx_msgid) {
msg_found = 1;
@@ -258,7 +260,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
}
}
- mutex_unlock(_device->tx_msg_lock);
+ spin_unlock_irqrestore(_device->tx_msg_lock, flags);
if (!msg_found) {
dev_warn(_dev->dev, "Unexpected response (msg id %ld) is "
"returned.\n", msg->msgid);
@@ -378,6 +380,8 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address
address,
struct acpi_ipmi_device *ipmi_device = handler_context;
int err, rem_time;
acpi_status status;
+ unsigned long flags;
+
/*
* IPMI opregion message.
* IPMI message is firstly written to the BMC and system software
@@ -395,9 +399,9 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address
address,
return AE_NO_MEMORY;
acpi_format_ipmi_msg(tx_msg, address, value);
- mutex_lock(_device->tx_msg_lock);
+ spin_lock_irqsave(_device->tx_msg_lock, flags);
list_add_tail(_msg->head, _device->tx_msg_list);
- mutex_unlock(_device->tx_msg_lock);
+ spin_unlock_irqrestore(_device->tx_msg_lock, flags);
[PATCH 1/1] drivers/acpi: acpi_ipmi.c replace mutex with spin_lock_irqsave
From: tcam...@redhat.com
From: Tony Camuso tcam...@redhat.com
We were getting occasional Scheduling while atomic call traces
during boot on some systems. Problem was first seen on a Cisco C210
but we were able to reproduce it on a Cisco c220m3. Setting
CONFIG_LOCKDEP and LOCKDEP_SUPPORT to 'y' exposed a lockdep around
tx_msg_lock in acpi_ipmi.c struct acpi_ipmi_device.
=
[ INFO: inconsistent lock state ]
2.6.32-415.el6.x86_64-debug-splck #1
-
inconsistent {SOFTIRQ-ON-W} - {IN-SOFTIRQ-W} usage.
ksoftirqd/3/17 [HC0[0]:SC1[1]:HE1:SE0] takes:
(ipmi_device-tx_msg_lock){+.?...}, at: [81337a27]
ipmi_msg_handler+0x71/0x126
{SOFTIRQ-ON-W} state was registered at:
[810ba11c] __lock_acquire+0x63c/0x1570
[810bb0f4] lock_acquire+0xa4/0x120
[815581cc] __mutex_lock_common+0x4c/0x400
[815586ea] mutex_lock_nested+0x4a/0x60
[8133789d] acpi_ipmi_space_handler+0x11b/0x234
[81321c62] acpi_ev_address_space_dispatch+0x170/0x1be
The ipmi_msg_handler() function in drivers/acpi/acpi_ipmi was taking
a mutex_lock after ultimately being called from a call chain initiated
by function smi_rcv_tasklet() in drivers/char/ipmi/ipmi_msghandler.c.
Documentation/mutex-design.txt on lines 93 and 94 says, mutexes may
not be used in hardware or software interrupt contexts such as
tasklets and timers.
The patch changes the mutex member tx_msg_lock in acpi_ipmi.c struct
acpi_ipmi_device to a spinlock_t and replaces mutex_lock/unlock pairs
with spin_lock_irqsave/unlock_irqrestore around tx_msg_lock for
accesses to that struct.
Tested the patch in a boot loop with lockdep debug enabled and never
saw the problem in over 400 reboots.
Signed-off-by: Tony Camuso tcam...@redhat.com
---
drivers/acpi/acpi_ipmi.c | 24 ++--
1 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index f40acef..a4b428d 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -57,7 +57,7 @@ struct acpi_ipmi_device {
struct list_head head;
/* the IPMI request message list */
struct list_head tx_msg_list;
- struct mutextx_msg_lock;
+ spinlock_t tx_msg_lock;
acpi_handle handle;
struct pnp_dev *pnp_dev;
ipmi_user_t user_interface;
@@ -147,6 +147,7 @@ static void acpi_format_ipmi_msg(struct acpi_ipmi_msg
*tx_msg,
struct kernel_ipmi_msg *msg;
struct acpi_ipmi_buffer *buffer;
struct acpi_ipmi_device *device;
+ unsigned long flags;
msg = tx_msg-tx_message;
/*
@@ -177,10 +178,10 @@ static void acpi_format_ipmi_msg(struct acpi_ipmi_msg
*tx_msg,
/* Get the msgid */
device = tx_msg-device;
- mutex_lock(device-tx_msg_lock);
+ spin_lock_irqsave(device-tx_msg_lock, flags);
device-curr_msgid++;
tx_msg-tx_msgid = device-curr_msgid;
- mutex_unlock(device-tx_msg_lock);
+ spin_unlock_irqrestore(device-tx_msg_lock, flags);
}
static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
@@ -242,6 +243,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
int msg_found = 0;
struct acpi_ipmi_msg *tx_msg;
struct pnp_dev *pnp_dev = ipmi_device-pnp_dev;
+ unsigned long flags;
if (msg-user != ipmi_device-user_interface) {
dev_warn(pnp_dev-dev, Unexpected response is returned.
@@ -250,7 +252,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
ipmi_free_recv_msg(msg);
return;
}
- mutex_lock(ipmi_device-tx_msg_lock);
+ spin_lock_irqsave(ipmi_device-tx_msg_lock, flags);
list_for_each_entry(tx_msg, ipmi_device-tx_msg_list, head) {
if (msg-msgid == tx_msg-tx_msgid) {
msg_found = 1;
@@ -258,7 +260,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg,
void *user_msg_data)
}
}
- mutex_unlock(ipmi_device-tx_msg_lock);
+ spin_unlock_irqrestore(ipmi_device-tx_msg_lock, flags);
if (!msg_found) {
dev_warn(pnp_dev-dev, Unexpected response (msg id %ld) is
returned.\n, msg-msgid);
@@ -378,6 +380,8 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address
address,
struct acpi_ipmi_device *ipmi_device = handler_context;
int err, rem_time;
acpi_status status;
+ unsigned long flags;
+
/*
* IPMI opregion message.
* IPMI message is firstly written to the BMC and system software
@@ -395,9 +399,9 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address
address,
return AE_NO_MEMORY;
acpi_format_ipmi_msg(tx_msg, address, value);
- mutex_lock(ipmi_device-tx_msg_lock);
+
