netdev
[Top] [All Lists]

Re: [openib-general] Re: LLTX and netif_stop_queue

To: Stephen Hemminger <shemminger@xxxxxxxx>
Subject: Re: [openib-general] Re: LLTX and netif_stop_queue
From: "David S. Miller" <davem@xxxxxxxxxxxxx>
Date: Wed, 19 Jan 2005 15:41:32 -0800
Cc: hadi@xxxxxxxxxx, iod00d@xxxxxx, eric.lemoine@xxxxxxxxx, roland@xxxxxxxxxxx, netdev@xxxxxxxxxxx, ak@xxxxxxx, openib-general@xxxxxxxxxx, kaber@xxxxxxxxx
In-reply-to: <20050119151853.259de49a@xxxxxxxxxxxxxxxxx>
References: <5cac192f0412230110628749e3@xxxxxxxxxxxxxx> <41CAF444.3000305@xxxxxxxxx> <5cac192f04122408102129af43@xxxxxxxxxxxxxx> <1104240717.1100.66.camel@xxxxxxxxxxxxxxxx> <5cac192f0501021530672a908a@xxxxxxxxxxxxxx> <1104764660.1048.578.camel@xxxxxxxxxxxxxxxx> <52brc68q05.fsf@xxxxxxxxxxx> <5cac192f05010308414a25b548@xxxxxxxxxxxxxx> <527jmu8nbw.fsf@xxxxxxxxxxx> <5cac192f0501030907c755135@xxxxxxxxxxxxxx> <20050103171227.GD7370@xxxxxxxxxxxxxxxxx> <1104812294.1085.53.camel@xxxxxxxxxxxxxxxx> <20050119144711.3fdd3d93.davem@xxxxxxxxxxxxx> <20050119151853.259de49a@xxxxxxxxxxxxxxxxx>
Sender: netdev-bounce@xxxxxxxxxxx
On Wed, 19 Jan 2005 15:18:53 -0800
Stephen Hemminger <shemminger@xxxxxxxx> wrote:

> Wondering, why not just have the drivers have a way to lock dev->queue_lock
> in the interrupt handler, and change the xmit to do spin_lock_irqsave?
> 
> Any driver that assumes it is being called with irq's enabled in transmit
> is probably already busted anyway.

Yes, that's an idea.

Effectively, LLTX is working around the fact that dev->xmit_lock is
BH disabling instead of IRQ disabling.  And there is no fundamental
reason for that.

Usually we strive for BH disabling locks in order to decrease the
amount of hard IRQ disabling time in the kernel.  But here, as soon
as we call into ->hard_start_xmit(), the driver typically turns hard
IRQs off anyways.

There are other things using this though, such as multicast list
upload.

If we change dev->xmit_lock to be an IRQ disabling lock, then drivers
can use it in place of their private tx_lock.

We would still need something special for loopback, which wants and
needs no locking at all.

Also, changing dev->xmit_lock to be IRQ disabling is not %100 trivial.
We'd need to verify the side-effects this has on gems like the
spin_unlock_wait(&dev->xmit_lock) in the Tulip winbond power management
code.  There is another fun case in the 6pack hamradio driver where it
is doing IRQ disabling when taking dev->xmit_lock.

Originally, dev->xmit_lock was added so that drivers that were SMP dumb
could stay that way.  Thus preserving the guarentee that there would be
only one active call into the dev->hard_start_xmit method across the
entire system.  I don't think any of that is relevant any longer.  All
of our network drivers are pretty clean in this regard.

<Prev in Thread] Current Thread [Next in Thread>