lkml.org 
[lkml]   [2008]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2] serial: 8250: fix shared interrupts issues with SMP and RT kernels
    On Tue, Jul 01, 2008 at 07:02:54PM +0400, Anton Vorontsov wrote:
    [...]
    > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
    > ---
    >
    > On Tue, Jul 01, 2008 at 02:43:53PM +0100, Alan Cox wrote:
    [...]
    > > It looks correct to me except that you cannot use spin_lock/disable_irq
    > > in that way safely. You must always disable_irq before taking the lock,
    > > or prove it is safe and use disable_irq_nosync
    > >
    > > The reason:
    > > CPU#0 spin_lock_... [taken]
    > > CPU#1 IRQ
    > > CPU#1 spin_lock [waits]
    > > CPU#0 disable_irq (deadlock)
    >
    > This deadlock possibility is interesting by itself, thanks for
    > mentioning it.
    >
    > But this can't happen here. IRQ will not grab the up->port.lock,
    > because port isn't registered in the 8250 IRQ handling chain (yet).

    Alan, are there any issues with the proof or the patch itself?

    Thanks,

    > As for _nosync, probably this is good idea indeed, and should be safe
    > AFAICS.
    >
    > drivers/serial/8250.c | 8 ++++++--
    > 1 files changed, 6 insertions(+), 2 deletions(-)
    >
    > diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
    > index 76ccef7..cad0c2d 100644
    > --- a/drivers/serial/8250.c
    > +++ b/drivers/serial/8250.c
    > @@ -1831,7 +1831,9 @@ static int serial8250_startup(struct uart_port *port)
    > * the interrupt is enabled. Delays are necessary to
    > * allow register changes to become visible.
    > */
    > - spin_lock_irqsave(&up->port.lock, flags);
    > + spin_lock(&up->port.lock);
    > + if (up->port.flags & UPF_SHARE_IRQ)
    > + disable_irq_nosync(up->port.irq);
    >
    > wait_for_xmitr(up, UART_LSR_THRE);
    > serial_out_sync(up, UART_IER, UART_IER_THRI);
    > @@ -1843,7 +1845,9 @@ static int serial8250_startup(struct uart_port *port)
    > iir = serial_in(up, UART_IIR);
    > serial_out(up, UART_IER, 0);
    >
    > - spin_unlock_irqrestore(&up->port.lock, flags);
    > + if (up->port.flags & UPF_SHARE_IRQ)
    > + enable_irq(up->port.irq);
    > + spin_unlock(&up->port.lock);
    >
    > /*
    > * If the interrupt is not reasserted, setup a timer to
    > --
    > 1.5.5.4
    >

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2


    \
     
     \ /
      Last update: 2008-07-09 21:33    [W:4.561 / U:0.372 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site