the latest irqlock patch can be found at:
http://redhat.com/~mingo/remove-irqlock-patches/remove-irqlock-2.5.27-H4
Changes in -H4:
- fix the cli()/sti() hack in ide/main.c, per Marcin Dalecki's
suggestion. [this leaves the tty layer as the only remaining subsystem
that still has cli()/sti() related hacks.]
Changes in -H3:
- init thread needs to have preempt_count of 1 until sched_init().
(William Lee Irwin III)
- clean up the irq-mask macros. (Linus)
- add barrier() to irq_enter() and irq_exit(). (based on Oleg Nesterov's
comment.)
- move the irqs-off check into preempt_schedule() and remove
CONFIG_DEBUG_IRQ_SCHEDULE.
Changes in -G5:
- remove spin_unlock_no_resched() and comment the affected places more
agressively.
Changes in -G3:
- slab.c needs to spin_unlock_no_resched(), instead of spin_unlock(). (It
also has to check for preemption in the right spot.) This should fix
the memory corruption.
- irq_exit() needs to run softirqs if interrupts not active - in the
previous patch it ran them when preempt_count() was 0, which is
incorrect.
- spinlock macros are updated to enable preemption after enabling
interrupts. Besides avoiding false positive warnings, this also
- fork.c has to call scheduler_tick() with preemption disabled -
otherwise scheduler_tick()'s spin_unlock can preempt!
- irqs_disabled() macro introduced.
- [ all other local_irq_enable() or sti instances conditional on
CONFIG_DEBUG_IRQ_SCHEDULE are to fix false positive warnings. ]
Changes in -G0:
- fix buggy in_softirq(). Fortunately the bug made the test broader,
which didnt result in algorithmical breakage, just suboptimal
performance.
- move do_softirq() processing into irq_exit() => this also fixes the
softirq processing bugs present in apic.c IRQ handlers that did not
test for softirqs after irq_exit().
- simplify local_bh_enable().
Changes in -F9:
- replace all instances of:
local_save_flags(flags);
local_irq_disable();
with the shorter form of:
local_irq_save(flags);
about 30 files are affected by this change.
Changes in -F8:
- preempt/hardirq/softirq count separation, cleanups.
- skbuff.c fix.
- use irq_count() in scheduler_tick()
Changes in -F3:
- the entry.S cleanups/speedups by Oleg Nesterov.
- a rather critical synchronize_irq() bugfix: if a driver frees an
interrupt that is still being probed then synchronize_irq() locks up.
This bug has caused a spurious boot-lockup on one of my testsystems,
ifconfig would lock up trying to close eth0.
- remove duplicate definitions from asm-i386/system.h, this fixes
compiler warnings.
Ingo
> - move the irqs-off check into preempt_schedule() and remove
> CONFIG_DEBUG_IRQ_SCHEDULE.
the config.in entry is still present..
On Wed, 24 Jul 2002, Christoph Hellwig wrote:
> > - move the irqs-off check into preempt_schedule() and remove
> > CONFIG_DEBUG_IRQ_SCHEDULE.
>
> the config.in entry is still present..
indeed. Fix is in -H5:
http://redhat.com/~mingo/remove-irqlock-patches/remove-irqlock-2.5.27-H5
Ingo
On Wed, 24 Jul 2002, Ingo Molnar wrote:
> the latest irqlock patch can be found at:
>
> http://redhat.com/~mingo/remove-irqlock-patches/remove-irqlock-2.5.27-H4
>
> Changes in -H4:
>
> - fix the cli()/sti() hack in ide/main.c, per Marcin Dalecki's
> suggestion. [this leaves the tty layer as the only remaining subsystem
> that still has cli()/sti() related hacks.]
Hi Ingo,
Marcin's suggestions will bring you nowhere.
You should remove all these locking from ide_unregister_subdriver()
because in 100% cases it is already called with ide_lock held.
Also in ide subsystem ide-tape.c needs fixing, however it is already
broken and proper locking fixing may be non-trivial task.
Regards
--
Bartlomiej
Bartlomiej Zolnierkiewicz wrote:
> On Wed, 24 Jul 2002, Ingo Molnar wrote:
>
>
>>the latest irqlock patch can be found at:
>>
>> http://redhat.com/~mingo/remove-irqlock-patches/remove-irqlock-2.5.27-H4
>>
>>Changes in -H4:
>>
>> - fix the cli()/sti() hack in ide/main.c, per Marcin Dalecki's
>> suggestion. [this leaves the tty layer as the only remaining subsystem
>> that still has cli()/sti() related hacks.]
>
>
> Hi Ingo,
>
> Marcin's suggestions will bring you nowhere.
>
> You should remove all these locking from ide_unregister_subdriver()
> because in 100% cases it is already called with ide_lock held.
Indeed they can be just removed.
On Wed, 24 Jul 2002, Marcin Dalecki wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > On Wed, 24 Jul 2002, Ingo Molnar wrote:
> >
> >
> >>the latest irqlock patch can be found at:
> >>
> >> http://redhat.com/~mingo/remove-irqlock-patches/remove-irqlock-2.5.27-H4
> >>
> >>Changes in -H4:
> >>
> >> - fix the cli()/sti() hack in ide/main.c, per Marcin Dalecki's
> >> suggestion. [this leaves the tty layer as the only remaining subsystem
> >> that still has cli()/sti() related hacks.]
> >
> >
> > Hi Ingo,
> >
> > Marcin's suggestions will bring you nowhere.
> >
> > You should remove all these locking from ide_unregister_subdriver()
> > because in 100% cases it is already called with ide_lock held.
>
> Indeed they can be just removed.
[bugging mode still on ;-)))]
No, they _have to_ be removed, you introduced bug with your fix.
Regards
--
Bartlomeij
diff -urP -I \$Id:.*Exp \$ -X /usr/src/patch.exclude linux-2.5.27-ingo-H6-org/arch/i386/config.in linux/arch/i386/config.in
--- linux-2.5.27-ingo-H6-org/arch/i386/config.in Mon Jul 22 14:19:20 2002
+++ linux/arch/i386/config.in Wed Jul 24 12:13:56 2002
@@ -412,6 +412,7 @@
bool 'Kernel debugging' CONFIG_DEBUG_KERNEL
if [ "$CONFIG_DEBUG_KERNEL" != "n" ]; then
+ bool ' Debug IRQ preempt interaction' CONFIG_DEBUG_IRQ
bool ' Debug memory allocations' CONFIG_DEBUG_SLAB
bool ' Memory mapped I/O debugging' CONFIG_DEBUG_IOVIRT
bool ' Magic SysRq key' CONFIG_MAGIC_SYSRQ
diff -urP -I \$Id:.*Exp \$ -X /usr/src/patch.exclude linux-2.5.27-ingo-H6-org/include/asm-i386/system.h linux/include/asm-i386/system.h
--- linux-2.5.27-ingo-H6-org/include/asm-i386/system.h Wed Jul 24 12:50:03 2002
+++ linux/include/asm-i386/system.h Wed Jul 24 12:36:08 2002
@@ -312,9 +312,26 @@
/* interrupt control.. */
#define local_save_flags(x) __asm__ __volatile__("pushfl ; popl %0":"=g" (x): /* no input */)
-#define local_irq_restore(x) __asm__ __volatile__("pushl %0 ; popfl": /* no output */ :"g" (x):"memory", "cc")
+#define _local_irq_restore(x) __asm__ __volatile__("pushl %0 ; popfl": /* no output */ :"g" (x):"memory", "cc")
#define local_irq_disable() __asm__ __volatile__("cli": : :"memory")
-#define local_irq_enable() __asm__ __volatile__("sti": : :"memory")
+#define _local_irq_enable() __asm__ __volatile__("sti": : :"memory")
+#ifdef CONFIG_DEBUG_IRQ
+#define test_preempt() if (unlikely(! current_thread_info()->preempt_count)){ \
+ printk("bad: irq_restore() with preemption_enabled!\n");\
+ show_stack(NULL); }
+#else
+#define test_preempt()
+#endif
+#define local_irq_enable() \
+ do { _local_irq_enable(); test_preempt(); } while(0)
+#define local_irq_restore(x) \
+ do { _local_irq_restore(x); test_preempt(); } while(0)
+#define local_irq_enable_preempt() \
+ do { _local_irq_enable(); preempt_check_resched() ; } while(0)
+#define local_irq_restore_preempt(x) \
+ do { _local_irq_restore(x); preempt_check_resched(); } while(0)
+
+
/* used in the idle loop; sti takes one instruction cycle to complete */
#define safe_halt() __asm__ __volatile__("sti; hlt": : :"memory")
diff -urP -I \$Id:.*Exp \$ -X /usr/src/patch.exclude linux-2.5.27-ingo-H6-org/include/linux/spinlock.h linux/include/linux/spinlock.h
--- linux-2.5.27-ingo-H6-org/include/linux/spinlock.h Wed Jul 24 12:50:03 2002
+++ linux/include/linux/spinlock.h Wed Jul 24 12:39:39 2002
@@ -26,17 +26,17 @@
#define write_lock_irq(lock) do { local_irq_disable(); write_lock(lock); } while (0)
#define write_lock_bh(lock) do { local_bh_disable(); write_lock(lock); } while (0)
-#define spin_unlock_irqrestore(lock, flags) do { _raw_spin_unlock(lock); local_irq_restore(flags); preempt_enable(); } while (0)
+#define spin_unlock_irqrestore(lock, flags) do { _raw_spin_unlock(lock); local_irq_restore_preempt(flags); } while (0)
#define _raw_spin_unlock_irqrestore(lock, flags) do { _raw_spin_unlock(lock); local_irq_restore(flags); } while (0)
-#define spin_unlock_irq(lock) do { _raw_spin_unlock(lock); local_irq_enable(); preempt_enable(); } while (0)
+#define spin_unlock_irq(lock) do { _raw_spin_unlock(lock); local_irq_enable_preempt(); } while (0)
#define spin_unlock_bh(lock) do { spin_unlock(lock); local_bh_enable(); } while (0)
-#define read_unlock_irqrestore(lock, flags) do { _raw_read_unlock(lock); local_irq_restore(flags); preempt_enable(); } while (0)
-#define read_unlock_irq(lock) do { _raw_read_unlock(lock); local_irq_enable(); preempt_enable(); } while (0)
+#define read_unlock_irqrestore(lock, flags) do { _raw_read_unlock(lock); local_irq_restore_preempt(flags); } while (0)
+#define read_unlock_irq(lock) do { _raw_read_unlock(lock); local_irq_enable_preempt(); } while (0)
#define read_unlock_bh(lock) do { read_unlock(lock); local_bh_enable(); } while (0)
-#define write_unlock_irqrestore(lock, flags) do { _raw_write_unlock(lock); local_irq_restore(flags); preempt_enable(); } while (0)
-#define write_unlock_irq(lock) do { _raw_write_unlock(lock); local_irq_enable(); preempt_enable(); } while (0)
+#define write_unlock_irqrestore(lock, flags) do { _raw_write_unlock(lock); local_irq_restore_preempt(flags); } while (0)
+#define write_unlock_irq(lock) do { _raw_write_unlock(lock); local_irq_enable_preempt(); } while (0)
#define write_unlock_bh(lock) do { write_unlock(lock); local_bh_enable(); } while (0)
#define spin_trylock_bh(lock) ({ int __r; local_bh_disable();\
__r = spin_trylock(lock); \
diff -urP -I \$Id:.*Exp \$ -X /usr/src/patch.exclude linux-2.5.27-ingo-H6-org/kernel/fork.c linux/kernel/fork.c
--- linux-2.5.27-ingo-H6-org/kernel/fork.c Wed Jul 24 12:50:03 2002
+++ linux/kernel/fork.c Wed Jul 24 12:46:52 2002
@@ -753,10 +753,8 @@
current->time_slice = 1;
preempt_disable();
scheduler_tick(0, 0);
- local_irq_restore(flags);
- preempt_enable();
- } else
- local_irq_restore(flags);
+ }
+ local_irq_restore_preempt(flags);
/*
* Ok, add it to the run-queues and make it
diff -urP -I \$Id:.*Exp \$ -X /usr/src/patch.exclude linux-2.5.27-ingo-H6-org/kernel/sched.c linux/kernel/sched.c
--- linux-2.5.27-ingo-H6-org/kernel/sched.c Wed Jul 24 12:50:03 2002
+++ linux/kernel/sched.c Wed Jul 24 12:12:02 2002
@@ -907,6 +907,7 @@
printk("bad: schedule() with irqs disabled!\n");
show_stack(NULL);
preempt_enable_no_resched();
+ return;
}
need_resched:
diff -urP -I \$Id:.*Exp \$ -X /usr/src/patch.exclude linux-2.5.27-ingo-H6-org/mm/slab.c linux/mm/slab.c
--- linux-2.5.27-ingo-H6-org/mm/slab.c Wed Jul 24 12:50:03 2002
+++ linux/mm/slab.c Wed Jul 24 12:38:27 2002
@@ -1386,9 +1386,8 @@
} else {
STATS_INC_ALLOCMISS(cachep);
objp = kmem_cache_alloc_batch(cachep,flags);
- local_irq_restore(save_flags);
/* end of non-preemptible region */
- preempt_enable();
+ local_irq_restore_preempt(save_flags);
if (!objp)
goto alloc_new_slab_nolock;
return objp;
Hello.
arch/i386/kernel/microcode.c:do_microcode_update() calls
smp_call_function(do_update_one). do_update_one() does
spin_lock/unlock. So smp_call_function_interrupt() needs
irq_enter().
entry.S has unneeded GET_THREAD_INFO(%ebx) in
device_not_available() trap.
Patch against 2.5.28.
--- arch/i386/kernel/smp.c~ Fri Jul 26 07:49:03 2002
+++ arch/i386/kernel/smp.c Fri Jul 26 07:53:45 2002
@@ -646,7 +646,10 @@
/*
* At this point the info structure may be out of scope unless wait==1
*/
+ irq_enter();
(*func)(info);
+ irq_exit();
+
if (wait) {
mb();
atomic_inc(&call_data->finished);
--- arch/i386/kernel/entry.S~ Fri Jul 26 07:49:03 2002
+++ arch/i386/kernel/entry.S Fri Jul 26 07:57:48 2002
@@ -417,7 +417,6 @@
ENTRY(device_not_available)
pushl $-1 # mark this as an int
SAVE_ALL
- GET_THREAD_INFO(%ebx)
movl %cr0, %eax
testl $0x4, %eax # EM (math emulation bit)
jne device_not_available_emulate
Oleg.
Hello.
Oleg Nesterov wrote:
> + irq_enter();
> (*func)(info);
> + irq_exit();
It is better to update call_data->finished before
doing soft irqs in irq_exit().
Against 2.5.28:
--- arch/i386/kernel/smp.c~ Fri Jul 26 07:49:03 2002
+++ arch/i386/kernel/smp.c Fri Jul 26 09:29:20 2002
@@ -646,10 +646,12 @@
/*
* At this point the info structure may be out of scope unless wait==1
*/
+ irq_enter();
(*func)(info);
if (wait) {
mb();
atomic_inc(&call_data->finished);
}
+ irq_exit();
}
--- arch/i386/kernel/entry.S~ Fri Jul 26 07:49:03 2002
+++ arch/i386/kernel/entry.S Fri Jul 26 07:57:48 2002
@@ -417,7 +417,6 @@
ENTRY(device_not_available)
pushl $-1 # mark this as an int
SAVE_ALL
- GET_THREAD_INFO(%ebx)
movl %cr0, %eax
testl $0x4, %eax # EM (math emulation bit)
jne device_not_available_emulate
Oleg.