Backport hr-tick fix into .25/.26

Hi all,

Since the introduction of 2.6.25-rc1 kernel I've been experiencing screen blanking during boot up.
I've reported this issue before (http://lkml.org/lkml/2008/3/12/290), and Rafael Wysocki opened a bug report (http://bugzilla.kernel.org/show_bug.cgi?id=10235).
Since the problem seemed to be related to the x server, I reported it to their bug tracker (https://bugs.freedesktop.org/show_bug.cgi?id=15602).

I've tested with 2 computers, both with Intel graphic cards and various versions of Ubuntu. Sorry, I couldn't do more testing :(
I know of one other person that ran into what seems to be the same bug. (http://lkml.org/lkml/2008/6/10/137 | http://bugzilla.kernel.org/show_bug.cgi?id=10892)

Now since 2.6.27-rc1 the bug has been resolved. I did a git bisect and found both the commit that introduced the bug and also the one that fixed it. Greg (and stable team), I was wondering if the commit that went into .27 that fixed the bug could be backported into both the .25 and .26 stable kernels?

This commit introduced the bug.

commit 8f4d37ec073c17e2d4aa8851df5837d798606d6f
Author: Peter Zijlstra <[email protected]>
Date: Fri Jan 25 21:08:29 2008 +0100

sched: high-res preemption tick

Use HR-timers (when available) to deliver an accurate preemption tick.

The regular scheduler tick that runs at 1/HZ can be too coarse when nice level are used. The fairness system will still keep the cpu utilisation 'fair' by then delaying the task that got an excessive amount of CPU time but try to minimize this by delivering preemption points spot-on.

The average frequency of this extra interrupt is sched_latency / nr_latency. Which need not be higher than 1/HZ, its just that the distribution within the sched_latency period is important.

:040000 040000 ab225228500f7a19d5ad20ca12ca3fc8ff5f5ad1 f1742e1d225a72aecea9d6961ed989b5943d31d8 M arch :040000 040000 25d85e4ef7a71b0cc76801a2526ebeb4dce180fe ae61510186b4fad708ef0211ac169decba16d4e5 M include :040000 040000 9247cec7dd506c648ac027c17e5a07145aa41b26 950832cc1dc4d30923f593ecec883a06b45d62e9 M kernel

And this one fixed it

commit 31656519e132f6612584815f128c83976a9aaaef
Author: Peter Zijlstra <[email protected]>
Date: Fri Jul 18 18:01:23 2008 +0200

sched, x86: clean up hrtick implementation

random uvesafb failures were reported against Gentoo:

http://bugs.gentoo.org/show_bug.cgi?id=222799

and Mihai Moldovan bisected it back to:

Linus suspected it to be hrtick + vm86 interaction and observed:

Right, I never liked having to fiddle with those TIF flags. Initially I needed it because the hrtimer base lock could not nest in the rq lock. That however is fixed these days.

Currently the only reason left to fiddle with the TIF flags is remote wakeups. We cannot program a remote cpu's hrtimer. I've been thinking about using the new and improved IPI function call stuff to implement hrtimer_start_on().

However that does require that smp_call_function_single(.wait=0) works from interrupt context - /me looks at the latest series from Jens - Yes that does seem to be supported, good.

Here's a stab at cleaning this stuff up ...

Mihai reported test success as well.

:040000 040000 5ae152350652713c58bd1700ba2c776a556b6985 40d22771987dc5814a1e18aa3cee82ae9e4faea5 M arch :040000 040000 4dfe3c6abd244d2da57b7801e47f073899124376 3863e3311a21dc049d5ad98f45c272e4a5269a2b M include :040000 040000 236b2824be1c7cf3c899a090498e4151543bba31 9f9779c89b781d8fa8950468deee42f419339bd7 M kernel

Justin Madru

hm, the backport of 31656519 is a bit intrusive.

find below is an (untested!) version of it - i havent even build-tested it. Does it work for you? But this is Greg's call really.

Ingo

-------------->

From: Ingo Molnar <[email protected]>
Date: Tue, 19 Aug 2008 02:29:00 +0200
Subject: [PATCH] sched, x86: clean up hrtick implementation

random uvesafb failures were reported against Gentoo:

http://bugs.gentoo.org/show_bug.cgi?id=222799

and Mihai Moldovan bisected it back to:

Linus suspected it to be hrtick + vm86 interaction and observed:

Right, I never liked having to fiddle with those TIF flags. Initially I needed it because the hrtimer base lock could not nest in the rq lock. That however is fixed these days.

Currently the only reason left to fiddle with the TIF flags is remote wakeups. We cannot program a remote cpu's hrtimer. I've been thinking about using the new and improved IPI function call stuff to implement hrtimer_start_on().

However that does require that smp_call_function_single(.wait=0) works from interrupt context - /me looks at the latest series from Jens - Yes that does seem to be supported, good.

Here's a stab at cleaning this stuff up ...

Mihai reported test success as well.

Conflicts:

include/asm-x86/thread_info.h
kernel/sched.c

---
 arch/x86/kernel/signal_32.c |    3 -
 arch/x86/kernel/signal_64.c |    3 -
 kernel/Kconfig.hz           |    2 +-
 kernel/sched.c              |  197 +++++++++++++------------------------------
 kernel/sched_fair.c         |    5 +-
 5 files changed, 61 insertions(+), 149 deletions(-)

index d923736..e1fc7bd 100644
@@ -667,8 +667,5 @@ do_notify_resume(struct pt_regs *regs, void *unused, __u32 thread_info_flags)
 	if (thread_info_flags & _TIF_SIGPENDING)
 		do_signal(regs);
 
-	if (thread_info_flags & _TIF_HRTICK_RESCHED)
-		hrtick_resched();
-
 	clear_thread_flag(TIF_IRET);
 }

index e53b267..88023fc 100644
@@ -502,9 +502,6 @@ void do_notify_resume(struct pt_regs *regs, void *unused,
 	/* deal with pending signal delivery */
 	if (thread_info_flags & _TIF_SIGPENDING)
 		do_signal(regs);
-
-	if (thread_info_flags & _TIF_HRTICK_RESCHED)
-		hrtick_resched();
 }
 
 void signal_fault(struct pt_regs *regs, void __user *frame, char *where)

index 526128a..2a202a8 100644
@@ -55,4 +55,4 @@ config HZ
 	default 1000 if HZ_1000
 
 config SCHED_HRTICK
-	def_bool HIGH_RES_TIMERS && X86
+	def_bool HIGH_RES_TIMERS

index 08ae848..e193a9d 100644
@@ -808,7 +808,6 @@ entity_tick(struct cfs_rq *cfs_rq, struct sched_entity *curr, int queued)
 #ifdef CONFIG_SCHED_HRTICK
 static void hrtick_start_fair(struct rq *rq, struct task_struct *p)
 {
-	int requeue = rq->curr == p;
 	struct sched_entity *se = &p->se;
 	struct cfs_rq *cfs_rq = cfs_rq_of(se);
 

@@ -829,10 +828,10 @@ static void hrtick_start_fair(struct rq *rq, struct task_struct *p)
 		 * Don't schedule slices shorter than 10000ns, that just
 		 * doesn't make sense. Rely on vruntime for fairness.
 		 */
-		if (!requeue)
+		if (rq->curr != p)
 			delta = max(10000LL, delta);
 
-		hrtick_start(rq, delta, requeue);
+		hrtick_start(rq, delta);
 	}
 }
 #else

I tried to apply it onto 2.6.26.2, but got this build error:

CC arch/x86/kernel/i8253.o
CC kernel/sched.o
CC arch/x86/kernel/pci-nommu.o
kernel/sched.c:537: error: field ‘hrtick_csd’ has incomplete type kernel/sched.c: In function ‘hrtick_start’:
kernel/sched.c:1080: error: implicit declaration of function ‘__smp_call_function_single’ CC arch/x86/kernel/tsc_32.o
make[2]: *** [kernel/sched.o] Error 1
make[1]: *** [kernel] Error 2
make[1]: *** Waiting for unfinished jobs....
CC arch/x86/kernel/io_delay.o
CC arch/x86/kernel/rtc.o
CC arch/x86/kernel/trampoline.o

Was I suppose to use a different kernel? (2.6.25[.x] or 2.6.26) I'll try 2.6.25.15

Anyways, I know it's Greg's call, and I'm just trying to inform him of a bug that's been fixed upstream that should be backported. If he declines, that's ok. I've opened a bug report with Ubuntu. But, it would help if I had a patch to show.

Justin Madru

It largely depends on all the new IPI stuff that went into 27 as well, so I'd be surprised if its easily backportable..

ah, i see. How about a simple patch then that disables hrtick [given that it's now fixed .27 but the fix is too intrusive to backport]? Would that be too risky for -stable?

Ingo

That's just flipping sched_feat(HRTICK) to 0 by default.

That sounds reasonable, I'll take a patch for that for -stable :)

thanks,

greg k-h

Thanks Greg :)

Will the patch make it into 2.6.26.3 and 2.6.25.16, or will it be in the next point releases (2.6.26.4 and 2.6.25.17)?

Justin Madru

As no one has sent me any patch to do this, it will be very hard to get into the release that is happening tomorrow. It will have to be one after that.

thanks,

greg k-h

The hrtick implementation in 2.6.25 and .26 has been known to cause boot problems with at least Intel GMA cards. see:

https://bugs.freedesktop.org/show_bug.cgi?id=15602
http://bugzilla.kernel.org/show_bug.cgi?id=10892

A full fix to hrtick went into 2.6.27 (31656519e132f6612584815f128c83976a9aaaef), but that fix is too intrusive to backport. Henceforth, we default to disable hrtick.

@@ -4,7 +4,7 @@

SCHED_FEAT(AFFINE_WAKEUPS, 1)
SCHED_FEAT(CACHE_HOT_BUDDY, 1)
SCHED_FEAT(SYNC_WAKEUPS, 1)
-SCHED_FEAT(HRTICK, 1)
+SCHED_FEAT(HRTICK, 0)
SCHED_FEAT(DOUBLE_TICK, 0)
SCHED_FEAT(NORMALIZED_SLEEPER, 1)
SCHED_FEAT(DEADLINE, 1)

Thing is, I'm still not understanding how, or if, the old code is wrong.

That said, this will paper over the problem just fine, and as its old kernels we're talking about I don't particularly care.

Just occured to me that a Kconfig change that always makes CONFIG_SCHED_HRTICK not set will also do.

Fair enough.

All I know is that right when I started testing .25-rc1 I kept getting a garbled boot splash, blank screen, and even lock-ups during booting. It seemed to be that if I disabled the boot splash and/or started x manually the problem would rarely occur. So, maybe the first change to hrtick just exposed a bug in the boot splash or the x server. If that's true then, I don't get why right when .27-rc1 was out all the boot problems were gone.

Bisecting found both commits (introduce and fix) to be about hrtick. So maybe the commit that introduced my bug wasn't completely wrong, but at least it was not right enough because of the second commit was trying to fix something -- right? And the kernel still boots with hrtick disabled, I tested that.

Wouldn't disabling CONFIG_HIGH_RES_TIMERS (in the menu) also disable CONFIG_SCHED_HRTICK? In any case what about this patch below?

The hrtick implementation in 2.6.25 and .26 has been known to cause boot problems with at least Intel GMA cards. see:

https://bugs.freedesktop.org/show_bug.cgi?id=15602
http://bugzilla.kernel.org/show_bug.cgi?id=10892

A full fix to hrtick went into 2.6.27 (31656519e132f6612584815f128c83976a9aaaef), but that fix is too intrusive to backport. Henceforth, we default to disable hrtick.

@@ -55,4 +55,5 @@
    default 1000 if HZ_1000
 

config SCHED_HRTICK

-    def_bool HIGH_RES_TIMERS && X86
+    bool
+    default n