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

[Mark Brown]

On Mon, Mar 06, 2017 at 05:22:12PM -0500, Tejun Heo wrote:
> On Thu, Feb 23, 2017 at 09:34:49AM -0800, Mark Brown wrote:

> > 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.

> What the mod function is implemneting is "whoever wins (runs the last)
> determines the timeout".  It is a bit unwiedly because unlike the
> timer, workqueue delay takes duration instead of the absoulte time
> making coordinating from the user side trickier.

Ah, OK.  That's what the regulator API was looking for (the timeout is
always the same, we just want to kick the starting point into the
future).  We're not trying to reduce timeouts, only increase them.

> > 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.

> I don't think it's a matter of "fixing" the existing
> mod_delayed_work().  What the new function is implementing wouldn't
> fit use cases where the timeout should only be shortened (IIRC,
> writeback code does that).

> I'm not against adding new interface to handle it better but I think
> it makes more sense to add both directions.  How about adding
> expedite_delayed_work_on() and postpone_delayed_work_on()?

That works for me.

Attachment: signature.asc
Description: PGP signature