Skip to contentSkip to conversation

Usage of mdelay() inside Interrupt

DISCUSSION Armin Wolf 3 participants 3 messages May 28, 2020 – Jun 3, 2020

Subsystems (2)
lore
Share & tools
lore

Armin Wolf

author
#0 lore raw

Hi,

while browsing the sourcefile of lib8390.c in drivers/net/ethernet/8390/, i noticed that inside of ei_rx_overrun(), which is called from inside a Interrupt handler, mdelay() is being used. So i wonder if the usage of mdelay() inside the Interrupt handler may cause problems since waiting ~10ms in Interrupt context seems a bit odd.

#1 lore raw
Quoted 9 lines · Armin Wolf

Hi,

while browsing the sourcefile of lib8390.c in drivers/net/ethernet/8390/, i noticed that inside of ei_rx_overrun(), which is called from inside a Interrupt handler, mdelay() is being used. So i wonder if the usage of mdelay() inside the Interrupt handler may cause problems since waiting ~10ms in Interrupt context seems a bit odd.

Hi Armin

It is legal. But it is not ideal. But reading the comments around it suggests the hardware is very particular about how you recover from an overrun, and maybe this is the most robust solution?

Overruns should not happen too often. If the statistics counter stats.rx_over_errors indicates it is happening a lot then maybe more investigation is needed, because it is going to be bad for performance.

Andrew

#2 lore raw
Quoted 7 lines · Andrew Lunn
Quoted 7 lines

while browsing the sourcefile of lib8390.c in drivers/net/ethernet/8390/, i noticed that inside of ei_rx_overrun(), which is called from inside a Interrupt handler, mdelay() is being used. So i wonder if the usage of mdelay() inside the Interrupt handler may cause problems since waiting ~10ms in Interrupt context seems a bit odd.

[...]

Quoted 3 lines

It is legal. But it is not ideal. But reading the comments around it suggests the hardware is very particular about how you recover from an overrun, and maybe this is the most robust solution?

This code is very old and dates back to 1996. At that time it was
busy-looping inline even:

    /*
     * Wait a full Tx time (1.2ms) + some guard time, NS says 1.6ms total.
     * Early datasheets said to poll the reset bit, but now they say that
     * it "is not a reliable indicator and subsequently should be ignored."
     * We wait at least 10ms.
     */
    wait_start_time = jiffies;
    while (jiffies - wait_start_time <= 1*HZ/100)
	barrier();

Previously it had:

    /* Wait for the reset to complete. This should happen almost instantly,
	   but could take up to 1.5msec in certain rare instances.  There is no
	   easy way of timing something in that range, so we use 'jiffies' as
	   a sanity check. */
    while ((inb_p(e8390_base+EN0_ISR) & ENISR_RESET) == 0)
		if (jiffies - reset_start_time > 1) {
			printk("%s: reset did not complete at ei_rx_overrun.\n",
				   dev->name);
			NS8390_init(dev, 1);
			return;
		}

going back to 1.1.x days. I don't have older history to hand. Nowadays I think it would make sense to rewrite it to use a timer or suchlike so that the post-reset part runs asynchronously, but such hardware is not exactly common anymore I'd imagine (I should have an ISA NE2000 clone somewhere IIRC, which I haven't used in like 20 years) and clearly nobody bothered.

Maciej