2.6.13-rt1

2.6.13-rt1

[Ingo Molnar]

i have released the 2.6.13-rt1 tree, which can be downloaded from the 
usual place:

  http://redhat.com/~mingo/realtime-preempt/

the new "eliminate the global PI lock" code from Steven Rostedt is now 
ready for prime-time. Smaller fixes otherwise. Please re-report any 
remaining regressions.

Changes since 2.6.13-rc7-rt1:

 - second (final) phase p->pi_lock SMP scalability improvement: replace 
   the pi_lock with per-task ->pi_lock and eliminate the global pi_lock 
   (Steven Rostedt)

 - fix for ->pi_lock code (Steven Rostedt)

 - improve ->pi_lock code on UP (Steven Rostedt)

 - x86_64 boot fix (Daniel Walker)

 - ALL_TASKS_PI fixes (Daniel Walker)

 - enabled ALL_TASKS_PI for debugging purposes

 - merge to 2.6.13-final

to build a 2.6.13-rt1 tree, the following patches should be applied:

   http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.13.tar.bz2
   http://redhat.com/~mingo/realtime-preempt/patch-2.6.13-rt1

	Ingo

Re: 2.6.13-rt1

[Steven Rostedt]

Ingo,

I think you have a slight glitch in your patch.

-- Steve

$ patch -p1 -s < /work/realtime-patches/patch-2.6.13-rt1
The next patch would delete the file Makefile.rej,
which does not exist!  Assume -R? [n]
Apply anyway? [n]

Re: 2.6.13-rt1

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo,
> 
> I think you have a slight glitch in your patch.
> 
> -- Steve
> 
> $ patch -p1 -s < /work/realtime-patches/patch-2.6.13-rt1
> The next patch would delete the file Makefile.rej,
> which does not exist!  Assume -R? [n]
> Apply anyway? [n]

indeed. I fixed this up in the file without uploading a new release, so 
new downloads shouldnt see this.

	Ingo

Re: 2.6.13-rt1

[Daniel Walker]

On Mon, 2005-08-29 at 10:48 +0200, Ingo Molnar wrote:

> 
>  - x86_64 boot fix (Daniel Walker)

Ingo, Did this work for you?

Daniel

Re: 2.6.13-rt1

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> On Mon, 2005-08-29 at 10:48 +0200, Ingo Molnar wrote:
> 
> > 
> >  - x86_64 boot fix (Daniel Walker)
> 
> Ingo, Did this work for you?

nope, it's a UP box.

	Ingo

Re: 2.6.13-rt1

[Daniel Walker]

On Mon, 2005-08-29 at 17:18 +0200, Ingo Molnar wrote:
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
> > On Mon, 2005-08-29 at 10:48 +0200, Ingo Molnar wrote:
> > 
> > > 
> > >  - x86_64 boot fix (Daniel Walker)
> > 
> > Ingo, Did this work for you?
> 
> nope, it's a UP box.


Does it hang early during like ACPI , or after init ?

Daniel

Re: 2.6.13-rt1

[Steven Rostedt]

Ingo,

I just found another deadlock in the pi_lock logic.  The PI logic is
very dependent on the P1->L1->P2->L2->P3 order.  But our good old friend
is back, the BKL.

Since the BKL is let go and regrabbed even if a task is grabbing another
lock, it messes up the order.  For example, it can do the following:
L1->P1->L2->P2->L1 if L1 is the BKL.  Luckly, (and I guess there's
really no other way) the BKL is only held by those that are currently
running, or at least not blocked on anyone.  So I added code in the
pi_setprio code to test if the next lock in the loop is the BKL and if
so, if its owner is the current task.  If so, the loop is broken.

Without this patch, I would constantly get lock ups on shutdown where it
sends SIGTERM to all the processes.  It usually would lock up on the
killing of udev.  But with the patch, I've shutdown a few times already
and no lockups so far.

-- Steve

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux_realtime_goliath/kernel/rt.c
===================================================================
--- linux_realtime_goliath/kernel/rt.c	(revision 308)
+++ linux_realtime_goliath/kernel/rt.c	(working copy)
@@ -816,6 +816,21 @@
 		l = w->lock;
 		TRACE_BUG_ON_LOCKED(!lock);
 
+		/*
+		 * The BKL can really be a pain.  It can happen that the lock
+		 * we are blocked on is owned by a task that is waiting for
+		 * the BKL, and we own it.  So, if this is the BKL and we own
+		 * it, then end the loop here.
+		 */
+		if (unlikely(l == &kernel_sem.lock) && lock_owner(l) == current_thread_info()) {
+			/*
+			 * No locks are held for locks, so fool the unlocking code
+			 * by thinking the last lock was the original.
+			 */
+			l = lock;
+			break;
+		}
+
 		if (l != lock)
 			__raw_spin_lock(&l->wait_lock);
 

Re: 2.6.13-rt1

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> Ingo,
> 
> I just found another deadlock in the pi_lock logic.  The PI logic is 
> very dependent on the P1->L1->P2->L2->P3 order.  But our good old 
> friend is back, the BKL.
> 
> Since the BKL is let go and regrabbed even if a task is grabbing 
> another lock, it messes up the order.  For example, it can do the 
> following: L1->P1->L2->P2->L1 if L1 is the BKL.  Luckly, (and I guess 
> there's really no other way) the BKL is only held by those that are 
> currently running, or at least not blocked on anyone.  So I added code 
> in the pi_setprio code to test if the next lock in the loop is the BKL 
> and if so, if its owner is the current task.  If so, the loop is 
> broken.
> 
> Without this patch, I would constantly get lock ups on shutdown where 
> it sends SIGTERM to all the processes.  It usually would lock up on 
> the killing of udev.  But with the patch, I've shutdown a few times 
> already and no lockups so far.

thanks, applied.

	Ingo

Re: 2.6.13-rt1

[Steven Rostedt]

Hi Ingo,

Looks like the BKL is a little more complicated than what I first sent.
I've been analyzing the logic and found that there's a point in time
where the BKL->P1->L1->P2->BKL can exist without any of the spinlocks
protecting it.  That is after P1 blocks on L1 but before it schedules
out and releases the BKL.  In this time another process on another CPU
could loop here.

The supplied patch fixes this.

Also, I need to look more into the logic of __up to see if the BKL can't
cause a deadlock with the grabbing and releasing of locks there.  So I
might be sending more patches to clean this up.

Do me a favor, and just take a quick look at the logic here, and make
sure that the situation is OK to break there, and that there won't be
any other side-effects, wrt. priority leaks.

Thanks,

-- Steve

Patch is against rt2

Signed-off-by: Steven Rostedt <rostedt@goodmis.org>

Index: linux_realtime_goliath/kernel/rt.c
===================================================================
--- linux_realtime_goliath/kernel/rt.c	(revision 310)
+++ linux_realtime_goliath/kernel/rt.c	(working copy)
@@ -760,11 +760,12 @@
  */
 static void pi_setprio(struct rt_mutex *lock, struct task_struct *task, int prio)
 {
-	struct rt_mutex *l = lock;
-	struct task_struct *p = task;
 	/*
 	 * We don't want to release the parameters locks.
 	 */
+	struct rt_mutex *l = lock;
+	struct task_struct *p = task;
+	int bkl = 0;
 
 	if (unlikely(!p->pid)) {
 		pi_null++;
@@ -800,7 +801,7 @@
 #endif
 
 		mutex_setprio(p, prio);
-		if (!w)
+		if (!w || unlikely(bkl))
 			break;
 		/*
 		 * If the task is blocked on a lock, and we just made
@@ -817,18 +818,31 @@
 		TRACE_BUG_ON_LOCKED(!lock);
 
 		/*
-		 * The BKL can really be a pain.  It can happen that the lock
-		 * we are blocked on is owned by a task that is waiting for
-		 * the BKL, and we own it.  So, if this is the BKL and we own
-		 * it, then end the loop here.
+		 * The BKL can really be a pain. It can happen where the
+		 * BKL is being held by one task that is just about to 
+		 * block on another task that is waiting for the BKL.
+		 * This isn't a deadlock, since the BKL is released
+		 * when the task goes to sleep.  This also means that
+		 * all holders of the BKL are not blocked, or are just
+		 * about to be blocked.
+		 *
+		 * Another side-effect of this is that there's a small
+		 * window where the spinlocks are not held, and the blocked
+		 * process hasn't released the BKL.  So if we are going
+		 * to boost the owner of the BKL, stop after that,
+		 * since that owner is either running, or about to sleep
+		 * but don't go any further or we are in a loop.
 		 */
-		if (unlikely(l == &kernel_sem.lock) && lock_owner(l) == current_thread_info()) {
-			/*
-			 * No locks are held for locks, so fool the unlocking code
-			 * by thinking the last lock was the original.
-			 */
-			l = lock;
-			break;
+		if (unlikely(l == &kernel_sem.lock)) {
+			if (lock_owner(l) == current_thread_info()) {
+				/*
+				 * No locks are held for locks, so fool the unlocking code
+				 * by thinking the last lock was the original.
+				 */
+				l = lock;
+				break;
+			}
+			bkl = 1;
 		}
 
 		if (l != lock)

Re: 2.6.13-rt1

[Daniel Walker]

Have you tried turning on 
"Non-preemptible critical section latency timing" or "Latency tracing"

I don't know if it's related to the PI changes, but I'm getting a crash
with those on em64t .

With both above options I get

<0>init[1]: segfault at ffffffff8010ef44 rip ffffffff8010ef44 rsp 00007fffferror 5
<0>init[1]: segfault at ffffffff8010ef44 rip ffffffff8010ef44 rsp 00007ffffffe8de8 error 5

printed never ending right after init starts.

If I only turn on "Non-preemptible critical section latency timing",
then when I enter the command,
"echo 0 > /proc/sys/kernel/preempt_max_latency"

The kernel will spit out a couple of max critical section updates , then
it will hang silently.

This is all new in 2.6.13-rtX . It could have just come in with 2.6.13 ,
but I thought I'd mention it.

Daniel

On Tue, 2005-08-30 at 09:06 -0400, Steven Rostedt wrote:
> Hi Ingo,
> 
> Looks like the BKL is a little more complicated than what I first sent.
> I've been analyzing the logic and found that there's a point in time
> where the BKL->P1->L1->P2->BKL can exist without any of the spinlocks
> protecting it.  That is after P1 blocks on L1 but before it schedules
> out and releases the BKL.  In this time another process on another CPU
> could loop here.
> 
> The supplied patch fixes this.
> 
> Also, I need to look more into the logic of __up to see if the BKL can't
> cause a deadlock with the grabbing and releasing of locks there.  So I
> might be sending more patches to clean this up.
> 
> Do me a favor, and just take a quick look at the logic here, and make
> sure that the situation is OK to break there, and that there won't be
> any other side-effects, wrt. priority leaks.
> 
> Thanks,
> 
> -- Steve
> 
> Patch is against rt2
> 
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> 
> Index: linux_realtime_goliath/kernel/rt.c
> ===================================================================
> --- linux_realtime_goliath/kernel/rt.c	(revision 310)
> +++ linux_realtime_goliath/kernel/rt.c	(working copy)
> @@ -760,11 +760,12 @@
>   */
>  static void pi_setprio(struct rt_mutex *lock, struct task_struct *task, int prio)
>  {
> -	struct rt_mutex *l = lock;
> -	struct task_struct *p = task;
>  	/*
>  	 * We don't want to release the parameters locks.
>  	 */
> +	struct rt_mutex *l = lock;
> +	struct task_struct *p = task;
> +	int bkl = 0;
>  
>  	if (unlikely(!p->pid)) {
>  		pi_null++;
> @@ -800,7 +801,7 @@
>  #endif
>  
>  		mutex_setprio(p, prio);
> -		if (!w)
> +		if (!w || unlikely(bkl))
>  			break;
>  		/*
>  		 * If the task is blocked on a lock, and we just made
> @@ -817,18 +818,31 @@
>  		TRACE_BUG_ON_LOCKED(!lock);
>  
>  		/*
> -		 * The BKL can really be a pain.  It can happen that the lock
> -		 * we are blocked on is owned by a task that is waiting for
> -		 * the BKL, and we own it.  So, if this is the BKL and we own
> -		 * it, then end the loop here.
> +		 * The BKL can really be a pain. It can happen where the
> +		 * BKL is being held by one task that is just about to 
> +		 * block on another task that is waiting for the BKL.
> +		 * This isn't a deadlock, since the BKL is released
> +		 * when the task goes to sleep.  This also means that
> +		 * all holders of the BKL are not blocked, or are just
> +		 * about to be blocked.
> +		 *
> +		 * Another side-effect of this is that there's a small
> +		 * window where the spinlocks are not held, and the blocked
> +		 * process hasn't released the BKL.  So if we are going
> +		 * to boost the owner of the BKL, stop after that,
> +		 * since that owner is either running, or about to sleep
> +		 * but don't go any further or we are in a loop.
>  		 */
> -		if (unlikely(l == &kernel_sem.lock) && lock_owner(l) == current_thread_info()) {
> -			/*
> -			 * No locks are held for locks, so fool the unlocking code
> -			 * by thinking the last lock was the original.
> -			 */
> -			l = lock;
> -			break;
> +		if (unlikely(l == &kernel_sem.lock)) {
> +			if (lock_owner(l) == current_thread_info()) {
> +				/*
> +				 * No locks are held for locks, so fool the unlocking code
> +				 * by thinking the last lock was the original.
> +				 */
> +				l = lock;
> +				break;
> +			}
> +			bkl = 1;
>  		}
>  
>  		if (l != lock)
> 
> 
> -
> 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/

Re: 2.6.13-rt1

[Steven Rostedt]

On Tue, 2005-08-30 at 15:34 -0700, Daniel Walker wrote:
> Have you tried turning on 
> "Non-preemptible critical section latency timing" or "Latency tracing"

I just turned on the following:

  CONFIG_CRITICAL_PREEMPT_TIMING
  CONFIG_CRITICAL_IRQSOFF_TIMING
  CONFIG_LATENCY_TRACE

recompiled and booted.  No problem here.

> 
> I don't know if it's related to the PI changes, but I'm getting a crash
> with those on em64t .
> 
> With both above options I get
> 
> <0>init[1]: segfault at ffffffff8010ef44 rip ffffffff8010ef44 rsp 00007fffferror 5
> <0>init[1]: segfault at ffffffff8010ef44 rip ffffffff8010ef44 rsp 00007ffffffe8de8 error 5
> 
> printed never ending right after init starts.
> 
> If I only turn on "Non-preemptible critical section latency timing",
> then when I enter the command,
> "echo 0 > /proc/sys/kernel/preempt_max_latency"
> 
> The kernel will spit out a couple of max critical section updates , then
> it will hang silently.
> 
> This is all new in 2.6.13-rtX . It could have just come in with 2.6.13 ,
> but I thought I'd mention it.

Did you try the latest patches I just sent. Mainly this last one on
-rt2?  There is a deadlock that is fixed wrt the BKL.

-- Steve

[PATCH] PREEMPT_RT vermagic

[Daniel Walker]

Ingo,
	This patch adds a vermagic hook so PREEMPT_RT modules can be
distinguished from PREEMPT_DESKTOP modules.
	
Signed-off-by: Daniel Walker <dwalker@mvista.com>
Index: linux-2.6.10/include/linux/vermagic.h
===================================================================
--- linux-2.6.10.orig/include/linux/vermagic.h	2004-12-24 21:35:50.000000000 +0000
+++ linux-2.6.10/include/linux/vermagic.h	2005-08-23 17:35:08.000000000 +0000
@@ -8,7 +8,11 @@
 #define MODULE_VERMAGIC_SMP ""
 #endif
 #ifdef CONFIG_PREEMPT
-#define MODULE_VERMAGIC_PREEMPT "preempt "
+# ifdef CONFIG_PREEMPT_RT
+# define MODULE_VERMAGIC_PREEMPT "preempt_rt "
+# else
+# define MODULE_VERMAGIC_PREEMPT "preempt "
+# endif
 #else
 #define MODULE_VERMAGIC_PREEMPT ""
 #endif

Re: [PATCH] PREEMPT_RT vermagic

[Ingo Molnar]

* Daniel Walker <dwalker@mvista.com> wrote:

> Ingo,
> 	This patch adds a vermagic hook so PREEMPT_RT modules can be
> distinguished from PREEMPT_DESKTOP modules.

vermagic is very crude and there are zillions of other details and 
.config flags that might make a module incompatible. You can use 
CONFIG_MODVERSIONS to get a stronger protection that vermagic, but 
that's far from perfect too. The right solution is the module signing 
framework in Fedora. Until that gets merged upstream just dont mix 
incompatible modules, and keep things tightly packaged.

	Ingo

Re: [PATCH] PREEMPT_RT vermagic

[Tom Rini]

On Wed, Aug 31, 2005 at 09:20:17AM +0200, Ingo Molnar wrote:
> 
> * Daniel Walker <dwalker@mvista.com> wrote:
> 
> > Ingo,
> > 	This patch adds a vermagic hook so PREEMPT_RT modules can be
> > distinguished from PREEMPT_DESKTOP modules.
> 
> vermagic is very crude and there are zillions of other details and 
> .config flags that might make a module incompatible. You can use 
> CONFIG_MODVERSIONS to get a stronger protection that vermagic, but 
> that's far from perfect too. The right solution is the module signing 
> framework in Fedora. Until that gets merged upstream just dont mix 
> incompatible modules, and keep things tightly packaged.

MODVERSIONS won't get the PREEMPT_RT vs PREEMPT_DESKTOP case right
without this, unless I'm missing something.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH] PREEMPT_RT vermagic

[Tom Rini]

On Wed, Aug 31, 2005 at 07:29:44AM -0700, Tom Rini wrote:
> On Wed, Aug 31, 2005 at 09:20:17AM +0200, Ingo Molnar wrote:
> > 
> > * Daniel Walker <dwalker@mvista.com> wrote:
> > 
> > > Ingo,
> > > 	This patch adds a vermagic hook so PREEMPT_RT modules can be
> > > distinguished from PREEMPT_DESKTOP modules.
> > 
> > vermagic is very crude and there are zillions of other details and 
> > .config flags that might make a module incompatible. You can use 
> > CONFIG_MODVERSIONS to get a stronger protection that vermagic, but 
> > that's far from perfect too. The right solution is the module signing 
> > framework in Fedora. Until that gets merged upstream just dont mix 
> > incompatible modules, and keep things tightly packaged.
> 
> MODVERSIONS won't get the PREEMPT_RT vs PREEMPT_DESKTOP case right
> without this, unless I'm missing something.

I'm missing something I see.

-- 
Tom Rini
http://gate.crashing.org/~trini/