From 06a8566bcf5cf7db9843a82cde7a33c7bf3947d9 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:13:23 +0800 Subject: ACPI / IPMI: Fix atomic context requirement of ipmi_msg_handler() This patch fixes the issues indicated by the test results that ipmi_msg_handler() is invoked in atomic context. BUG: scheduling while atomic: kipmi0/18933/0x10000100 Modules linked in: ipmi_si acpi_ipmi ... CPU: 3 PID: 18933 Comm: kipmi0 Tainted: G AW 3.10.0-rc7+ #2 Hardware name: QCI QSSC-S4R/QSSC-S4R, BIOS QSSC-S4R.QCI.01.00.0027.070120100606 07/01/2010 ffff8838245eea00 ffff88103fc63c98 ffffffff814c4a1e ffff88103fc63ca8 ffffffff814bfbab ffff88103fc63d28 ffffffff814c73e0 ffff88103933cbd4 0000000000000096 ffff88103fc63ce8 ffff88102f618000 ffff881035c01fd8 Call Trace: [] dump_stack+0x19/0x1b [] __schedule_bug+0x46/0x54 [] __schedule+0x83/0x59c [] __cond_resched+0x22/0x2d [] _cond_resched+0x14/0x1d [] mutex_lock+0x11/0x32 [] ? __default_send_IPI_dest_field.constprop.0+0x53/0x58 [] ipmi_msg_handler+0x23/0x166 [ipmi_si] [] deliver_response+0x55/0x5a [] handle_new_recv_msgs+0xb67/0xc65 [] ? read_tsc+0x9/0x19 [] ? _raw_spin_lock_irq+0xa/0xc [] ipmi_thread+0x5c/0x146 [ipmi_si] ... Also Tony Camuso says: 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: [] 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 fix implemented by this change has been tested by Tony: Tested the patch in a boot loop with lockdep debug enabled and never saw the problem in over 400 reboots. Reported-and-tested-by: Tony Camuso Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki --- drivers/acpi/acpi_ipmi.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index f40acef80269..a6977e12d574 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -39,6 +39,7 @@ #include #include #include +#include MODULE_AUTHOR("Zhao Yakui"); MODULE_DESCRIPTION("ACPI IPMI Opregion driver"); @@ -57,7 +58,7 @@ struct acpi_ipmi_device { struct list_head head; /* the IPMI request message list */ struct list_head tx_msg_list; - struct mutex tx_msg_lock; + spinlock_t tx_msg_lock; acpi_handle handle; struct pnp_dev *pnp_dev; ipmi_user_t user_interface; @@ -147,6 +148,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 +179,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 +244,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 +253,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 +261,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 +381,7 @@ 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); + spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list); - mutex_unlock(&ipmi_device->tx_msg_lock); + spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); err = ipmi_request_settime(ipmi_device->user_interface, &tx_msg->addr, tx_msg->tx_msgid, @@ -413,9 +417,9 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, status = AE_OK; end_label: - mutex_lock(&ipmi_device->tx_msg_lock); + spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_del(&tx_msg->head); - mutex_unlock(&ipmi_device->tx_msg_lock); + spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); kfree(tx_msg); return status; } @@ -457,7 +461,7 @@ static void acpi_add_ipmi_device(struct acpi_ipmi_device *ipmi_device) INIT_LIST_HEAD(&ipmi_device->head); - mutex_init(&ipmi_device->tx_msg_lock); + spin_lock_init(&ipmi_device->tx_msg_lock); INIT_LIST_HEAD(&ipmi_device->tx_msg_list); ipmi_install_space_handler(ipmi_device); -- cgit v1.2.3 From 8a61e12e84597b5f8155ac91b44dea866ccfaac2 Mon Sep 17 00:00:00 2001 From: Yinghai Lu Date: Fri, 20 Sep 2013 10:43:56 -0700 Subject: acpi-cpufreq: skip loading acpi_cpufreq after intel_pstate If the hw supports intel_pstate and acpi_cpufreq, intel_pstate will get loaded first. acpi_cpufreq_init() will call acpi_cpufreq_early_init() and that will allocate perf data and init those perf data in ACPI core, (that will cover all CPUs). But later it will free them as cpufreq_register_driver(acpi_cpufreq) will fail as intel_pstate is already registered Use cpufreq_get_current_driver() to check if we can skip the acpi_cpufreq loading. Signed-off-by: Yinghai Lu Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/acpi-cpufreq.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/cpufreq/acpi-cpufreq.c b/drivers/cpufreq/acpi-cpufreq.c index a1260b4549db..d2c3253e015e 100644 --- a/drivers/cpufreq/acpi-cpufreq.c +++ b/drivers/cpufreq/acpi-cpufreq.c @@ -986,6 +986,10 @@ static int __init acpi_cpufreq_init(void) { int ret; + /* don't keep reloading if cpufreq_driver exists */ + if (cpufreq_get_current_driver()) + return 0; + if (acpi_disabled) return 0; -- cgit v1.2.3 From 26ca8694344af4c833d22590c5b77d6b9cff0722 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Fri, 20 Sep 2013 22:37:31 +0530 Subject: cpufreq: check cpufreq driver is valid and cpufreq isn't disabled in cpufreq_get() cpufreq_get() can be called from external drivers which might not be aware if cpufreq driver is registered or not. And so we should actually check if cpufreq driver is registered or not and also if cpufreq is active or disabled, at the beginning of cpufreq_get(). Otherwise call to lock_policy_rwsem_read() might hit BUG_ON(!policy). Reported-and-tested-by: Linus Walleij Signed-off-by: Viresh Kumar Reviewed-by: Srivatsa S. Bhat Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/cpufreq.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers') diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 89b3c52cd5c3..04548f7023af 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1460,6 +1460,9 @@ unsigned int cpufreq_get(unsigned int cpu) { unsigned int ret_freq = 0; + if (cpufreq_disabled() || !cpufreq_driver) + return -ENOENT; + if (!down_read_trylock(&cpufreq_rwsem)) return 0; -- cgit v1.2.3 From 116decb7e48b46a8c171bd47f377fa527067f788 Mon Sep 17 00:00:00 2001 From: Sachin Kamat Date: Wed, 18 Sep 2013 10:44:53 +0530 Subject: cpufreq: exynos5440: Fix potential NULL pointer dereference If 'dvfs_info' is NULL (due to devm_kzalloc failure) the failure error message would try to dereference it. Use 'pdev' instead. Signed-off-by: Sachin Kamat Acked-by: Viresh Kumar Signed-off-by: Rafael J. Wysocki --- drivers/cpufreq/exynos5440-cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/cpufreq/exynos5440-cpufreq.c b/drivers/cpufreq/exynos5440-cpufreq.c index d514c152fd1a..be5380ecdcd4 100644 --- a/drivers/cpufreq/exynos5440-cpufreq.c +++ b/drivers/cpufreq/exynos5440-cpufreq.c @@ -457,7 +457,7 @@ err_free_table: opp_free_cpufreq_table(dvfs_info->dev, &dvfs_info->freq_table); err_put_node: of_node_put(np); - dev_err(dvfs_info->dev, "%s: failed initialization\n", __func__); + dev_err(&pdev->dev, "%s: failed initialization\n", __func__); return ret; } -- cgit v1.2.3 From b27b14cebfdb687e6d281d2cf82858a30796dc2a Mon Sep 17 00:00:00 2001 From: Hanjun Guo Date: Sun, 22 Sep 2013 15:42:41 +0800 Subject: ACPI / scan: fix typo in comments of acpi_bus_unregister_driver() "APIC" should be "ACPI" here. Signed-off-by: Hanjun Guo Signed-off-by: Rafael J. Wysocki --- drivers/acpi/scan.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index fbdb82e70d10..611ce9061dc5 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1121,7 +1121,7 @@ int acpi_bus_register_driver(struct acpi_driver *driver) EXPORT_SYMBOL(acpi_bus_register_driver); /** - * acpi_bus_unregister_driver - unregisters a driver with the APIC bus + * acpi_bus_unregister_driver - unregisters a driver with the ACPI bus * @driver: driver to unregister * * Unregisters a driver with the ACPI bus. Searches the namespace for all -- cgit v1.2.3