Re: some weird redundancy with a workqueue?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 19 October 2012 14:25, Sannu K <sannumail4foss@xxxxxxxxx> wrote:
> On Fri, Oct 19, 2012 at 6:42 PM, Robert P. J. Day <rpjday@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxx
>> 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@xxxxxxxxxxxxxxxxx
> 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@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Shaohua Li <shaohua.li@xxxxxxxxx>
    Signed-off-by: Zhang Rui <rui.zhang@xxxxxxxxx>
    Signed-off-by: Len Brown <len.brown@xxxxxxxxx>

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

_______________________________________________
Kernelnewbies mailing list
Kernelnewbies@xxxxxxxxxxxxxxxxx
http://lists.kernelnewbies.org/mailman/listinfo/kernelnewbies


[Index of Archives]     [Newbies FAQ]     [Linux Kernel Mentors]     [Linux Kernel Development]     [IETF Annouce]     [Git]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux SCSI]     [Linux ACPI]
  Powered by Linux