some weird redundancy with a workqueue?
is there something i'm missing about the following snippet of code in drivers/acpi/osl.c? 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); does anyone else find that code a bit ... odd? rday -- ======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
On Fri, Oct 19, 2012 at 6:42 PM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
is there something i'm missing about the following snippet of code in drivers/acpi/osl.c?
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);
does anyone else find that code a bit ... odd?
rday
--
======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca
Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Yes it really looks odd. This should have been caught in code review before merging into mainline kernel code. I also doubt if I miss something as it got merged and lives in kernel. Thanks and Regards, Sannu K
On 19 October 2012 14:25, Sannu K <sannumail4foss@gmail.com> wrote:
On Fri, Oct 19, 2012 at 6:42 PM, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
is there something i'm missing about the following snippet of code in drivers/acpi/osl.c?
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);
does anyone else find that code a bit ... odd?
rday
--
======================================================================== Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca
Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday ========================================================================
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Yes it really looks odd. This should have been caught in code review before merging into mainline kernel code. I also doubt if I miss something as it got merged and lives in kernel.
Thanks and Regards, Sannu K
_______________________________________________ Kernelnewbies mailing list Kernelnewbies@kernelnewbies.org http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies
Hi, Yes, very odd. The commit that introduced this code have links to bugzilla. One of the entries in the bug report does have a patch [1] that uses the "if else" to do different things, but then the final commit [2] does not have them. [1] https://bugzilla.kernel.org/attachment.cgi?id=23689 [2] Commit that added this code: commit bc73675b99fd9850dd914be01d71af99c5d2a1ae Author: Zhang Rui <rui.zhang@intel.com> Date: Mon Mar 22 15:48:54 2010 +0800 ACPI: fixes a false alarm from lockdep fixes a false alarm from lockdep, as acpi hotplug workqueue waits other workqueues. http://bugzilla.kernel.org/show_bug.cgi?id=14553 https://bugzilla.kernel.org/show_bug.cgi?id=15521 Original-patch-from: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Shaohua Li <shaohua.li@intel.com> Signed-off-by: Zhang Rui <rui.zhang@intel.com> Signed-off-by: Len Brown <len.brown@intel.com> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 8e6d866..900da68 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -758,7 +758,14 @@ static acpi_status __acpi_os_execute(acpi_execute_type type, 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(&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); + ret = queue_work(queue, &dpc->work); if (!ret) { -Filipe
participants (3)
-
Filipe Rinaldi -
Robert P. J. Day -
Sannu K