netdev - Re: Usage of mdelay() inside Interrupt

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] [day] [month] [year] [list]

Message-ID: <alpine.LFD.2.21.2006032358160.3052124@eddie.linux-mips.org>
Date:   Thu, 4 Jun 2020 00:29:35 +0100 (BST)
From:   "Maciej W. Rozycki" <macro@....com>
To:     Andrew Lunn <andrew@...n.ch>
cc:     Armin Wolf <W_Armin@....de>, netdev@...r.kernel.org,
        "Maciej W. Rozycki" <macro@...ux-mips.org>
Subject: Re: Usage of mdelay() inside Interrupt

On Fri, 29 May 2020, Andrew Lunn wrote:

> > 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.
[...]
> 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