Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1594596 > unrolled thread

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

Started byMark Brown <broonie@kernel.org>
First post2017-03-07 21:10 +0100
Last post2017-03-10 19:50 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH 1/2] workqueue: Add new function mod_fwd_delayed_work() Mark Brown <broonie@kernel.org> - 2017-03-07 21:10 +0100
    Re: [PATCH 1/2] workqueue: Add new function mod_fwd_delayed_work() Harald Geyer <harald@ccbib.org> - 2017-03-10 19:50 +0100

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

FromMark Brown <broonie@kernel.org>
Date2017-03-07 21:10 +0100
SubjectRe: [PATCH 1/2] workqueue: Add new function mod_fwd_delayed_work()
Message-ID<titAK-46W-19@gated-at.bofh.it>

[Multipart message — attachments visible in raw view] — view raw

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!

[toc] | [next] | [standalone]


#1598024

FromHarald Geyer <harald@ccbib.org>
Date2017-03-10 19:50 +0100
Message-ID<tjxLY-7Vp-13@gated-at.bofh.it>
In reply to#1594596
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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web