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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <47E9B72C.9030809@windriver.com>
Date:	Wed, 26 Mar 2008 10:38:36 +0800
From:	yshi <yang.shi@...driver.com>
To:	Jeff Garzik <jgarzik@...ox.com>
CC:	netdev@...r.kernel.org, linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Improvev netconsole support for RTL8139 NIC driver

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@...driver.com>
---
 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@...driver.com>
>> ---
>> 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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html