public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@suse.de>
To: linux-kernel@vger.kernel.org, stable@kernel.org,
	torvalds@linux-foundation.org
Cc: Justin Forbes <jmforbes@linuxtx.org>,
	Zwane Mwaikambo <zwane@arm.linux.org.uk>,
	"Theodore Ts'o" <tytso@mit.edu>,
	Randy Dunlap <rdunlap@xenotime.net>,
	Dave Jones <davej@redhat.com>,
	Chuck Wolber <chuckw@quantumlinux.com>,
	Chris Wedgwood <reviews@ml.cw.f00f.org>,
	Michael Krufky <mkrufky@linuxtv.org>,
	Chuck Ebbert <cebbert@redhat.com>,
	Domenico Andreoli <cavokz@gmail.com>,
	akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk,
	mingo@elte.hu, drepper@redhat.com, jan.kiszka@web.de,
	Thomas Gleixner <tglx@linutronix.de>,
	Chris Wright <chrisw@sous-sol.org>,
	Greg Kroah-Hartman <gregkh@suse.de>
Subject: [patch 15/26] posix-timers: Prevent softirq starvation by small intervals and SIG_IGN
Date: Mon, 30 Jul 2007 21:32:59 -0700	[thread overview]
Message-ID: <20070731043259.GP3975@kroah.com> (raw)
In-Reply-To: <20070731043047.GA3975@kroah.com>

[-- Attachment #1: posix-timers-prevent-softirq-starvation-by-small-intervals-and-sig_ign.patch --]
[-- Type: text/plain, Size: 3783 bytes --]

-stable review patch.  If anyone has any objections, please let us know.

------------------

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 <tglx@linutronix.de>
Acked-by: Ingo Molnar <mingo@elte.hu>
Cc: Jan Kiszka <jan.kiszka@web.de>
Cc: Ulrich Drepper <drepper@redhat.com>
Signed-off-by: Chris Wright <chrisw@sous-sol.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
 kernel/posix-timers.c |   35 +++++++++++++++++++++++++++++++++--
 1 file changed, 33 insertions(+), 2 deletions(-)

--- linux-2.6.21.6.orig/kernel/posix-timers.c
+++ linux-2.6.21.6/kernel/posix-timers.c
@@ -354,9 +354,40 @@ static enum hrtimer_restart posix_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;

-- 

  parent reply	other threads:[~2007-07-31  4:36 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20070731042108.546594256@blue.kroah.org>
2007-07-31  4:30 ` [patch 00/26] 2.6.21.7 -stable review Greg KH
2007-07-31  4:31   ` [patch 01/26] BNX2: Fix netdev watchdog on 5708 Greg KH
2007-07-31  4:31   ` [patch 02/26] sparsemem: fix oops in x86_64 show_mem Greg KH
2007-07-31  4:31   ` [patch 03/26] rt-mutex: Fix stale return value Greg KH
2007-07-31  4:31   ` [patch 04/26] rt-mutex: Fix chain walk early wakeup bug Greg KH
2007-07-31  4:31   ` [patch 05/26] pi-futex: Fix exit races and locking problems Greg KH
2007-07-31  4:31   ` [patch 06/26] hpt366: disallow Ultra133 for HPT374 Greg KH
2007-07-31  4:31   ` [patch 07/26] md: Fix two raid10 bugs Greg KH
2007-07-31  4:32   ` [patch 08/26] md: Fix bug in error handling during raid1 repair Greg KH
2007-07-31  4:32   ` [patch 09/26] dm crypt: disable barriers Greg KH
2007-07-31  4:32   ` [patch 10/26] dm crypt: fix call to clone_init Greg KH
2007-07-31  4:32   ` [patch 11/26] dm crypt: fix avoid cloned bio ref after free Greg KH
2007-07-31  4:32   ` [patch 12/26] dm crypt: fix remove first_clone Greg KH
2007-07-31  4:32   ` [patch 13/26] hugetlb: fix get_policy for stacked shared memory files Greg KH
2007-07-31  4:32   ` [patch 14/26] sched: fix next_interval determination in idle_balance() Greg KH
2007-07-31 15:02     ` Paul E. McKenney
2007-07-31  4:32   ` Greg KH [this message]
2007-07-31  4:33   ` [patch 16/26] FUTEX: Restore the dropped ERSCH fix Greg KH
2007-07-31  4:33   ` [patch 17/26] audit: fix oops removing watch if audit disabled Greg KH
2007-07-31  4:33   ` [patch 18/26] POWERPC: Fix subtle FP state corruption bug in signal return on SMP Greg KH
2007-07-31  4:33   ` [patch 19/26] mm: kill validate_anon_vma to avoid mapcount BUG Greg KH
2007-07-31  4:33   ` [patch 20/26] saa7134: fix thread shutdown handling Greg KH
2007-07-31  5:05     ` Mauro Carvalho Chehab
2007-07-31  5:16       ` Greg KH
2007-07-31  4:33   ` [patch 21/26] serial: clear proper MPSC interrupt cause bits Greg KH
2007-07-31  4:33   ` [patch 22/26] i386: fix infinite loop with singlestep int80 syscalls Greg KH
2007-07-31  4:33   ` [patch 23/26] NTP: remove clock_was_set() call to prevent deadlock Greg KH
2007-07-31  4:33   ` [patch 24/26] sky2: workaround for lost IRQ Greg KH
2007-07-31  4:33   ` [patch 25/26] V4L: bttv: fix v4l1 api usage breaking the driver Greg KH
2007-07-31  4:34   ` [patch 26/26] V4L: cx88-blackbird: fix vidioc_g_tuner never ending list of tuners Greg KH
2007-07-31  4:43   ` [patch 00/26] 2.6.21.7 -stable review Greg KH
2007-07-31 10:50     ` Stefan Richter
2007-07-31 19:47       ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070731043259.GP3975@kroah.com \
    --to=gregkh@suse.de \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=cavokz@gmail.com \
    --cc=cebbert@redhat.com \
    --cc=chrisw@sous-sol.org \
    --cc=chuckw@quantumlinux.com \
    --cc=davej@redhat.com \
    --cc=drepper@redhat.com \
    --cc=jan.kiszka@web.de \
    --cc=jmforbes@linuxtx.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=mkrufky@linuxtv.org \
    --cc=rdunlap@xenotime.net \
    --cc=reviews@ml.cw.f00f.org \
    --cc=stable@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    --cc=zwane@arm.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox