Messages in this thread | | | Date | Wed, 15 Apr 2015 13:35:15 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH 1/3] hrtimer: Fix race between hrtimer_start() and __run_hrtimer() |
| |
On Wed, 15 Apr 2015, Peter Zijlstra wrote: > On Wed, Apr 15, 2015 at 12:26:58PM +0200, Thomas Gleixner wrote: > > On Wed, 15 Apr 2015, Peter Zijlstra wrote: > > > hrtimer: Fix race between hrtimer_start() and __run_hrtimer() > > > > I don't think that subject line is correct. > > > > Back in the early hrtimer days we made deliberately the design > > decision that this kind of usage is forbidden. The reason for this is > > that the hrtimer infrastructure cannot provide proper > > serialization. So we thought it would be a sane restruction that > > restarting a timer from the callback should not be mixed with > > concurrent restarts from a different call site. > > Ah I was not aware. Until I changed the locking it was possible simply > because everything was serialized by the base lock. So the concurrent > start would either land before the callback or after it but not in the > middle like it can now. > > > So I rather prefer a subject line like this > > > > hrtimer: Allow concurrent hrtimer_start() for self restarting timers > > > > /me copy/paste, done! :-) > > > > To that effect, add a WARN when someone tries to forward an already > > > enqueued timer. > > > > The warnon itself is nice, but what about sites which use > > hrtimer_set_expires() and hrtimer_start_expires()? > > They are all inlines, furthermore forward is the most common way to > change the expiry of periodic / self restarting timers so would gain us > most.
Right. I just wanted to mention it.
> How about this then?
Looks good. Should I add those 3 patches to the other pile of hrtimer stuff?
Thanks,
tglx
| |