Date: Mon, 6 Mar 2017 17:22:12 -0500
From: Tejun Heo <tj@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Harald Geyer <harald@ccbib.org>, Liam Girdwood <lgirdwood@gmail.com>,
        Lai Jiangshan <jiangshanlai@gmail.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] workqueue: Add new function mod_fwd_delayed_work()
Message-ID: <20170306222212.GM26127@htj.duckdns.org>
References: <1487785285-3567-1-git-send-email-harald@ccbib.org>
 <20170222182111.4jajk2ed52okx323@sirena.org.uk>
 <E1cgdBA-0000LS-VU@stardust.g4.wien.funkfeuer.at>
 <20170223173449.c747nrfr3oxrjrr7@sirena.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170223173449.c747nrfr3oxrjrr7@sirena.org.uk>
User-Agent: Mutt/1.7.1 (2016-10-04)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1652
Lines: 37

Hello,

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.

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

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()?

Thanks.

-- 
tejun