[patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[Ingo Molnar]

i have released the -V0.7.47-00 Real-Time Preemption patch, which can be 
downloaded from the usual place:

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

this patch reintroduces the plist.h code from Daniel Walker and Inaky 
Perez-Gonzalez. It's also a merge to 2.6.12-rc4.

to build a -V0.7.47-00 tree, the following patches have to be applied:

   http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.11.tar.bz2
   http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.12-rc4.bz2
   http://redhat.com/~mingo/realtime-preempt/realtime-preempt-2.6.12-rc4-V0.7.47-00

	Ingo

RE: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[kus Kusche Klaus]

> i have released the -V0.7.47-00 Real-Time Preemption patch, 
> which can be 
> downloaded from the usual place:
> 
>     http://redhat.com/~mingo/realtime-preempt/
> 
> this patch reintroduces the plist.h code from Daniel Walker and Inaky 
> Perez-Gonzalez. It's also a merge to 2.6.12-rc4.
> 
> to build a -V0.7.47-00 tree, the following patches have to be applied:
> 
>    http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.11.tar.bz2
>    
> http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.12-rc4.bz2
>    
>
http://redhat.com/~mingo/realtime-preempt/realtime-preempt-2.6.12-rc4-V0
.7.47-00

It lacks "plist.h", but two "#include" refer to it?

-- 
Klaus Kusche                 (Software Development - Control Systems)
KEBA AG             Gewerbepark Urfahr, A-4041 Linz, Austria (Europe)
Tel: +43 / 732 / 7090-3120                 Fax: +43 / 732 / 7090-6301
E-Mail: kus@keba.com                                WWW: www.keba.com

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[Ingo Molnar]

* kus Kusche Klaus <kus@keba.com> wrote:

> > i have released the -V0.7.47-00 Real-Time Preemption patch, 
> > which can be 
> > downloaded from the usual place:
> > 
> >     http://redhat.com/~mingo/realtime-preempt/
> > 
> > this patch reintroduces the plist.h code from Daniel Walker and Inaky 
> > Perez-Gonzalez. It's also a merge to 2.6.12-rc4.
> > 
> > to build a -V0.7.47-00 tree, the following patches have to be applied:
> > 
> >    http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.11.tar.bz2
> >    
> > http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.12-rc4.bz2
> >    
> >
> http://redhat.com/~mingo/realtime-preempt/realtime-preempt-2.6.12-rc4-V0
> .7.47-00
> 
> It lacks "plist.h", but two "#include" refer to it?

yeah - patch messup. I've uploaded -01 which adds the missing file.

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[George Anzinger]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

Ingo,

Could you add the attached patch to the RT patch.  It fixes the memory 
corruption issue with the 2timer_test that we have been tracking down.

Also, I think that del_timer_sync and friends need to be turned on if soft_irq 
is preemptable.

Comments?
-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/

[-- Attachment #2: rt-timers-fix.patch --]
[-- Type: text/plain, Size: 7042 bytes --]

Source: MontaVista Software, Inc.  George Anzinger<george@mvista.com>
MR: 11506
Type: Defect Fix 
Disposition:  needs submitting to RT community patch
Keywords:
Signed-off-by: George Anzinger<george@mvista.com>
Description:

This change adds code to protect timers while they are being accesed by
the timer call back code.  This is now needed because the call back code
is preemptable when ever softirqs are preemptable.  In the past this was
needed only for SMP systems and it is that code which is expanded and
enhanced to account for the possiblility that the timer call back may be
preempted for some period of time.  (In the SMP case this time was
rather short so a spin was accetable.)

 posix-timers.c |   82 ++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 58 insertions(+), 24 deletions(-)

Index: linux-2.6.10/kernel/posix-timers.c
===================================================================
--- linux-2.6.10.orig/kernel/posix-timers.c
+++ linux-2.6.10/kernel/posix-timers.c
@@ -94,7 +94,7 @@ static DEFINE_SPINLOCK(idr_lock);
 #define TIMER_INACTIVE 1
 #define TIMER_RETRY 1
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 # define timer_active(tmr) \
 		((tmr)->it_timer.entry.prev != (void *)TIMER_INACTIVE)
 # define set_timer_inactive(tmr) \
@@ -102,10 +102,28 @@ static DEFINE_SPINLOCK(idr_lock);
 			(tmr)->it_timer.entry.prev = (void *)TIMER_INACTIVE; \
 		} while (0)
 #else
-# define timer_active(tmr) BARFY	// error to use outside of SMP
+# define timer_active(tmr) BARFY   /* error to use outside of SMP | RT */
 # define set_timer_inactive(tmr) do { } while (0)
 #endif
 /*
+ * For RT the timer call backs are preemptable.  This means that folks
+ * trying to delete timers may run into timers that are "active" for
+ * long times.  To help out with this we provide a wake up function to
+ * wake up a caller who wants waking when a timer clears the call back.
+ * This is the same sort of thing that the del_timer_sync does, but we
+ * need (in the HRT case) to cover two lists and not just the one.
+ */
+#ifdef CONFIG_PREEMPT_SOFTIRQS
+#include <linux/wait.h>
+static DECLARE_WAIT_QUEUE_HEAD(timer_wake_queue);
+#define wake_timer_waiters() wake_up(&timer_wake_queue)
+#define wait_for_timer(timer) wait_event(timer_wake_queue, !timer_active(timer))
+
+#else
+#define wake_timer_waiters()
+#define wait_for_timer(timer)
+#endif
+/*
  * we assume that the new SIGEV_THREAD_ID shares no bits with the other
  * SIGEV values.  Here we put out an error if this assumption fails.
  */
@@ -461,6 +479,7 @@ static void posix_timer_fn(unsigned long
 			schedule_next_timer(timr);
 	}
 	unlock_timer(timr, flags); /* hold thru abs lock to keep irq off */
+	wake_timer_waiters();
 }
 
 
@@ -922,18 +941,20 @@ do_timer_settime(struct k_itimer *timr, 
 	 * careful here.  If smp we could be in the "fire" routine which will
 	 * be spinning as we hold the lock.  But this is ONLY an SMP issue.
 	 */
-#ifdef CONFIG_SMP
-	if (timer_active(timr) && !del_timer(&timr->it_timer))
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
+	if (timer_active(timr) && !del_timer(&timr->it_timer)) {
 		/*
-		 * It can only be active if on an other cpu.  Since
-		 * we have cleared the interval stuff above, it should
-		 * clear once we release the spin lock.  Of course once
-		 * we do that anything could happen, including the
-		 * complete melt down of the timer.  So return with
-		 * a "retry" exit status.
+		 * It can only be active if on an other cpu (unless RT).
+		 * Since we have cleared the interval stuff above, it
+		 * should clear once we release the spin lock.  Of
+		 * course once we do that anything could happen,
+		 * including the complete melt down of the timer.  So
+		 * return with a "retry" exit status.  If RT we do a
+		 * formal wait as the function code is fully
+		 * preemptable...
 		 */
 		return TIMER_RETRY;
-
+	}
 	set_timer_inactive(timr);
 #else
 	del_timer(&timr->it_timer);
@@ -1011,7 +1032,8 @@ retry:
 							       &new_spec, rtn);
 	unlock_timer(timr, flag);
 	if (error == TIMER_RETRY) {
-		rtn = NULL;	// We already got the old time...
+		wait_for_timer(timr);
+		rtn = NULL;	/* We already got the old time... */
 		goto retry;
 	}
 
@@ -1025,17 +1047,19 @@ retry:
 static inline int do_timer_delete(struct k_itimer *timer)
 {
 	timer->it_incr = 0;
-#ifdef CONFIG_SMP
-	if (timer_active(timer) && !del_timer(&timer->it_timer))
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
+	if (timer_active(timer) && !del_timer(&timer->it_timer)) {
 		/*
-		 * It can only be active if on an other cpu.  Since
-		 * we have cleared the interval stuff above, it should
-		 * clear once we release the spin lock.  Of course once
-		 * we do that anything could happen, including the
-		 * complete melt down of the timer.  So return with
-		 * a "retry" exit status.
+		 * It can only be active if on an other cpu (unless RT).
+		 * Since we have cleared the interval stuff above, it
+		 * should clear once we release the spin lock.  Of
+		 * course once we do that anything could happen,
+		 * including the complete melt down of the timer.  So
+		 * return with a "retry" exit status.  For RT we do a
+		 * formal wait as it could take a while.
 		 */
 		return TIMER_RETRY;
+	}
 #else
 	del_timer(&timer->it_timer);
 #endif
@@ -1051,7 +1075,7 @@ sys_timer_delete(timer_t timer_id)
 	struct k_itimer *timer;
 	long flags;
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	int error;
 retry_delete:
 #endif
@@ -1059,11 +1083,12 @@ retry_delete:
 	if (!timer)
 		return -EINVAL;
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	error = p_timer_del(&posix_clocks[timer->it_clock], timer);
 
 	if (error == TIMER_RETRY) {
 		unlock_timer(timer, flags);
+		wait_for_timer(timer);
 		goto retry_delete;
 	}
 #else
@@ -1092,17 +1117,18 @@ static inline void itimer_delete(struct 
 {
 	unsigned long flags;
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	int error;
 retry_delete:
 #endif
 	spin_lock_irqsave(&timer->it_lock, flags);
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	error = p_timer_del(&posix_clocks[timer->it_clock], timer);
 
 	if (error == TIMER_RETRY) {
 		unlock_timer(timer, flags);
+		wait_for_timer(timer);
 		goto retry_delete;
 	}
 #else
@@ -1369,6 +1395,14 @@ void clock_was_set(void)
 		list_del_init(&timr->abs_timer_entry);
 		if (add_clockset_delta(timr, &new_wall_to) &&
 		    del_timer(&timr->it_timer))  /* timer run yet? */
+			/*
+			 * Note that we only do this if the timer is/was
+			 * in the list.  If it happens to be active an
+			 * not in the timer list, it must be in the call
+			 * back function, we leave it to that code to do
+			 * the right thing.  I.e we do NOT need
+			 * del_timer_sync()
+			 */
 			add_timer(&timr->it_timer);
 		list_add(&timr->abs_timer_entry, &abs_list.list);
 		spin_unlock_irq(&abs_list.lock);

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[Lee Revell]

On Mon, 2005-05-09 at 11:17 +0200, Ingo Molnar wrote:
> * kus Kusche Klaus <kus@keba.com> wrote:
> 
> > > i have released the -V0.7.47-00 Real-Time Preemption patch, 
> > > which can be 
> > > downloaded from the usual place:
> > > 
> > >     http://redhat.com/~mingo/realtime-preempt/
> > > 
> > > this patch reintroduces the plist.h code from Daniel Walker and Inaky 
> > > Perez-Gonzalez. It's also a merge to 2.6.12-rc4.
> > > 
> > > to build a -V0.7.47-00 tree, the following patches have to be applied:
> > > 
> > >    http://kernel.org/pub/linux/kernel/v2.6/linux-2.6.11.tar.bz2
> > >    
> > > http://kernel.org/pub/linux/kernel/v2.6/testing/patch-2.6.12-rc4.bz2
> > >    
> > >
> > http://redhat.com/~mingo/realtime-preempt/realtime-preempt-2.6.12-rc4-V0
> > .7.47-00
> > 
> > It lacks "plist.h", but two "#include" refer to it?
> 
> yeah - patch messup. I've uploaded -01 which adds the missing file.
> 

Ingo,

Can you add Mingming's ext3 patch to the next version?  For my workload
at least, this seems to be the last important latency breaker that we
need to go upstream.

Lee

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[Ingo Molnar]

* Lee Revell <rlrevell@joe-job.com> wrote:

> Ingo,
> 
> Can you add Mingming's ext3 patch to the next version?  For my 
> workload at least, this seems to be the last important latency breaker 
> that we need to go upstream.

yeah, agreed - i've applied it to my tree and it's looking good in my 
ext3 tests.

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[Ingo Molnar]

* George Anzinger <george@mvista.com> wrote:

> Also, I think that del_timer_sync and friends need to be turned on if 
> soft_irq is preemptable.

you mean the #ifdef CONFIG_SMP should be:

	#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)

?

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[George Anzinger]

Ingo Molnar wrote:
> * George Anzinger <george@mvista.com> wrote:
> 
> 
>>Also, I think that del_timer_sync and friends need to be turned on if 
>>soft_irq is preemptable.
> 
> 
> you mean the #ifdef CONFIG_SMP should be:
> 
> 	#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
> 
> ?
Yes, exactly.

> 

-- 
George Anzinger   george@mvista.com
High-res-timers:  http://sourceforge.net/projects/high-res-timers/

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[George Anzinger]

[-- Attachment #1: Type: text/plain, Size: 683 bytes --]

Here is the update based on the latest code 
(realtime-preempt-2.6.12-rc5-V0.7.47-09).

George


Ingo Molnar wrote:
> * George Anzinger <george@mvista.com> wrote:
> 
> 
>>Ingo,
>>
>>Could you add the attached patch to the RT patch.  It fixes the memory 
>>corruption issue with the 2timer_test that we have been tracking down.
>>
>>Also, I think that del_timer_sync and friends need to be turned on if 
>>soft_irq is preemptable.
> 
> 
> could you resend this patch, it doesnt apply to current -RT anymore and 
> i'm not brave enough to do a merge of it.
> 
> 	Ingo
> 

-- 
George Anzinger   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

[-- Attachment #2: rt-timers-fix.patch --]
[-- Type: text/plain, Size: 7016 bytes --]

Source: MontaVista Software, Inc.  George Anzinger<george@mvista.com>
MR: 11506
Type: Defect Fix 
Disposition:  needs submitting to RT community patch
Keywords:
Signed-off-by: George Anzinger<george@mvista.com>
Description:

This change adds code to protect timers while they are being accesed by
the timer call back code.  This is now needed because the call back code
is preemptable when ever softirqs are preemptable.  In the past this was
needed only for SMP systems and it is that code which is expanded and
enhanced to account for the possiblility that the timer call back may be
preempted for some period of time.  (In the SMP case this time was
rather short so a spin was accetable.)

 posix-timers.c |   93 ++++++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 63 insertions(+), 30 deletions(-)

Index: linux-2.6.12-rc/kernel/posix-timers.c
===================================================================
--- linux-2.6.12-rc.orig/kernel/posix-timers.c
+++ linux-2.6.12-rc/kernel/posix-timers.c
@@ -94,7 +94,7 @@ static DEFINE_SPINLOCK(idr_lock);
  */
 #define TIMER_INACTIVE 1
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 # define timer_active(tmr) \
 		((tmr)->it.real.timer.entry.prev != (void *)TIMER_INACTIVE)
 # define set_timer_inactive(tmr) \
@@ -102,10 +102,28 @@ static DEFINE_SPINLOCK(idr_lock);
 			(tmr)->it.real.timer.entry.prev = (void *)TIMER_INACTIVE; \
 		} while (0)
 #else
-# define timer_active(tmr) BARFY	// error to use outside of SMP
+# define timer_active(tmr) BARFY   /* error to use outside of SMP | RT */
 # define set_timer_inactive(tmr) do { } while (0)
 #endif
 /*
+ * For RT the timer call backs are preemptable.  This means that folks
+ * trying to delete timers may run into timers that are "active" for
+ * long times.  To help out with this we provide a wake up function to
+ * wake up a caller who wants waking when a timer clears the call back.
+ * This is the same sort of thing that the del_timer_sync does, but we
+ * need (in the HRT case) to cover two lists and not just the one.
+ */
+#ifdef CONFIG_PREEMPT_SOFTIRQS
+#include <linux/wait.h>
+static DECLARE_WAIT_QUEUE_HEAD(timer_wake_queue);
+#define wake_timer_waiters() wake_up(&timer_wake_queue)
+#define wait_for_timer(timer) wait_event(timer_wake_queue, !timer_active(timer))
+
+#else
+#define wake_timer_waiters()
+#define wait_for_timer(timer)
+#endif
+/*
  * we assume that the new SIGEV_THREAD_ID shares no bits with the other
  * SIGEV values.  Here we put out an error if this assumption fails.
  */
@@ -527,6 +545,7 @@ static void posix_timer_fn(unsigned long
 			schedule_next_timer(timr);
 	}
 	unlock_timer(timr, flags); /* hold thru abs lock to keep irq off */
+	wake_timer_waiters();
 }
 
 
@@ -983,18 +1002,20 @@ common_timer_set(struct k_itimer *timr, 
 	 * careful here.  If smp we could be in the "fire" routine which will
 	 * be spinning as we hold the lock.  But this is ONLY an SMP issue.
 	 */
-#ifdef CONFIG_SMP
-	if (timer_active(timr) && !del_timer(&timr->it.real.timer))
-		/*
-		 * It can only be active if on an other cpu.  Since
-		 * we have cleared the interval stuff above, it should
-		 * clear once we release the spin lock.  Of course once
-		 * we do that anything could happen, including the
-		 * complete melt down of the timer.  So return with
-		 * a "retry" exit status.
-		 */
-		return TIMER_RETRY;
-
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
+	if (timer_active(timr) && !del_timer(&timr->it.real.timer)) {
+  		/*
+		 * It can only be active if on an other cpu (unless RT).
+		 * Since we have cleared the interval stuff above, it
+		 * should clear once we release the spin lock.  Of
+		 * course once we do that anything could happen,
+		 * including the complete melt down of the timer.  So
+		 * return with a "retry" exit status.  If RT we do a
+		 * formal wait as the function code is fully
+		 * preemptable...
+  		 */
+  		return TIMER_RETRY;
+	}
 	set_timer_inactive(timr);
 #else
 	del_timer(&timr->it.real.timer);
@@ -1069,7 +1090,8 @@ retry:
 
 	unlock_timer(timr, flag);
 	if (error == TIMER_RETRY) {
-		rtn = NULL;	// We already got the old time...
+		wait_for_timer(timr);
+		rtn = NULL;	/* We already got the old time... */
 		goto retry;
 	}
 
@@ -1083,17 +1105,19 @@ retry:
 static inline int common_timer_del(struct k_itimer *timer)
 {
 	timer->it.real.incr = 0;
-#ifdef CONFIG_SMP
-	if (timer_active(timer) && !del_timer(&timer->it.real.timer))
-		/*
-		 * It can only be active if on an other cpu.  Since
-		 * we have cleared the interval stuff above, it should
-		 * clear once we release the spin lock.  Of course once
-		 * we do that anything could happen, including the
-		 * complete melt down of the timer.  So return with
-		 * a "retry" exit status.
-		 */
-		return TIMER_RETRY;
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
+	if (timer_active(timer) && !del_timer(&timer->it.real.timer)) {
+  		/*
+		 * It can only be active if on an other cpu (unless RT).
+		 * Since we have cleared the interval stuff above, it
+		 * should clear once we release the spin lock.  Of
+		 * course once we do that anything could happen,
+		 * including the complete melt down of the timer.  So
+		 * return with a "retry" exit status.  For RT we do a
+		 * formal wait as it could take a while.
+  		 */
+  		return TIMER_RETRY;
+	}
 #else
 	del_timer(&timer->it.real.timer);
 #endif
@@ -1114,7 +1138,7 @@ sys_timer_delete(timer_t timer_id)
 	struct k_itimer *timer;
 	long flags;
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	int error;
 retry_delete:
 #endif
@@ -1122,7 +1146,7 @@ retry_delete:
 	if (!timer)
 		return -EINVAL;
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	error = timer_delete_hook(timer);
 
 	if (error == TIMER_RETRY) {
@@ -1155,17 +1179,18 @@ static inline void itimer_delete(struct 
 {
 	unsigned long flags;
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	int error;
 retry_delete:
 #endif
 	spin_lock_irqsave(&timer->it_lock, flags);
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
 	error = timer_delete_hook(timer);
 
 	if (error == TIMER_RETRY) {
 		unlock_timer(timer, flags);
+		wait_for_timer(timer);
 		goto retry_delete;
 	}
 #else
@@ -1424,6 +1449,14 @@ void clock_was_set(void)
 		list_del_init(&timr->it.real.abs_timer_entry);
 		if (add_clockset_delta(timr, &new_wall_to) &&
 		    del_timer(&timr->it.real.timer))  /* timer run yet? */
+			/*
+			 * Note that we only do this if the timer is/was
+			 * in the list.  If it happens to be active an
+			 * not in the timer list, it must be in the call
+			 * back function, we leave it to that code to do
+			 * the right thing.  I.e we do NOT need
+			 * del_timer_sync()
+			 */
 			add_timer(&timr->it.real.timer);
 		list_add(&timr->it.real.abs_timer_entry, &abs_list.list);
 		spin_unlock_irq(&abs_list.lock);

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[Ingo Molnar]

* George Anzinger <george@mvista.com> wrote:

> Ingo Molnar wrote:
> >* George Anzinger <george@mvista.com> wrote:
> >
> >
> >>Also, I think that del_timer_sync and friends need to be turned on if 
> >>soft_irq is preemptable.
> >
> >
> >you mean the #ifdef CONFIG_SMP should be:
> >
> >	#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
> >
> >?
> Yes, exactly.

i have done this in -47-09 and it seems to work fine - is it sufficient?

	Ingo

Re: [patch] Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00

[George Anzinger]

Ingo Molnar wrote:
> * George Anzinger <george@mvista.com> wrote:
> 
> 
>>Ingo Molnar wrote:
>>
>>>* George Anzinger <george@mvista.com> wrote:
>>>
>>>
>>>
>>>>Also, I think that del_timer_sync and friends need to be turned on if 
>>>>soft_irq is preemptable.
>>>
>>>
>>>you mean the #ifdef CONFIG_SMP should be:
>>>
>>>	#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
>>>
>>>?
>>
>>Yes, exactly.
> 
> 
> i have done this in -47-09 and it seems to work fine - is it sufficient?

I think so.  Time will tell...

-- 
George Anzinger   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

RT and timers

[Daniel Walker]

George, 

	I wanted to show you the code below, from the RT patch. I think 
it's possible that, if the code isn't changed in the way below, the 
while() loop could run forever. If jiffies is very fast moving , and the 
softirqd is low priority. Do you have any comments on this?

Daniel


@@ -436,13 +437,30 @@ static int cascade(tvec_base_t *base, tv
 static inline void __run_timers(tvec_base_t *base)
 {
        struct timer_list *timer;
+       unsigned long jiffies_sample = jiffies;

        spin_lock_irq(&base->lock);
-       while (time_after_eq(jiffies, base->timer_jiffies)) {
+       while (time_after_eq(jiffies_sample, base->timer_jiffies)) {
                struct list_head work_list = LIST_HEAD_INIT(work_list);
                struct list_head *head = &work_list;
                int index = base->timer_jiffies & TVR_MASK;

Re: RT and timers

[George Anzinger]

Daniel Walker wrote:
> George, 
> 
> 	I wanted to show you the code below, from the RT patch. I think 
> it's possible that, if the code isn't changed in the way below, the 
> while() loop could run forever. If jiffies is very fast moving , and the 
> softirqd is low priority. Do you have any comments on this?

What you are saying is that it is possible that the kernel will not be able to 
keep up the timer list if we lower its priority.  Making this change just allows 
softirqd to do other things and come right back to this code.  If we care at all 
about timers, we should do a better job of setting priorities.

In the end, I think we will want to go to a system where timers are just moved 
to an expired list on the jiffie interrupt (this is very fast except for the 
cascade, as the whole list is moved in one operation).  The expired list would 
then be processed by a dedicated timer delivery thread that shifts its priority 
to match the priority of the highest priority pending timer.

Thomas Gleixner (added to the cc) is currently working on just such a change.

So, in short, I don't see the point to the suggested change.  If the kernel is 
late, it is best to let it catch up as fast as it can by looping here.  The only 
counter argument that makes sense to me it that in this case we are starving 
other softirqd driven tasks, but that should, if any thing, lighten the timer 
load and let this complete faster.

George
-- 


> 
> Daniel
> 
> 
> @@ -436,13 +437,30 @@ static int cascade(tvec_base_t *base, tv
>  static inline void __run_timers(tvec_base_t *base)
>  {
>         struct timer_list *timer;
> +       unsigned long jiffies_sample = jiffies;
> 
>         spin_lock_irq(&base->lock);
> -       while (time_after_eq(jiffies, base->timer_jiffies)) {
> +       while (time_after_eq(jiffies_sample, base->timer_jiffies)) {
>                 struct list_head work_list = LIST_HEAD_INIT(work_list);
>                 struct list_head *head = &work_list;
>                 int index = base->timer_jiffies & TVR_MASK;
> 
> 
> -
> 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/
> 

-- 
George Anzinger   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: RT and timers

[Daniel Walker]

On Thu, 9 Jun 2005, George Anzinger wrote:

> 
> So, in short, I don't see the point to the suggested change.  If the kernel is 
> late, it is best to let it catch up as fast as it can by looping here.  The only 
> counter argument that makes sense to me it that in this case we are starving 
> other softirqd driven tasks, but that should, if any thing, lighten the timer 
> load and let this complete faster.

I'm mainly concerned because that loop never breaks . It seems like there 
could be a condition when the loop never stops. For instance , a very 
accurate timer interrupt, and timers that continually reset themselves.

Daniel 

Re: RT and timers

[George Anzinger]

Daniel Walker wrote:
> On Thu, 9 Jun 2005, George Anzinger wrote:
> 
> 
>>So, in short, I don't see the point to the suggested change.  If the kernel is 
>>late, it is best to let it catch up as fast as it can by looping here.  The only 
>>counter argument that makes sense to me it that in this case we are starving 
>>other softirqd driven tasks, but that should, if any thing, lighten the timer 
>>load and let this complete faster.
> 
> 
> I'm mainly concerned because that loop never breaks . It seems like there 
> could be a condition when the loop never stops. For instance , a very 
> accurate timer interrupt, and timers that continually reset themselves.

As I recall, it is not possible to put a timer in the list for the current time. 
  It will be put in the next tick slot or, with HRT, be passed to the hrt code. 
  The only case this might fail is if a kernel hrt user restarts his timer for 
"now" or prior to "now".  This is bad and hard to correct.  The posix-timers 
code does not restart timers until the signal is delivered and then from the 
user thread, not the softirq context.

Did I miss something here?

-- 
George Anzinger   george@mvista.com
HRT (High-res-timers):  http://sourceforge.net/projects/high-res-timers/

Re: RT and timers

[Thomas Gleixner]

On Thu, 2005-06-09 at 18:36 -0700, George Anzinger wrote:
> > I'm mainly concerned because that loop never breaks . It seems like there 
> > could be a condition when the loop never stops. For instance , a very 
> > accurate timer interrupt, and timers that continually reset themselves.
> 
> As I recall, it is not possible to put a timer in the list for the current time. 
>   It will be put in the next tick slot or, with HRT, be passed to the hrt code. 
>   The only case this might fail is if a kernel hrt user restarts his timer for 
> "now" or prior to "now".  This is bad and hard to correct.  The posix-timers 
> code does not restart timers until the signal is delivered and then from the 
> user thread, not the softirq context.
> 
> Did I miss something here?

No, you're right. Expired timers are inserted into the next slot, so the
loop is not able to starve itself.


tglx