Jeff Garzik <jgarzik@xxxxxxxxx> writes:
> > 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...
>
> Most interrupt routines already test this, look at
>
> static inline int netif_rx_schedule_prep(struct net_device *dev)
> {
> return netif_running(dev) &&
> !test_and_set_bit(__LINK_STATE_RX_SCHED, &dev->state);
> }
>
> It shouldn't schedule unless the interface is running.
Yes.
> However... I believe it was you that added this check to 8139cp.c:
>
> /* close possible race's with dev_close */
> if (unlikely(!netif_running(dev))) {
> cpw16(IntrMask, 0);
> goto out;
> }
Yes, I added. And my suggestion was about this.
Because in case of 8139too, I got too many interrupts about pending RX
during the following, and the following wasn't finished.
(dev->close() wasn't called).
dev_close(),
clear_bit(__LINK_STATE_START, &dev->state);
smp_mb__after_clear_bit(); /* Commit netif_running(). */
while (test_bit(__LINK_STATE_RX_SCHED, &dev->state)) {
/* No hurry. */
current->state = TASK_INTERRUPTIBLE;
schedule_timeout(1);
}
> I do wonder about the consequences, on some hardware, about receiving
> an interrupt and -not- processing the RX or TX completions associated
> with that. For most NIC hardware, you'll get sane behavior, but not
> all, I bet...
Is this meaning should _not_ receive the interrupt about pending RX/TX?
Thanks.
--
OGAWA Hirofumi <hirofumi@xxxxxxxxxxxxxxxxxx>
|