netdev
[Top] [All Lists]

Re: [PATCH] Updated 8139too with NAPI

To: Jeff Garzik <jgarzik@xxxxxxxxx>
Subject: Re: [PATCH] Updated 8139too with NAPI
From: OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
Date: Fri, 31 Oct 2003 02:12:41 +0900
Cc: Stephen Hemminger <shemminger@xxxxxxxx>, netdev@xxxxxxxxxxx
In-reply-to: <3FA01629.2080202@pobox.com>
References: <3F9070B6.9090306@pobox.com> <873cdqbt6z.fsf@devron.myhome.or.jp> <20031020131106.6862e951.shemminger@osdl.org> <877k2ysohc.fsf@devron.myhome.or.jp> <20031028114707.1d234da6.shemminger@osdl.org> <3FA01629.2080202@pobox.com>
Sender: netdev-bounce@xxxxxxxxxxx
User-agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.3
Jeff Garzik <jgarzik@xxxxxxxxx> writes:

> I'd like to apply this to net-drivers-2.[45]-exp, but it doesn't apply
> cleanly.  Can you wait a bit, for my posting of those queues, and then
> resend?

Probably more tester/reviewer is always welcome, so I think it's good.


Stephen Hemminger <shemminger@xxxxxxxx> writes:

> +     /* Receive packets are processed by poll routine.
> +        If not running start it now. */
> +     if (status & RxAckBits){
> +             if (netif_rx_schedule_prep(dev)) {
> +                     RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
> +                     __netif_rx_schedule (dev);
>               }

I think that this style is common issues of NIC drivers for "NAPI
support and intr-mask" handling. Looks like __netif_poll_disable()
also has the following problem.

For example,

        in ISR
                [...]
                if (status & RxAckBits){
                        if (netif_rx_schedule_prep(dev)) {
                                RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
                                __netif_rx_schedule (dev);
                        }
                }
                [...]

        in dev_close()
                [...]
                while (test_bit(__LINK_STATE_RX_SCHED, &dev->state)) {
                        /* No hurry. */
                        current->state = TASK_INTERRUPTIBLE;
                        schedule_timeout(1);
                }
                  --------- (*)
                if (dev->stop)
                        dev->stop(dev);
                [...]

If Rx interrupt happen on (*) point, ISR can't ack or disable.
Then it will start busy Rx interrupt loop. So if this happen, it's
seriously problem on UP system.
-- 
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>