Messages in this thread |  | | Date | Mon, 8 Jun 2015 22:52:46 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 0/3] hrtimer: HRTIMER_STATE_ fixes |
| |
On 06/08, Thomas Gleixner wrote: > > On Mon, 8 Jun 2015, Peter Zijlstra wrote: > > On Mon, 2015-06-08 at 17:10 +0200, Oleg Nesterov wrote: > > > On 06/08, Peter Zijlstra wrote: > > > > > > > > > I tend to agree, but I think its a pre-existing problem, not one > > > > > introduced by my proposed patch. > > > > > > > > Something like this would fix that I think. It fully preserves > > > > timer->state over hrtimer_start_range_ns(). > > > > > > Yes, but I think we can do a bit better. > > > > > > Only for initial review, I need to re-check this... > > > > I'm having a wee bit of bother spotting how you version is 'better'. > > > > > And. I think that after you remove STATE_CALLBACK we can even kill > > > timer->state altogether, but this is another story. > > > > Ah, yes, we could introduce timerqueue_is_queued() which uses > > RB_EMPTY_NODE(). Obviating the need for hrtimer::state entirely. > > Which won't work for the migration case unless we have some trickery > like we do with double linked lists (not setting the prev member to > NULL on dequeue).
Of course, but this is trivial, no? Nevermind, I could easily miss somthing and right now this is off-topic.
What do you think about this series? To me it makes sense in any case. But I need (at least)to update the changelogs. In particular 3/3 doesn't explain why do we need this change. If you missed the previous discussion, this (hopefully) fixes the problem with the auto-rearming timers, the "random" hrtimer_restart() wrongly creates a window when this timer looks as !hrtimer_inactive().
Peter, I tried to think again about ->running and all I can say is that I am totally confused ;) I'll try to write another email tomorrow.
Oleg.
|  |