>On Fri, 18 Dec 1998, Andrea Arcangeli wrote:
>
>> On Thu, 17 Dec 1998, Linus Torvalds wrote:
>>
>> > - disable_irq()/enable_irq() now nests properly, as Alan convinced me
>> > (quite rightly) that they have to nest in order to work sanely with
>> > shared interrupt and multiple CPU's and various other schenarios.
>>
>> The disable_irq depth patch cause my kernel to crash under X at the first
>> PC beep. In drivers/char/*. there aren't suspect/obvious
>> disable/enable_irq() misusage though...
>>
>> To generate a beep I launch emacs and I press C^g.
>
>i think the kernel should moan about counter underflows. Does it work if
>you fix it this way?
>
> (something like {if(!counter) moan();} in irq.c line 706 or so ;)
>
>just like we complain about dentry and buffer counter underflows ...
Done.
And the lockup was due a change I done in do_gettimeofday() that was doing
a disable_irq() enable_irq() instead of a global cli(). Now everything is
fine here too. The reason of the lockup was that the timer irq was stopped
and so there was not timer running anymore... etc...
This patch fix the old bug in the show() emergency code and is more
pedantic about depth underflows.
Index: linux/arch/i386/kernel/irq.c
diff -u linux/arch/i386/kernel/irq.c:1.1.1.2 linux/arch/i386/kernel/irq.c:1.1.1.1.2.5
--- linux/arch/i386/kernel/irq.c:1.1.1.2 Fri Dec 18 10:16:20 1998
+++ linux/arch/i386/kernel/irq.c Fri Dec 18 15:30:18 1998
@@ -335,16 +335,17 @@
int i;
unsigned long *stack;
int cpu = smp_processor_id();
+ extern char *get_options(char *str, int *ints);
printk("\n%s, CPU %d:\n", str, cpu);
printk("irq: %d [%d %d]\n",
atomic_read(&global_irq_count), local_irq_count[0], local_irq_count[1]);
printk("bh: %d [%d %d]\n",
atomic_read(&global_bh_count), local_bh_count[0], local_bh_count[1]);
- stack = (unsigned long *) &str;
+ stack = (unsigned long *) &stack;
for (i = 40; i ; i--) {
unsigned long x = *++stack;
- if (x > (unsigned long) &init_task_union && x < (unsigned long) &vsprintf) {
+ if (x > (unsigned long) &get_options && x < (unsigned long) &vsprintf) {
printk("<[%08lx]> ", x);
}
}
@@ -706,9 +707,17 @@
unsigned long flags;
spin_lock_irqsave(&irq_controller_lock, flags);
- if (!--irq_desc[irq].depth) {
+ switch (irq_desc[irq].depth) {
+ case 1:
irq_desc[irq].status &= ~IRQ_DISABLED;
irq_desc[irq].handler->enable(irq);
+ /* fall throught */
+ default:
+ irq_desc[irq].depth--;
+ break;
+ case 0:
+ printk("enable_irq() unbalanced from %p\n",
+ __builtin_return_address(0));
}
spin_unlock_irqrestore(&irq_controller_lock, flags);
}
This patch remove the unused bits. I don' t know what they are used for...
Maybe for alignment, but without them the irq_desc_t struct is just 32*4
bits so it seems just word aligned to me...
Index: linux/arch/i386/kernel/irq.h
diff -u linux/arch/i386/kernel/irq.h:1.1.1.2 linux/arch/i386/kernel/irq.h:1.1.1.1.2.2
--- linux/arch/i386/kernel/irq.h:1.1.1.2 Fri Dec 18 10:16:20 1998
+++ linux/arch/i386/kernel/irq.h Fri Dec 18 15:30:18 1998
@@ -38,7 +38,6 @@
struct hw_interrupt_type *handler; /* handle/enable/disable functions */
struct irqaction *action; /* IRQ action list */
unsigned int depth; /* Disable depth for nested irq disables */
- unsigned int unused[2];
} irq_desc_t;
#define IRQ0_TRAP_VECTOR 0x51
This patch do the s/disable_irq/cli/ in my gettimeofday patch.
Index: linux/arch/i386/kernel/time.c
diff -u linux/arch/i386/kernel/time.c:1.1.1.1 linux/arch/i386/kernel/time.c:1.1.1.1.2.10
--- linux/arch/i386/kernel/time.c:1.1.1.1 Fri Nov 20 00:03:43 1998
+++ linux/arch/i386/kernel/time.c Fri Dec 18 15:30:18 1998
@@ -20,6 +20,10 @@
* (C. Scott Ananian <cananian@alumni.princeton.edu>, Andrew D.
* Balsa <andrebalsa@altern.org>, Philip Gladstone <philip@raptor.com>;
* ported from 2.0.35 Jumbo-9 by Michael Krause <m.krause@tu-harburg.de>).
+ * 1998-12-16 Andrea Arcangeli
+ * Fixed Jumbo-9 code in 2.1.131: do_gettimeofday was missing 1 jiffy
+ * because was not accounting lost_ticks. I also removed some ugly
+ * not needed global cli() and where needed I used a disable_irq(0).
*/
/* What about the "updated NTP code" stuff in 2.0 time.c? It's not in
@@ -92,9 +96,9 @@
eax -= last_tsc_low; /* tsc_low delta */
/*
- * Time offset = (tsc_low delta) * fast_gettimeoffset_quotient.
- * = (tsc_low delta) / (clocks_per_usec)
- * = (tsc_low delta) / (clocks_per_jiffy / usecs_per_jiffy)
+ * Time offset = (tsc_low delta) * fast_gettimeoffset_quotient
+ * = (tsc_low delta) * (usecs_per_clock)
+ * = (tsc_low delta) * (usecs_per_jiffy / clocks_per_jiffy)
*
* Using a mull instead of a divl saves up to 31 clock cycles
* in the critical path.
@@ -230,17 +234,19 @@
*/
void do_gettimeofday(struct timeval *tv)
{
+ extern volatile unsigned long lost_ticks;
unsigned long flags;
- save_flags(flags);
- cli();
+ save_flags(flags); cli();
*tv = xtime;
tv->tv_usec += do_gettimeoffset();
- if (tv->tv_usec >= 1000000) {
+ if (lost_ticks)
+ tv->tv_usec += lost_ticks * (1000000/HZ);
+ restore_flags(flags);
+ while (tv->tv_usec >= 1000000) {
tv->tv_usec -= 1000000;
tv->tv_sec++;
}
- restore_flags(flags);
}
void do_settimeofday(struct timeval *tv)
@@ -254,7 +260,7 @@
*/
tv->tv_usec -= do_gettimeoffset();
- if (tv->tv_usec < 0) {
+ while (tv->tv_usec < 0) {
tv->tv_usec += 1000000;
tv->tv_sec--;
}
@@ -399,18 +405,20 @@
*/
static void pentium_timer_interrupt(int irq, void *dev_id, struct pt_regs *regs)
{
- int count, flags;
+ int count;
/* It is important that these two operations happen almost at the
* same time. We do the RDTSC stuff first, since it's faster. To
- * avoid any inconsistencies, we disable interrupts locally.
+ * avoid any inconsistencies, we need interrupts disabled locally.
*/
+
+ /*
+ * Interrupts are just disabled locally since the timer irq has the
+ * SA_INTERRUPT flag set. -arca
+ */
- __save_flags(flags);
- __cli();
/* read Pentium cycle counter */
- __asm__("rdtsc"
- :"=a" (last_tsc_low):: "eax", "edx");
+ __asm__("rdtsc" : "=a" (last_tsc_low) : : "edx");
outb_p(0x00, 0x43); /* latch the count ASAP */
@@ -419,8 +427,7 @@
count = ((LATCH-1) - count) * TICK_SIZE;
delay_at_last_interrupt = (count + LATCH/2) / LATCH;
- __restore_flags(flags);
-
+
timer_interrupt(irq, NULL, regs);
}
Andrea Arcangeli
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/