Jeff Garzik <jgarzik@xxxxxxxxx> writes:
> > + if (work_done < orig_budget) {
> > + unsigned long flags;
> > + int status;
> > +
> > + spin_lock_irqsave(&ep->napi_lock, flags);
> > + epic_napi_irq_on(dev, ep);
> > + __netif_rx_complete(dev);
> > + spin_unlock_irqrestore(&ep->napi_lock, flags);
> > +
> > + status = inl(ioaddr + INTSTAT);
> > + if (status & EpicNapiEvent) {
> > + epic_napi_irq_off(dev, ep);
> > + goto rx_action;
> > + }
>
> Need to add a netif_running() check to the 'if' test at the top of the
> quote.
>
> Are you (or somebody else?) interested in reviewing all the in-tree
> NAPI drivers, and seeing if other drivers have this bug? I think
> 8139cp.c does at least, maybe e100 too... Such a fix would need to go
> into 2.4.x as well.
Umm.. the above code is part of ->poll(). I think xxx_interrut() need
netif_running() instead. The driver must clear __LINK_STATE_RX_SCHED
flag...
BTW, ->napi_lock is unneeded because netif_schedule() is already
atomic, it need only local_irq_enable/disable().
After __netif_rx_complete() must not do "goto rx_action", otherwise it
may become cause of twice scheduleing, it should move before spin_lock().
Thanks.
--
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
|