: scheduler complex macros fixes
Subsystems (3)
PATCH 1/1 : scheduler complex macros fixes
The attached patch fixes problems with the "complex" macros in the scheduler, as discussed about a week ago with Ingo on this mailing list. On some platforms (at least IA64, sparc64) the RQ lock must be released before the context switch in order to avoid a potential deadlock. The macros used in 2.5.26 are buggy and allow the load_balancer to steal the prev task right before the context switch occurs. If the prev task exits in the mean time, the kernel crashes.
These fixes are already in Ingo's patches for the SCHED_BATCH extension but didn't make it into 2.5.26. They are a MUST for the platforms using complex macros, please consider applying...
Regards,
Erich
include/asm-arm/system.h +3 -4
@@ -64,10 +64,9 @@ extern asmlinkage void __backtrace(void)struct thread_info ;extern struct task_struct *__switch_to (struct thread_info *,struct thread_info *); -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) +#define prepare_arch_switch (rq, next) do { } while(0) +#define finish_arch_switch (rq, prev) spin_unlock_irq(&(rq)->lock) +#define task_running (rq, p) ((rq)->curr == (p))#define switch_to (prev,next,last) \ do { \
include/asm-i386/system.h +3 -4
@@ -11,10 +11,9 @@struct task_struct ;/* one of the stranger aspects of C forward declarations.. */ extern void FASTCALL (__switch_to (struct task_struct *prev,struct task_struct *next)); -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) +#define prepare_arch_switch (rq, next) do { } while(0) +#define finish_arch_switch (rq, prev) spin_unlock_irq(&(rq)->lock) +#define task_running (rq, p) ((rq)->curr == (p))#define switch_to (prev,next,last) do { \ asm volatile("pushl %%esi\n\t" \
include/asm-ia64/system.h +4 -0
@@ -429,6 +429,10 @@ extern void ia64_load_extra (struct task } while (0)#endif +#define prepare_arch_switch (rq, next) do {spin_lock(&(next)->switch_lock); spin_unlock(&(rq)->lock);} while(0) +#define finish_arch_switch (rq, prev) do {spin_unlock_irq(&(prev)->switch_lock);} while(0) +#define task_running (rq, p) (((rq)->curr == (p)) || spin_is_locked(&(p)->switch_lock)) +#endif /* __KERNEL__ */ #endif /* __ASSEMBLY__ */
include/asm-ppc/system.h +3 -4
@@ -83,10 +83,9 @@ extern void cacheable_memzero(void *p, ustruct device_node ;extern void note_scsi_host (struct device_node *,void *); -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) +#define prepare_arch_switch (rq, next) do { } while(0) +#define finish_arch_switch (rq, prev) spin_unlock_irq(&(rq)->lock) +#define task_running (rq, p) ((rq)->curr == (p))struct task_struct ;extern void __switch_to (struct task_struct *,struct task_struct *);
include/asm-s390/system.h +3 -4
@@ -18,10 +18,9 @@#endif #include <linux/kernel.h> -#define prepare_arch_schedule (prev) do { } while (0) -#define finish_arch_schedule (prev) do { } while (0) -#define prepare_arch_switch (rq) do { } while (0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) +#define prepare_arch_switch (rq, next) do { } while(0) +#define finish_arch_switch (rq, prev) spin_unlock_irq(&(rq)->lock) +#define task_running (rq, p) ((rq)->curr == (p))#define switch_to (prev,next,last) do { \ if (prev == next) \
include/asm-s390x/system.h +3 -4
@@ -18,10 +18,9 @@#endif #include <linux/kernel.h> -#define prepare_arch_schedule (prev) do { } while (0) -#define finish_arch_schedule (prev) do { } while (0) -#define prepare_arch_switch (rq) do { } while (0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) +#define prepare_arch_switch (rq, next) do { } while(0) +#define finish_arch_switch (rq, prev) spin_unlock_irq(&(rq)->lock) +#define task_running (rq, p) ((rq)->curr == (p))#define switch_to (prev,next),last do { \ if (prev == next) \
include/asm-sparc64/system.h +6 -7
@@ -144,13 +144,12 @@ extern void __flushw_user(void);#define flush_user_windows flushw_user#define flush_register_windows flushw_all -#define prepare_arch_schedule (prev) task_lock(prev) -#define finish_arch_schedule (prev) task_unlock(prev) -#define prepare_arch_switch (rq) \ -do { spin_unlock(&(rq)->lock); \ - flushw_all(); \ -} while (0) -#define finish_arch_switch (rq) __sti() +#define prepare_arch_switch (rq, next) do {spin_lock(&(next)->switch_lock); \ + spin_unlock(&(rq)->lock); +flushw_all (); \ + } while(0) +#define finish_arch_switch (rq, prev) do {spin_unlock_irq(&(prev)->switch_lock);} while(0) +#define task_running (rq, p) (((rq)->curr == (p)) || spin_is_locked(&(p)->switch_lock))#ifndef CONFIG_DEBUG_SPINLOCK #define CHECK_LOCKS (PREV ) do { } while(0)
include/asm-x86_64/system.h +3 -4
@@ -13,10 +13,9 @@#define LOCK_PREFIX ""#endif -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) +#define prepare_arch_switch (rq, next) do { } while(0) +#define finish_arch_switch (rq, prev) spin_unlock_irq(&(rq)->lock) +#define task_running (rq, p) ((rq)->curr == (p))#define __STR (x) #x#define STR (x) __STR(x)
include/linux/init_task.h +1 -0
@@ -78,6 +78,7 @@ pending: { NULL, &tsk.pending.head, {{0}}}, \ blocked: {{0}}, \ alloc_lock: SPIN_LOCK_UNLOCKED, \ + switch_lock: SPIN_LOCK_UNLOCKED, \ journal_info: NULL, \ }
include/linux/sched.h +2 -0
@@ -359,6 +359,8 @@ struct task_struct {u32 self_exec_id;/* Protection of (de-)allocation: mm, files, fs, tty */ spinlock_t alloc_lock; +/* context-switch lock */ +spinlock_t switch_lock;/* journalling filesystem info */ void *journal_info;
kernel/fork.c +1 -0
@@ -675,6 +675,7 @@ struct task_struct *do_fork(unsigned loninit_completion (&vfork); }spin_lock_init (&p->alloc_lock); +spin_lock_init (&p->switch_lock);clear_tsk_thread_flag (p,TIF_SIGPENDING );init_sigpending (&p->pending);
kernel/sched.c +10 -12
@@ -306,7 +306,7 @@ void wait_task_inactive(task_t * p) repeat:preempt_disable (); rq =task_rq (p); -if (unlikely (rq->curr == p)) { +if (unlikely (task_running (rq, p))) {cpu_relax ();/* * enable/disable preemption just to make this
@@ -317,7 +317,7 @@ repeat: goto repeat; } rq =task_rq_lock (p, &flags); -if (unlikely (rq->curr == p)) { +if (unlikely (task_running (rq, p))) {task_rq_unlock (rq, &flags);preempt_enable (); goto repeat;
@@ -337,7 +337,7 @@ repeat:*/ void kick_if_running (task_t * p) { -if (p ==task_rq (p)->curr) +if ((task_running (task_rq (p), p)) && (p->cpu !=smp_processor_id ()))resched_task (p); }#endif
@@ -365,7 +365,7 @@ repeat_lock_task:* Fast-migrate the task if it's not running or runnable * currently. Do not violate hard affinity. */ -if (unlikely (sync && (rq->curr != p) && +if (unlikely (sync && !task_running (rq, p) && (task_cpu (p) !=smp_processor_id ()) && (p->cpus_allowed & (1UL <<smp_processor_id ())))) {
@@ -443,8 +443,7 @@ void sched_exit(task_t * p)#if CONFIG_SMP ||CONFIG_PREEMPT asmlinkage void schedule_tail(task_t *prev) { -finish_arch_switch (this_rq ()); -finish_arch_schedule (prev); +finish_arch_switch (this_rq (), prev); }#endif
@@ -646,7 +645,7 @@ skip_queue:#define CAN_MIGRATE_TASK (p,rq,this_cpu) \ ((jiffies - (p)->sleep_timestamp > cache_decay_ticks) && \ - ((p) != (rq)->curr) && \ + ((p) != (rq)->curr) && !task_running(rq, p) && \ ((p)->cpus_allowed & (1UL << (this_cpu))))if (!CAN_MIGRATE_TASK (tmp, busiest, this_cpu)) {
@@ -816,7 +815,6 @@ need_resched: rq =this_rq ();release_kernel_lock (prev,smp_processor_id ()); -prepare_arch_schedule (prev); prev->sleep_timestamp = jiffies;spin_lock_irq (&rq->lock);
@@ -873,11 +871,11 @@ switch_tasks: rq->nr_switches++; rq->curr = next; -prepare_arch_switch (rq); +prepare_arch_switch (rq, next); prev =context_switch (prev, next);barrier (); rq =this_rq (); -finish_arch_switch (rq); +finish_arch_switch (rq, prev); }else spin_unlock_irq (&rq->lock);finish_arch_schedule (prev);
@@ -1106,7 +1104,7 @@ void set_user_nice(task_t *p, long nice)* If the task is running and lowered its priority, * or increased its priority then reschedule its CPU: */ -if ((NICE_TO_PRIO (nice) < p->static_prio) || (p == rq->curr)) +if ((NICE_TO_PRIO (nice) < p->static_prio) ||task_running (rq, p))resched_task (rq->curr); } out_unlock:
@@ -1779,7 +1777,7 @@ void set_cpus_allowed(task_t *p, unsigne* If the task is not on a runqueue (and not running), then * it is sufficient to simply update the task's cpu field. */ -if (!p->array && (p != rq->curr)) { +if (!p->array && !task_running (rq, p)) {set_task_cpu (p,__ffs (p->cpus_allowed));task_rq_unlock (rq, &flags); goto out;
Quoted 2 lines · Erich Focht
The attached patch fixes problems with the "complex" macros in the scheduler, as discussed about a week ago with Ingo on this mailing list.
the fix has been in my tree for some time, the latest version, against 2.5.26 is at:
http://people.redhat.com/mingo/O(1)-scheduler/sched-2.5.26-B7
it has a number of other fixes as well, plus the SCHED_BATCH feature.
Ingo
Hi Ingo,
Quoted 4 lines · Ingo Molnar
Quoted 2 lines · Erich Focht
The attached patch fixes problems with the "complex" macros in the scheduler, as discussed about a week ago with Ingo on this mailing list.
Quoted 8 lines
the fix has been in my tree for some time, the latest version, against 2.5.26 is at:
http://people.redhat.com/mingo/O(1)-scheduler/sched-2.5.26-B7
it has a number of other fixes as well, plus the
SCHED_BATCHfeature.Ingo
Thanks. But as long as SCHED_BATCH ins't in 2.5.X, we need this separate fix. And if it's in the main tree, you don't need it additionally in your patches. The fix is O(1) specific and I thought it should not hide inside the SCHED_BATCH patches, some people might encounter these problems...
Regards,
Erich
Quoted 5 lines · Erich Focht
Thanks. But as long as
SCHED_BATCHins't in 2.5.X, we need this separate fix. And if it's in the main tree, you don't need it additionally in your patches. The fix is O(1) specific and I thought it should not hide inside theSCHED_BATCHpatches, some people might encounter these problems...
well, SCHED_BATCH is in fact ready, so we might as well put it in. All the suggestions mentioned on lkml (or in private) are now included, there are no pending (known) problems, no objections, and a number of people are using it with success.
Ingo
Quoted 5 lines · Ingo Molnar
well,
SCHED_BATCHis in fact ready, so we might as well put it in. All the suggestions mentioned on lkml (or in private) are now included, there are no pending (known) problems, no objections, and a number of people are using it with success.
Well, I do actually have objections, mainly because I think it modifies code that I'd rather have cleaned up in other ways _first_ (ie the return path to user mode, which has pending irq_count/bh_count/preempt issues that I consider to be about a million times more important than batch scheduling).
So I'd rather have the non-batch stuff done first and independently.
Linus
Quoted 6 lines · Linus Torvalds
Quoted 4 lines · Ingo Molnar
well,
SCHED_BATCHis in fact ready, so we might as well put it in. All the suggestions mentioned on lkml (or in private) are now included, there are no pending (known) problems, no objections, and a number of people are using it with success.
Quoted 6 lines
Well, I do actually have objections, mainly because I think it modifies code that I'd rather have cleaned up in other ways _first_ (ie the return path to user mode, which has pending irq_count/bh_count/preempt issues that I consider to be about a million times more important than batch scheduling).
Thats too bad. I've been looking at Ingo's SCHED_BATCH design to help suspend processes with out lock problems while doing multithreaded core dumps.
Is there any ETA on the return path to user mode clean up?
--mgross
Quoted 5 lines · mgross
Thats too bad. I've been looking at Ingo's
SCHED_BATCHdesign to help suspend processes with out lock problems while doing multithreaded core dumps.Is there any ETA on the return path to user mode clean up?
oh, there's no fundamental problem here, i'm doing the return-path-cleanup patch too :-) [which, as a side-effect, also removes the global irq lock.] I temporarily suspended it for the sake of the SCHED_BATCH feature and other scheduler maintainance work. But it just got higher priority, so ...
Ingo
Quoted 1 line · Linus Torvalds
So I'd rather have the non-batch stuff done first and independently.
sure - all the non-batch bits are attached, against 2.5.26-vanilla. It compiles & boots just fine on UP & SMP as well.
Ingo
include/linux/sched.h renamed from include/linux/sched.h.orig +7 -3
@@ -116,10 +116,11 @@/* * Scheduling policies */ -#define SCHED_OTHER 0 +#define SCHED_NORMAL 0#define SCHED_FIFO 1#define SCHED_RR 2 +struct sched_param {int sched_priority; };
@@ -207,7 +209,7 @@/* * Priority of a process goes from 0..MAX_PRIO-1, valid RT -* priority is 0..MAX_RT_PRIO-1, and SCHED_OTHER tasks are +* priority is 0..MAX_RT_PRIO-1, and SCHED_NORMAL tasks are * in the range MAX_RT_PRIO..MAX_PRIO-1. Priority values * are inverted: lower p->prio value means higher priority. *
@@ -264,7 +266,7 @@unsigned long policy;unsigned long cpus_allowed; -unsigned int time_slice; +unsigned int time_slice, first_time_slice;struct list_head tasks;
@@ -359,6 +361,8 @@u32 self_exec_id;/* Protection of (de-)allocation: mm, files, fs, tty */ spinlock_t alloc_lock; +/* context-switch lock */ +spinlock_t switch_lock;/* journalling filesystem info */ void *journal_info;
@@ -391,6 +395,7 @@#define PF_IOTHREAD 0x00020000/* this thread is needed for doing I/O to swap */ #define PF_FROZEN 0x00040000/* frozen for system suspend */ #define PF_INVALIDATE 0x00080000/* debug: unmounting an fs. killme. */ +/* * Ptrace flags */
include/linux/init_task.h renamed from include/linux/init_task.h.orig +2 -1
@@ -47,7 +47,7 @@ lock_depth: -1, \ prio: MAX_PRIO-20, \ static_prio: MAX_PRIO-20, \ - policy: SCHED_OTHER, \ + policy: SCHED_NORMAL, \ cpus_allowed: -1, \ mm: NULL, \ active_mm: &init_mm, \
@@ -78,6 +78,7 @@ pending: { NULL, &tsk.pending.head, {{0}}}, \ blocked: {{0}}, \ alloc_lock: SPIN_LOCK_UNLOCKED, \ + switch_lock: SPIN_LOCK_UNLOCKED, \ journal_info: NULL, \ }
include/asm-x86_64/system.h renamed from include/asm-x86_64/system.h.orig +0 -5
@@ -13,11 +13,6 @@#define LOCK_PREFIX ""#endif -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) -#define __STR (x) #x#define STR (x) __STR(x)
include/asm-ppc/system.h renamed from include/asm-ppc/system.h.orig +0 -5
@@ -83,11 +83,6 @@struct device_node ;extern void note_scsi_host (struct device_node *,void *); -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) -struct task_struct ;extern void __switch_to (struct task_struct *,struct task_struct *);#define switch_to (prev, next, last) __switch_to((prev), (next))
include/asm-s390x/system.h renamed from include/asm-s390x/system.h.orig +0 -5
@@ -18,11 +18,6 @@#endif #include <linux/kernel.h> -#define prepare_arch_schedule (prev) do { } while (0) -#define finish_arch_schedule (prev) do { } while (0) -#define prepare_arch_switch (rq) do { } while (0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) -#define switch_to (prev,next),last do { \ if (prev == next) \ break; \
include/asm-s390/system.h renamed from include/asm-s390/system.h.orig +0 -5
@@ -18,11 +18,6 @@#endif #include <linux/kernel.h> -#define prepare_arch_schedule (prev) do { } while (0) -#define finish_arch_schedule (prev) do { } while (0) -#define prepare_arch_switch (rq) do { } while (0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) -#define switch_to (prev,next,last) do { \ if (prev == next) \ break; \
include/asm-sparc64/system.h renamed from include/asm-sparc64/system.h.orig +8 -6
@@ -144,13 +144,15 @@#define flush_user_windows flushw_user#define flush_register_windows flushw_all -#define prepare_arch_schedule (prev) task_lock(prev) -#define finish_arch_schedule (prev) task_unlock(prev) -#define prepare_arch_switch (rq) \ -do { spin_unlock(&(rq)->lock); \ - flushw_all(); \ +#define prepare_arch_switch (rq, next) \ +do { spin_lock(&(next)->switch_lock); \ + spin_unlock(&(rq)->lock); \ + flushw_all(); \ +} while (0) + +#define finish_arch_switch (rq, prev) \ +do { spin_unlock_irq(&(prev)->switch_lock); \ } while (0) -#define finish_arch_switch (rq) __sti()#ifndef CONFIG_DEBUG_SPINLOCK #define CHECK_LOCKS (PREV ) do { } while(0)
include/asm-i386/system.h renamed from include/asm-i386/system.h.orig +0 -5
@@ -11,11 +11,6 @@struct task_struct ;/* one of the stranger aspects of C forward declarations.. */ extern void FASTCALL (__switch_to (struct task_struct *prev,struct task_struct *next)); -#define prepare_arch_schedule (prev) do { } while(0) -#define finish_arch_schedule (prev) do { } while(0) -#define prepare_arch_switch (rq) do { } while(0) -#define finish_arch_switch (rq) spin_unlock_irq(&(rq)->lock) -#define switch_to (prev,next,last) do { \ asm volatile("pushl %%esi\n\t" \ "pushl %%edi\n\t" \
kernel/timer.c renamed from kernel/timer.c.orig +1 -1
@@ -888,7 +888,7 @@if (t.tv_sec ==0 && t.tv_nsec <=2000000L && - current->policy !=SCHED_OTHER ) + current->policy !=SCHED_NORMAL ) {/* * Short delay requests up to 2 ms will be handled with
kernel/fork.c renamed from kernel/fork.c.orig +7 -3
@@ -615,7 +615,6 @@ unsigned long stack_size) {int retval; -unsigned long flags;struct task_struct *p =NULL ;struct completion vfork;
@@ -675,6 +674,7 @@init_completion (&vfork); }spin_lock_init (&p->alloc_lock); +spin_lock_init (&p->switch_lock);clear_tsk_thread_flag (p,TIF_SIGPENDING );init_sigpending (&p->pending);
@@ -737,9 +737,13 @@* total amount of pending timeslices in the system doesnt change, * resulting in more scheduling fairness. */ -__save_flags (flags);__cli (); p->time_slice = (current->time_slice +1 ) >>1 ; +/* +* The remainder of the first timeslice might be recovered by +* the parent if the child exits early enough. +*/ + p->first_time_slice =1 ; current->time_slice >>=1 ;if (!current->time_slice) {/*
@@ -751,7 +755,7 @@scheduler_tick (0 ,0 ); } p->sleep_timestamp = jiffies; -__restore_flags (flags); +__sti ();/* * Ok, add it to the run-queues and make it
kernel/sched.c renamed from kernel/sched.c.orig +218 -194
678 lines not on this page · raw message
kernel/signal.c renamed from kernel/signal.c.orig +1 -3
@@ -500,7 +500,6 @@ {set_tsk_thread_flag (t,TIF_SIGPENDING ); -#ifdef CONFIG_SMP /* * If the task is running on a different CPU * force a reschedule on the other CPU to make
@@ -511,9 +510,8 @@* process of changing - but no harm is done by that * other than doing an extra (lightweight) IPI interrupt. */ -if ((t->state ==TASK_RUNNING ) && (t->thread_info->cpu !=smp_processor_id ())) +if (t->state ==TASK_RUNNING )kick_if_running (t); -#endif if (t->state &TASK_INTERRUPTIBLE ) {wake_up_process (t);return ;
kernel/exit.c renamed from kernel/exit.c.orig +1 -1
@@ -182,7 +182,7 @@ current->exit_signal =SIGCHLD ; current->ptrace =0 ; -if ((current->policy ==SCHED_OTHER ) && (task_nice (current) <0 )) +if ((current->policy ==SCHED_NORMAL ) && (task_nice (current) <0 ))set_user_nice (current,0 );/* cpus_allowed? */ /* rt_priority? */
Hi Ingo,
Quoted 7 lines · Ingo Molnar
Quoted 1 line · Linus Torvalds
So I'd rather have the non-batch stuff done first and independently.
sure - all the non-batch bits are attached, against 2.5.26-vanilla. It compiles & boots just fine on UP & SMP as well.
Ingo
thanks for the complete update without the SCHED_BATCH! I was too lazy to strip out all your changes and I'm sure I would have made mistakes...
Best regards,
Erich