Re: eepro100.c

From: Andrew Morton (andrewm@uow.edu.au)
Date: Mon Mar 27 2000 - 00:17:14 EST


Andrey Savochkin wrote:
>
> Hello,
>
> On Sat, Mar 25, 2000 at 03:05:50PM +0000, Andrew Morton wrote:
> > Is wait_for_cmd_done() racy? It is called from hard_start_xmit()
> > outside the spinlock as well as from get_stats(). set_multicast_list()
> > also.
>
> Here is an excerpt from speedo_get_stats()
> 1880 if (dev->start) {
> 1881 unsigned long flags;
> 1882 /* Take a spinlock to make wait_for_cmd_done and sending the
> 1883 * command atomic. --SAW */
> 1884 spin_lock_irqsave(&sp->lock, flags);
> 1885 wait_for_cmd_done(ioaddr + SCBCmd);
> 1886 outb(CUDumpStats, ioaddr + SCBCmd);
> 1887 spin_unlock_irqrestore(&sp->lock, flags);
> 1888 }
>
> Which driver version are you looking at? :-)

Ah. 2.3.99pre3 needs this.

> > mdio_read() is called from various unprotected contexts, one of which is
> > a bh. Confident about this?
>
> I know about mdio_read() problem.
> mdio_read() is used only in
> - initialization code (no special care is needed)
> - timer routine
> - TX timeout routine (these two are exclusive against each other)
> - ioctl
> The easiest solution is to make reading ioctl being privileged too.

What do you mean by "privileged"? The global cli would protect the
ioctl - otherwise it would need to share a lock with the timer and tx
timeout?

> ...
>
> > tx_timeout() appears to be unprotected from the ISR.
>
> Any specific example?

In the 2.3 driver tx_timeout() does serious stuff with both the tx and
rx hardware. I'm wondering if there's some way in which
speedo_interrupt() can be invoked during this. An rx interrupt would do
it. A spinlock_irqsave() in tx_timeout() would prevent this.

-- 
-akpm-

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Mar 31 2000 - 21:00:18 EST