rc2-mm2: unneeded cli/sti in fix-preemption-race patch
PATCH 1/1 rc2-mm2: unneeded cli/sti in fix-preemption-race patch
Hello.
Quoted 3 lines · Thomas Gleixner
The patch prevents this by leaving interrupts disabled and calling a seperate function
preempt_schedule_irq().
sched-fix-preemption-race-core-i386.patch:
Quoted 7 lines
+++ 25-akpm/arch/i386/kernel/entry.S @@ -189,6 +189,7 @@ ENTRY(resume_userspace) #ifdef CONFIG_PREEMPT ENTRY(resume_kernel) + cli
I think this 'cli' is unneeded. resume_kernel: is used on return path from do_IRQ() which leaves interrupts disabled. irq_desc->handler->end() should not enable interrupts, yes?
And we have preempt_stop==cli in ret_from_exception: case, before ret_from_intr.
Quoted 8 lines
+++ 25-akpm/kernel/sched.c
@@ -2872,6 +2872,48 @@ need_resched:
...
+asmlinkage void __sched preempt_schedule_irq(void)
+{
...
+ local_irq_enable();
+ schedule();It is ok to enter schedule() with interrupts disabled, so this 'sti' seems to be unneeded too.
Am I missed something?
arch/i386/kernel/entry.S renamed from arch/i386/kernel/entry.S~ +0 -1
@@ -189,7 +189,6 @@ ENTRY(resume_userspace) #ifdef CONFIG_PREEMPT ENTRY(resume_kernel) - cli cmpl $0,TI_preempt_count(%ebp) # non-zero preempt_count ? jnz restore_all need_resched:
kernel/sched.c renamed from kernel/sched.c~ +0 -1
@@ -2971,7 +2971,6 @@ need_resched: saved_lock_depth = task->lock_depth; task->lock_depth =-1 ;#endif -local_irq_enable ();schedule ();local_irq_disable ();#ifdef CONFIG_PREEMPT_BKL
Quoted 16 lines · Oleg Nesterov
Hello.
Quoted 3 lines · Thomas Gleixner
The patch prevents this by leaving interrupts disabled and calling a seperate function
preempt_schedule_irq().sched-fix-preemption-race-core-i386.patch:
Quoted 7 lines
+++ 25-akpm/arch/i386/kernel/entry.S @@ -189,6 +189,7 @@ ENTRY(resume_userspace) #ifdef CONFIG_PREEMPT ENTRY(resume_kernel) + cli
Quoted 6 lines
I think this 'cli' is unneeded.
resume_kernel: is used on return path fromdo_IRQ()which leaves interrupts disabled.irq_desc->handler->end()should not enable interrupts, yes?And we have preempt_stop==cli in
ret_from_exception: case, beforeret_from_intr.
Right. I missed the preempt_stop == cli.
Quoted 11 lines
Quoted 8 lines
+++ 25-akpm/kernel/sched.c @@ -2872,6 +2872,48 @@ need_resched: ... +asmlinkage void __sched preempt_schedule_irq(void) +{ ... + local_irq_enable(); + schedule();It is ok to enter
schedule()with interrupts disabled, so this 'sti' seems to be unneeded too.
Makes sense.
tglx
Quoted 22 lines
Signed-off-by Oleg Nesterov <[email protected]>arch/i386/kernel/entry.S renamed from arch/i386/kernel/entry.S~ +0
-1@@ -189,7 +189,6 @@ ENTRY(resume_userspace) #ifdef CONFIG_PREEMPT ENTRY(resume_kernel) - cli cmpl $0,TI_preempt_count(%ebp) # non-zero preempt_count ? jnz restore_all need_resched:kernel/sched.c renamed from kernel/sched.c~ +0
-1@@ -2971,7 +2971,6 @@ need_resched: saved_lock_depth = task->lock_depth; task->lock_depth =-1 ;#endif -local_irq_enable ();schedule ();local_irq_disable ();#ifdef CONFIG_PREEMPT_BKL