* [PATCH 2/2]ACPI: assign different lockdep key for acpi works
@ 2009-12-11 3:04 Shaohua Li
2009-12-11 21:57 ` Andrew Morton
0 siblings, 1 reply; 3+ messages in thread
From: Shaohua Li @ 2009-12-11 3:04 UTC (permalink / raw)
To: linux-kernel, linux-acpi; +Cc: lenb, akpm, mingo
Differentiate works for three different workqueue in ACPI. This fixes
a fase alarm from lockdep, as acpi hotplug workqueue waits other
workqueues.
http://bugzilla.kernel.org/show_bug.cgi?id=14553
Signed-off-by: Shaohua Li <shaohua.li@intel.com>
---
drivers/acpi/osl.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
Index: linux/drivers/acpi/osl.c
===================================================================
--- linux.orig/drivers/acpi/osl.c 2009-12-10 10:40:15.000000000 +0800
+++ linux/drivers/acpi/osl.c 2009-12-10 10:43:25.000000000 +0800
@@ -729,6 +729,8 @@ static acpi_status __acpi_os_execute(acp
struct acpi_os_dpc *dpc;
struct workqueue_struct *queue;
int ret;
+ static struct lock_class_key hp_key, notify_key, acpid_key;
+
ACPI_DEBUG_PRINT((ACPI_DB_EXEC,
"Scheduling function [%p(%p)] for deferred execution.\n",
function, context));
@@ -758,7 +760,15 @@ static acpi_status __acpi_os_execute(acp
queue = hp ? kacpi_hotplug_wq :
(type == OSL_NOTIFY_HANDLER ? kacpi_notify_wq : kacpid_wq);
dpc->wait = hp ? 1 : 0;
- INIT_WORK(&dpc->work, acpi_os_execute_deferred);
+ if (queue == kacpi_hotplug_wq)
+ INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred,
+ hp_key, "acpi_hp_work");
+ else if (queue == kacpi_notify_wq)
+ INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred,
+ notify_key, "acpi_notify_work");
+ else
+ INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred,
+ acpid_key, "acpi_acpid_work");
ret = queue_work(queue, &dpc->work);
if (!ret) {
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 2/2]ACPI: assign different lockdep key for acpi works 2009-12-11 3:04 [PATCH 2/2]ACPI: assign different lockdep key for acpi works Shaohua Li @ 2009-12-11 21:57 ` Andrew Morton 2009-12-14 1:08 ` Shaohua Li 0 siblings, 1 reply; 3+ messages in thread From: Andrew Morton @ 2009-12-11 21:57 UTC (permalink / raw) To: Shaohua Li; +Cc: linux-kernel, linux-acpi, lenb, mingo On Fri, 11 Dec 2009 11:04:58 +0800 Shaohua Li <shaohua.li@intel.com> wrote: > Differentiate works for three different workqueue in ACPI. This fixes > a fase alarm from lockdep, as acpi hotplug workqueue waits other > workqueues. > > http://bugzilla.kernel.org/show_bug.cgi?id=14553 > > Signed-off-by: Shaohua Li <shaohua.li@intel.com> > --- > drivers/acpi/osl.c | 12 +++++++++++- > 1 file changed, 11 insertions(+), 1 deletion(-) > > Index: linux/drivers/acpi/osl.c > =================================================================== > --- linux.orig/drivers/acpi/osl.c 2009-12-10 10:40:15.000000000 +0800 > +++ linux/drivers/acpi/osl.c 2009-12-10 10:43:25.000000000 +0800 > @@ -729,6 +729,8 @@ static acpi_status __acpi_os_execute(acp > struct acpi_os_dpc *dpc; > struct workqueue_struct *queue; > int ret; > + static struct lock_class_key hp_key, notify_key, acpid_key; > + > ACPI_DEBUG_PRINT((ACPI_DB_EXEC, > "Scheduling function [%p(%p)] for deferred execution.\n", > function, context)); > @@ -758,7 +760,15 @@ static acpi_status __acpi_os_execute(acp > queue = hp ? kacpi_hotplug_wq : > (type == OSL_NOTIFY_HANDLER ? kacpi_notify_wq : kacpid_wq); > dpc->wait = hp ? 1 : 0; > - INIT_WORK(&dpc->work, acpi_os_execute_deferred); > + if (queue == kacpi_hotplug_wq) > + INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred, > + hp_key, "acpi_hp_work"); > + else if (queue == kacpi_notify_wq) > + INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred, > + notify_key, "acpi_notify_work"); > + else > + INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred, > + acpid_key, "acpi_acpid_work"); > ret = queue_work(queue, &dpc->work); > > if (!ret) { Yes, that's a shortcoming in the present INIT_WORK(). However I think you can solve this more simply, without introducing INIT_WORK_KEY(): if (queue == kacpi_hotplug_wq) INIT_WORK(&dpc->work, acpi_os_execute_deferred); else if (queue == kacpi_notify_wq) INIT_WORK(&dpc->work, acpi_os_execute_deferred); else INIT_WORK(&dpc->work, acpi_os_execute_deferred); it looks silly, but I think the compiler will do the right thing with it - you'll get a separate lock_class_key at each site. Obviously it would need an explanatory comment. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/2]ACPI: assign different lockdep key for acpi works 2009-12-11 21:57 ` Andrew Morton @ 2009-12-14 1:08 ` Shaohua Li 0 siblings, 0 replies; 3+ messages in thread From: Shaohua Li @ 2009-12-14 1:08 UTC (permalink / raw) To: Andrew Morton Cc: linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, lenb@kernel.org, mingo@elte.hu On Sat, Dec 12, 2009 at 05:57:16AM +0800, Andrew Morton wrote: > On Fri, 11 Dec 2009 11:04:58 +0800 > Shaohua Li <shaohua.li@intel.com> wrote: > > > Differentiate works for three different workqueue in ACPI. This fixes > > a fase alarm from lockdep, as acpi hotplug workqueue waits other > > workqueues. > > > > http://bugzilla.kernel.org/show_bug.cgi?id=14553 > > > > Signed-off-by: Shaohua Li <shaohua.li@intel.com> > > --- > > drivers/acpi/osl.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > Index: linux/drivers/acpi/osl.c > > =================================================================== > > --- linux.orig/drivers/acpi/osl.c 2009-12-10 10:40:15.000000000 +0800 > > +++ linux/drivers/acpi/osl.c 2009-12-10 10:43:25.000000000 +0800 > > @@ -729,6 +729,8 @@ static acpi_status __acpi_os_execute(acp > > struct acpi_os_dpc *dpc; > > struct workqueue_struct *queue; > > int ret; > > + static struct lock_class_key hp_key, notify_key, acpid_key; > > + > > ACPI_DEBUG_PRINT((ACPI_DB_EXEC, > > "Scheduling function [%p(%p)] for deferred execution.\n", > > function, context)); > > @@ -758,7 +760,15 @@ static acpi_status __acpi_os_execute(acp > > queue = hp ? kacpi_hotplug_wq : > > (type == OSL_NOTIFY_HANDLER ? kacpi_notify_wq : kacpid_wq); > > dpc->wait = hp ? 1 : 0; > > - INIT_WORK(&dpc->work, acpi_os_execute_deferred); > > + if (queue == kacpi_hotplug_wq) > > + INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred, > > + hp_key, "acpi_hp_work"); > > + else if (queue == kacpi_notify_wq) > > + INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred, > > + notify_key, "acpi_notify_work"); > > + else > > + INIT_WORK_KEY(&dpc->work, acpi_os_execute_deferred, > > + acpid_key, "acpi_acpid_work"); > > ret = queue_work(queue, &dpc->work); > > > > if (!ret) { > > Yes, that's a shortcoming in the present INIT_WORK(). However I think > you can solve this more simply, without introducing INIT_WORK_KEY(): > > if (queue == kacpi_hotplug_wq) > INIT_WORK(&dpc->work, acpi_os_execute_deferred); > else if (queue == kacpi_notify_wq) > INIT_WORK(&dpc->work, acpi_os_execute_deferred); > else > INIT_WORK(&dpc->work, acpi_os_execute_deferred); > > it looks silly, but I think the compiler will do the right thing with it > - you'll get a separate lock_class_key at each site. > > Obviously it would need an explanatory comment. Yes, I tried this way before, it centainly works. I just thought maybe others need similar API. If I'm over thinking, let's take your way. Thanks, Shaohua ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-12-14 1:08 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-12-11 3:04 [PATCH 2/2]ACPI: assign different lockdep key for acpi works Shaohua Li 2009-12-11 21:57 ` Andrew Morton 2009-12-14 1:08 ` Shaohua Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox