Re: [PATCH] workqueue: Implement delayed_work_busy()

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

 



Hello,

cc'ing ACPI folks.

On Wed, May 24, 2017 at 10:16:02PM +0200, Alex Naidis wrote:
> 2017-05-24 22:01 GMT+02:00 Tejun Heo <tj@xxxxxxxxxx>:
> > Hello, Alex.
> >
> > On Wed, May 24, 2017 at 09:58:19PM +0200, Alex Naidis wrote:
> >> Yeah, I agree, it is wrong to rely on work_busy() providing correct data.
> >> However sometimes it is useful to have an indicator like this to at least
> >> catch some cases where requeuing work would be obsolete.
> >> This applies for delayed work too.
> >
> > Can you elaborage on "requeueing work would be obsolate" a bit?
> Sure.
> The case that I mean is represented well by the usage of work_busy here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/acpi/bus.c#n534
> It checks work_busy() before requeuing the work, since in some cases
> it is obsolete
> to requeue the work if it is already pending or running.

So, there is no point in testing whether a work item is pending before
queeing it.  It's completely spurious.  The only part which makes
sense is if it doesn't want to queue a work item if it's already
running (but note that this isn't synchronized properly) for some
reason, but given that it can race with workqueue operation and
spuriously return false when the work item just started running, it
can't be for correctness.

Rafael, Len, can you please explain why work_busy() test is there?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux