On Sun, 2006-11-05 at 14:00 +0100, Eric Lemoine wrote: > Hi! > > Some (long) time ago benh wrote a blaming comment in sungem.c about > that driver's locking strategy. That comment basically says that we > probably don't need two spinlocks.
Yeah :) Note that I mostly blamed myself there ... Just never found the time to sit down a figure out a proper locking. > I agree! > > Proposal: > > Today's sungem effectively uses two spinlock's: "lock" and "tx_lock". > > "tx_lock" is held by the xmit function when sending out a packet. Lots > of functions grab "tx_lock" not to mess up with xmit (gem_stop_phy(), > gem_change_mtu(), etc.). > > All of these funcs also take "lock"! > > What we could do is remove "lx_lock", have the above functions take > only "lock", and rely on dev->_xmit_lock to protect the xmit func from > reentrance. In that case, obviously, the driver wouldn't feature LLTX > anymore. We could probably do even better but yeah, a single lock is a good start. Overall, I'm unhappy with the infrastructure provided by the network stack though. (Might be better nowadays, but last I looked, for example, I couldn't properly do things like stopping MAPI poll from set_multicast etc... due to locks held by the upper level). > When (re-)configuring we'd now quiesce the device, with the new > functions gem_netif_stop() and gem_full_lock(), in the same way as tg3 > does. > > gem_interrupt(), gem_poll(), and gem_start_xmit() could become lockless. Fast! > > Basically this proposal makes the data path faster, the control path > slower, and simplifies the code by using one single spinlock within > the driver. > > If the idea seems reasonable to you guys I can go ahead and cook up > something... I certainly does. Bring on the patch ! :-) Ben. - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
