From 58229a18994215bbfe0bcd1c99d2e039f30b076b Mon Sep 17 00:00:00 2001 From: Thomas Gleixner Date: Thu, 21 Jun 2007 20:45:15 +0000 Subject: [PATCH] posix-timers: Prevent softirq starvation by small intervals and SIG_IGN posix-timers which deliver an ignored signal are currently rearmed in the timer softirq: This is necessary because the timer needs to be delivered again when SIG_IGN is removed. This is not a problem, when the interval is reasonable. With high resolution timers enabled one might arm a posix timer with a very small interval and ignore the signal. This might lead to a softirq starvation when the interval is so small that the timer is requeued onto the softirq pending list right away. This problem was pointed out by Jan Kiszka. Thanks Jan ! The correct solution would be to stop the timer, when the signal is ignored and rearm it when SIG_IGN is removed. Unfortunately this requires modification in sigaction and involves non trivial sighand locking. It's too late in the release cycle for such a change. For now we just keep the timer running and enforce that the timer only fires every jiffie. This does not break anything as we keep the overrun counter correct. It adds a little inaccuracy to the timer_gettime() interface, but... The more complex change is necessary anyway to fix another short coming of the current implementation, which I discovered while looking at this problem: A pending signal is discarded when SIG_IGN is set. In case that a posixtimer signal is pending then it is discarded as well, but when SIG_IGN is removed later nothing rearms the timer. This is not new, it's that way since posix timers have been merged. So nothing to worry about right now. I have a working solution to fix all of this, but the impact is too large for both stable and 2.6.22. I'm going to send it out for review in the next days. This should go into 2.6.21.stable as well. Signed-off-by: Thomas Gleixner Acked-by: Ingo Molnar Cc: Jan Kiszka Cc: Ulrich Drepper Cc: Stable Team Signed-off-by: Linus Torvalds --- kernel/posix-timers.c | 35 +++++++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/kernel/posix-timers.c b/kernel/posix-timers.c index 588c99da03..329ce01720 100644 --- a/kernel/posix-timers.c +++ b/kernel/posix-timers.c @@ -353,9 +353,40 @@ static enum hrtimer_restart posix_timer_fn(struct hrtimer *timer) * it should be restarted. */ if (timr->it.real.interval.tv64 != 0) { + ktime_t now = hrtimer_cb_get_time(timer); + + /* + * FIXME: What we really want, is to stop this + * timer completely and restart it in case the + * SIG_IGN is removed. This is a non trivial + * change which involves sighand locking + * (sigh !), which we don't want to do late in + * the release cycle. + * + * For now we just let timers with an interval + * less than a jiffie expire every jiffie to + * avoid softirq starvation in case of SIG_IGN + * and a very small interval, which would put + * the timer right back on the softirq pending + * list. By moving now ahead of time we trick + * hrtimer_forward() to expire the timer + * later, while we still maintain the overrun + * accuracy, but have some inconsistency in + * the timer_gettime() case. This is at least + * better than a starved softirq. A more + * complex fix which solves also another related + * inconsistency is already in the pipeline. + */ +#ifdef CONFIG_HIGH_RES_TIMERS + { + ktime_t kj = ktime_set(0, NSEC_PER_SEC / HZ); + + if (timr->it.real.interval.tv64 < kj.tv64) + now = ktime_add(now, kj); + } +#endif timr->it_overrun += - hrtimer_forward(timer, - hrtimer_cb_get_time(timer), + hrtimer_forward(timer, now, timr->it.real.interval); ret = HRTIMER_RESTART; ++timr->it_requeue_pending; -- 2.39.5