2017-03-07 20:07:25

by Mark Brown

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

On Mon, Feb 27, 2017 at 08:17:10PM +0100, Harald Geyer wrote:
> Mark Brown writes:

> > I'd need to figure out exactly what the restrictions are and like I say
> > the name of the function itself is confusing, I suspect because it
> > predates SMP.

> I guess you know that, but just to avoid any confusion: The bug in the
> regulator code is not related to SMP at all.

It's not? Oh. I had formed the impression that this was a race
condition.

> > Actually yes, though not immediately. Another option is to just rename
> > the current function and all the callers en masse then add a new, safe
> > mod_delayed_work().

> Okay by me. I'm removing the issue from my todo list and hand it over
> to you and your minions ... :)

Well, we need to figure out what the desired solution is!


Attachments:
(No filename) (788.00 B)
signature.asc (488.00 B)
Download all attachments

2017-03-10 18:44:39

by Harald Geyer

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

Mark Brown writes:
> On Mon, Feb 27, 2017 at 08:17:10PM +0100, Harald Geyer wrote:
> > Mark Brown writes:
>
> > > I'd need to figure out exactly what the restrictions are and like I say
> > > the name of the function itself is confusing, I suspect because it
> > > predates SMP.
>
> > I guess you know that, but just to avoid any confusion: The bug in the
> > regulator code is not related to SMP at all.
>
> It's not? Oh. I had formed the impression that this was a race
> condition.

I have used the word "race" in the description of patch 2/2 - sorry if
that was the wrong word, I'm trained in physics not computer science.
The description of patch 2/2 was:

> regulator: core: Fix race on multiple calls to regulator_disable_deferred()
>
> The old code has two issues:
> 1) When multiple consumers call regulator_disable_deferred() close in time,
> always the delay requested in the first call is used.
> 2) When a consumer calls regulator_disable_deferred(), but enables and
> calls regulator_disable_deferred() again before the timer fires, the timer
> isn't reset.
>
> Both issues can cause the regulator to get disabled early.

Issue (1) is what I had thought of a race - not in multiprocessing but
in multitasking. Please educate me, if this is not the case or if you can
think of a way how I could have expressed the idea more clearly.

> Well, we need to figure out what the desired solution is!

Do we have reached consensus on what to do? Do people wait for me to do some
work? Based on the comments in this thread it seems one way forward is,
that I'm resending the series with mod_fwd_delayed_work() renamed to
postpone_delayed_work(). Please let me know if you still consider
alternative approaches or need more time to evaluate the situation.

TIA,
Harald
--
If you want to support my work:
see http://friends.ccbib.org/harald/supporting/
or donate via CLAM to xASPBtezLNqj4cUe8MT5nZjthRSEjrRQXN
or via peercoin to P98LRdhit3gZbHDBe7ta5jtXrMJUms4p7w