lkml.org 
[lkml]   [2000]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: PATCH 2.4.0.10.3: pc_keyb and q40_keyb cleanup
    On Tue, Oct 17, 2000 at 05:26:23AM -0400, Jeff Garzik wrote:
    > Well, the -spin lock- exists for serialization. My question is.... Why
    > does pc_keyb irq handler disable local irqs for this case? What is the
    > race/deadlock that exists with spin_lock in the irq handler, but does
    > not exist with spin_lock_irqsave in the irq handler?

    As said the save part isn't necessary.

    This is a trace of the deadlock:

    irq 2 runs
    keyboard_interrupt
    irqs are been left enabled
    spin_lock()
    here irq 12 runs
    keyboard_interrupt
    here doesn't matter if irqs are enabled or disabled of course
    spin_lock() -> dealdock

    Really if you really want to, you could remove the local irq disable by
    replacing it with a disable_irq_nosync(other irq). Something like this is safe
    and obviously right. But I don't think the keyboard_interrupt is long enough to
    justify playing with the irq controller [that in IA32 UP isn't memory mapped]
    instead of using the fast in CPU core local cli, also the cli version allows
    the other irq to be started in parallel to us in another CPU.

    keyboard_interrupt(irq, ...) {
    int other_irq;

    switch(irq) {
    case KEYBOARD_IRQ:
    other_irq = AUX_IRQ;
    break;
    case AUX_IRQ:
    other_irq = KEYBOARD_IRQ;
    break;
    default:
    panic("wrong irq");
    }

    disable_irq_nosync(other_irq);
    spin_lock();
    -- critical section --
    spin_unlock();
    enable_irq(other_irq);
    }

    BTW, after reading the code some more it seems you could remove the irq disable
    also if CONFIG_PSMOUSE is not defined. So you can probably also do:

    keyboard_interrupt(irq, ...) {
    #ifdef CONFIG_PSMOUSE
    int other_irq;

    switch(irq) {
    case KEYBOARD_IRQ:
    other_irq = AUX_IRQ;
    break;
    case AUX_IRQ:
    other_irq = KEYBOARD_IRQ;
    break;
    default:
    panic("wrong irq");
    }

    disable_irq_nosync(other_irq);
    #endif
    spin_lock();
    -- critical section --
    spin_unlock();
    #ifdef CONFIG_PSMOUSE
    enable_irq(other_irq);
    #endif
    }

    Or the __cli version:

    keyboard_interrupt(irq, ...) {
    #ifdef CONFIG_PSMOUSE
    __cli();
    #endif
    spin_lock();
    -- critical section --
    spin_unlock();
    #ifdef CONFIG_PSMOUSE
    __sti(); /* this wouldn't be necessary if 2.4.x would
    honour SA_INTERRUPT for the shared irqs
    as 2.2.x */
    #endif
    }

    Andrea
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 12:41    [W:0.025 / U:0.976 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site