Re: [patch 5/8] hrtimer remove state field

[Roman Zippel <zippel@linux-m68k.org>]

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/