Re: [PATCH 1/2] workqueue: Add new function mod_fwd_delayed_work()

[Mark Brown]

On Wed, Feb 22, 2017 at 09:06:23PM +0100, Harald Geyer wrote:
> Mark Brown writes:
> > On Wed, Feb 22, 2017 at 05:41:24PM +0000, Harald Geyer wrote:
> > > Drivers calling queue_delayed_work() or mod_delayed_work() multiple times
> > > on the same work without coordination get undefined behaviour. Add a new
> > > function, which is easier to use.

> > The obvious question here, especially in the case of mod_delayed_work(),
> > is why not fix the existing functions to have the expected behaviour?

> AFAICS the existing functions behave as documented. I don't feel to be an
> authority to decide that the documented behaviour is not right. Actually
> I think that what mod_delayed_work() does, is a valid operation, even
> if many current users probably don't want it.

It is *very* non-obvious that mod_delayed_work() will have a problem
from the documentation, there's "mod_delayed_work_on() on local CPU" as
the body of the description but honestly I'm struggling to tell if
that's even there intentionally or anything other than an implementation
detail.  I'd expect to see some words describing the situations where it
can be used or something, both the name and the lack of any information
about issues suggest it's the default thing and will work safely.

> I was suprised when I found that no function like mod_fwd_delayed_work()
> existed, so you have a point there.

I suspect people are just using mod_delayed_work(), not realising that
there are restrictions.  I'm thinking that perhaps it should be fixed
to be safe for calling from different contexts and a new function with
the existing behaviour added, that seems less error prone.

Attachment: signature.asc
Description: PGP signature