Messages in this thread Patch in this message | | | Date | Thu, 16 Mar 2006 02:05:37 +0100 (CET) | From | Roman Zippel <> | Subject | Re: [patch 5/8] hrtimer remove state field |
| |
Hi,
On Sun, 12 Mar 2006, Thomas Gleixner wrote:
> But the problem I described now happened with the current patch queue - > without the hrtimer_active() check. I have no direct access to the > machine which lets this surface and I just tried to reconstruct the > scenario from the sparse information which was provided by the customer. > All I can tell, that it is related to something similar and a requeue > happens where none should happen.
I have an idea what might have happened. You don't advance the pending state, if the signal isn't queued, so that the pending state is screwed up afterwards. Although I don't see how it could crash the kernel (it has only the potential to mess up the timer queue via hrtimer_forward() a bit), but I don't know what other patches were applied.
> I make the check a BUG_ON(!hrtimer_active(timer) so it might show up in > -mm again. Ok ?
That's fine. The point is that it's currently not needed to allow the user this behaviour. It's a bit unfortunate that such API details were not discussed before it got merged - users have a tendency to (ab)use a system in the least expected way (especially if they somehow gotten the idea it's much better than the old stuff in every way). For example no current user restarts an active timer, which could be used to simplify the locking. If we tightened a bit what a user is allowed to do, we could gain flexibility on the other side, e.g. allow drivers to create timer sources or how to integrate cpu timer.
bye, Roman
Signed-off-by: Roman Zippel <zippel@linux-m68k.org>
Index: linux-2.6-git/kernel/posix-timers.c =================================================================== --- linux-2.6-git.orig/kernel/posix-timers.c 2006-03-14 19:48:21.000000000 +0100 +++ linux-2.6-git/kernel/posix-timers.c 2006-03-15 20:54:38.000000000 +0100 @@ -353,6 +353,7 @@ hrtimer_forward(&timr->it.real.timer, timr->it.real.interval); ret = HRTIMER_RESTART; + ++timr->it_requeue_pending; } } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |