WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] Re: [PATCH] IRQ handling race and spurious IIR read in

To: Alan Cox <alan@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] Re: [PATCH] IRQ handling race and spurious IIR read in serial/8250.c
From: Robert Evans <evans.robert.n@xxxxxxxxx>
Date: Thu, 18 Jun 2009 21:39:20 -0400
Cc: Jeremy Fitzhardinge <jeremy@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Markus Armbruster <armbru@xxxxxxxxxx>, Anders Kaseorg <andersk@xxxxxxx>, "linux-serial@xxxxxxxxxxxxxxx" <linux-serial@xxxxxxxxxxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 19 Jun 2009 08:29:58 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=vEKhiVrH7XNrjCPEiDn2jaUmgVGA348E3JvvQ4txeug=; b=IOvTi7neN+Zg/DutI6WnK42zCo9l4URPByUuZ8K/OHTuKt2temTfMoXhC2hJt7COgH vqacWRuBARQLzdMo2Iv4LMOlIAXZKvvtxCX85x3zGHbYO04VeQU/kfSLShN04bmriAIS NMKOb+uNv5FGE604wJlAtXTwZR8krlbyKrh8A=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=pC7Q2itVlFoPG3xGFXlUogFBavni0vAnsRcnVt1AtJiqxXP9qxdbFf1/udIPA+FRSu ML2faK5X3X71/qZNbwzyfad8NA1eR7DfJzJ44AO072BVsMRHzzQJtbm4izPXJ4kNfi39 q/904mw0msKsFH++GXAXpm6Le8Di4Vi6VocY4=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20090312213831.5045f5d0@xxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <874oxyei9k.fsf@xxxxxxxxxxxxxxxxx> <20090312180955.1f52a401@xxxxxxxxxxxxxxxxxxx> <18873.25299.416627.13221@xxxxxxxxxxxxxxxxxxxxxxxx> <20090312213831.5045f5d0@xxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On Thu, Mar 12, 2009 at 5:38 PM, Alan Cox<alan@xxxxxxxxxxxxxxxxxxx> wrote:
...
>>  4. Any fix which does not involve completely removing UART_BUG_TXEN
>>     will need my change.
>
> Or the locking fix
>
> It was described as a band aid so I treated it as such. Regardless of the
> right thing to do long term its not a 2.6.29 candidate at this point but
> certainly something that wants looking into in 2.6.30

Stratus has experienced this same problem on bare metal.  An analysis
and proposed fixes were posted to this list on 2008-08-13.  See
  http://marc.info/?l=linux-serial&m=121863945506976&w=2

The first proposed fix takes the port's irq lock when starting up the
UART to provide mutual exclusion between the "quick test" in the
startup code and the interrupt service routine.  Stratus has quite a
bit or runtime with this locking fix on kernels of the 2.6.18 vintage
with good success.

Is this a "right thing" fix that could be considered for inclusion upstream?

--- linux-2.6.18-92-old/drivers/serial/8250.c   2008-08-13
14:07:08.000000000 +0000
+++ linux-2.6.18-92-new/drivers/serial/8250.c   2008-08-13
14:04:12.000000000 +0000
@@ -1749,65 +1749,73 @@

               timeout = timeout > 6 ? (timeout / 2 - 2) : 1;

               up->timer.data = (unsigned long)up;
               mod_timer(&up->timer, jiffies + timeout);
       } else {
               retval = serial_link_irq_chain(up);
               if (retval)
                       return retval;
       }

       /*
        * Now, initialize the UART
        */
       serial_outp(up, UART_LCR, UART_LCR_WLEN8);

-       spin_lock_irqsave(&up->port.lock, flags);
+       if (is_real_interrupt(up->port.irq)) {
+               spin_lock_irqsave(&irq_lists[up->port.irq].lock, flags);
+               spin_lock(&up->port.lock);
+       } else
+               spin_lock_irqsave(&up->port.lock, flags);
       if (up->port.flags & UPF_FOURPORT) {
               if (!is_real_interrupt(up->port.irq))
                       up->port.mctrl |= TIOCM_OUT1;
       } else
               /*
                * Most PC uarts need OUT2 raised to enable interrupts.
                */
               if (is_real_interrupt(up->port.irq))
                       up->port.mctrl |= TIOCM_OUT2;

       serial8250_set_mctrl(&up->port, up->port.mctrl);

       /*
        * Do a quick test to see if we receive an
        * interrupt when we enable the TX irq.
        */
       serial_outp(up, UART_IER, UART_IER_THRI);
       lsr = serial_in(up, UART_LSR);
       iir = serial_in(up, UART_IIR);
       serial_outp(up, UART_IER, 0);

       if (lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT) {
               if (!(up->bugs & UART_BUG_TXEN)) {
                       up->bugs |= UART_BUG_TXEN;
                       pr_debug("ttyS%d - enabling bad tx status workarounds\n",
                                port->line);
               }
       } else {
               up->bugs &= ~UART_BUG_TXEN;
       }

-       spin_unlock_irqrestore(&up->port.lock, flags);
+       if (is_real_interrupt(up->port.irq)) {
+               spin_unlock(&up->port.lock);
+               spin_unlock_irqrestore(&irq_lists[up->port.irq].lock, flags);
+       } else
+               spin_unlock_irqrestore(&up->port.lock, flags);

       /*
        * Finally, enable interrupts.  Note: Modem status interrupts
        * are set via set_termios(), which will be occurring imminently
        * anyway, so we don't enable them here.
        */
       up->ier = UART_IER_RLSI | UART_IER_RDI;
       serial_outp(up, UART_IER, up->ier);

       if (up->port.flags & UPF_FOURPORT) {
               unsigned int icp;
               /*
                * Enable interrupts on the AST Fourport board
                */
               icp = (up->port.iobase & 0xfe0) | 0x01f;
               outb_p(0x80, icp);


Robert N. Evans
Software Engineer
STRATUS TECHNOLOGIES
111 Powdermill Road,
Maynard, MA 01754-3409  U.S.A.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>
  • Re: [Xen-devel] Re: [PATCH] IRQ handling race and spurious IIR read in serial/8250.c, Robert Evans <=