Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00
PATCH 1/1 Real-Time Preemption, -RT-2.6.12-rc4-V0.7.47-00
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
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?
Signature · 2 lines
George Anzinger [email protected] High-res-timers: http://sourceforge.net/projects/high-res-timers/
Source: MontaVista Software, Inc. George Anzinger<[email protected]>
MR: 11506
Type: Defect Fix
Disposition: needs submitting to RT community patch Keywords:
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(-)
kernel/posix-timers.c +58 -24
@@ -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 longschedule_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);
Quoted 2 lines · George Anzinger
Also, I think that
del_timer_syncand friends need to be turned on ifsoft_irqis preemptable.
you mean the #ifdef CONFIG_SMP should be:
#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)
?
Ingo
Quoted 11 lines · Ingo Molnar
Quoted 1 line · George Anzinger
Also, I think that
del_timer_syncand friends need to be turned on ifsoft_irqis preemptable.you mean the #ifdef
CONFIG_SMPshould be:#if defined(CONFIG_SMP) || defined(CONFIG_PREEMPT_SOFTIRQS)?
Yes, exactly.
Quoted 1 line
Signature · 2 lines
George Anzinger [email protected] High-res-timers: http://sourceforge.net/projects/high-res-timers/
Quoted 8 lines · George Anzinger
Quoted 7 lines · Ingo Molnar
Quoted 2 lines · George Anzinger
Also, I think that
del_timer_syncand friends need to be turned on ifsoft_irqis preemptable.
Quoted 6 lines
Quoted 5 lines
you mean the #ifdef
CONFIG_SMPshould 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
Quoted 14 lines · Ingo Molnar
Quoted 11 lines · George Anzinger
Quoted 9 lines · Ingo Molnar
Quoted 1 line · George Anzinger
Also, I think that
del_timer_syncand friends need to be turned on ifsoft_irqis preemptable.you mean the #ifdef
CONFIG_SMPshould be:
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
#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...
Signature · 2 lines
George Anzinger [email protected] HRT (High-res-timers): http://sourceforge.net/projects/high-res-timers/
RT and timers
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
Quoted 4 lines · 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, thewhile()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
Quoted 24 lines
Daniel
(quoted diff) +2
-2@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Signature · 2 lines
George Anzinger [email protected] HRT (High-res-timers): http://sourceforge.net/projects/high-res-timers/
Re: RT and timers
Quoted 6 lines · George Anzinger
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
Quoted 7 lines · Daniel Walker
Quoted 1 line · George Anzinger
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?
Signature · 2 lines
George Anzinger [email protected] HRT (High-res-timers): http://sourceforge.net/projects/high-res-timers/
Re: RT and timers
Quoted 4 lines · George Anzinger
Quoted 3 lines
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.
Quoted 8 lines
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
A message that is not in the archive (sent off-list or not yet received): [email protected]
Here is the update based on the latest code (realtime-preempt-2.6.12-rc5-V0.7.47-09).
George
Quoted 14 lines · Ingo Molnar
Quoted 5 lines · George Anzinger
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_syncand friends need to be turned on ifsoft_irqis 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
Signature · 2 lines
George Anzinger [email protected] HRT (High-res-timers): http://sourceforge.net/projects/high-res-timers/
Source: MontaVista Software, Inc. George Anzinger<[email protected]>
MR: 11506
Type: Defect Fix
Disposition: needs submitting to RT community patch Keywords:
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(-)
kernel/posix-timers.c +63 -30
@@ -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 longschedule_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);