Re: [PATCH] Improvev netconsole support for RTL8139 NIC driver

[yshi]

Hi Jeff,

Thanks for the comment. According your suggestion, I added 
local_irq_save()/local_irq_restore() in rtl8139_netpoll_controller().
Move the burden to netpoll layer. The following is the modified patch:

Replaced disable_irq()/enable_irq() with
local_irq_save()/local_irq_restore() to improve
netconsole/netpoll support on RTL8139 NIC driver.

Signed-off-by: Yang Shi <yang.shi@xxxxxxxxxxxxx>
---
b/drivers/net/8139too.c |    6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
---

--- a/drivers/net/8139too.c
+++ b/drivers/net/8139too.c
@@ -2199,9 +2199,11 @@ static irqreturn_t rtl8139_interrupt (in
 */
static void rtl8139_poll_controller(struct net_device *dev)
{
-       disable_irq(dev->irq);
+       unsigned long flags;
+
+       local_irq_save(flags);
       rtl8139_interrupt(dev->irq, dev);
-       enable_irq(dev->irq);
+       local_irq_restore(flags);
}
#endif

Thanks.

Yang
Jeff Garzik åé:
> yshi wrote:
> > In current RTL8139 NIC driver, spin_lock()/spin_unlock() is used
> > for irq handler. But for netconsole/netpoll, it prefers
> > spin_lock_irqsave()/spin_unlcok_irqrestore(). So this patch fixed
> > this problem to improve netconsole/netpoll support.
> >
> > Signed-off-by: Yang Shi <yang.shi@xxxxxxxxxxxxx>
> > ---
> > b/drivers/net/8139too.c |    9 +++++++++
> > 1 file changed, 9 insertions(+)
> > ---
> >
> > --- a/drivers/net/8139too.c
> > +++ b/drivers/net/8139too.c
> > @@ -2136,8 +2136,13 @@ static irqreturn_t rtl8139_interrupt (in
> >        u16 status, ackstat;
> >        int link_changed = 0; /* avoid bogus "uninit" warning */
> >        int handled = 0;
> > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > +       unsigned long flags;
> >
> > +       spin_lock_irqsave (&tp->lock, flags);
> > +#else
> >        spin_lock (&tp->lock);
> > +#endif
> >        status = RTL_R16 (IntrStatus);
> >
> >        /* shared irq? */
> > @@ -2185,7 +2190,11 @@ static irqreturn_t rtl8139_interrupt (in
> >                        RTL_W16 (IntrStatus, TxErr);
> >        }
> >  out:
> > +#ifdef CONFIG_NET_POLL_CONTROLLER
> > +       spin_unlock_irqrestore (&tp->lock, flags);
> > +#else
> >        spin_unlock (&tp->lock);
> > +#endif
>
> This is bogus -- you should never need to slow down the hot path in 
> such a way.
>
> Add a local_irq_save()/restore() to rtl8139_poll_controller() or a 
> similar solution, putting the burden on the netpoll/netconsole layer.
>
>     Jeff

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