Skip to contentSkip to conversation

LLTX for tg3

PATCH In review Andi Kleen 3 participants 3 messages Sep 7, 2004 – Sep 12, 2004

Subsystems (2)
lore
Share & tools
lore

Andi Kleen

author
#0 lore raw

PATCH 1/1 LLTX for tg3

Add LLTX suppor to tg3. Locking was already safe for it, so only trivial changes.

Depends on the LLTX patch sent earlier.

drivers/net/tg3.c renamed from drivers/net/tg3.c-o +6 -1
diff -u linux-2.6.8/drivers/net/tg3.c-o linux-2.6.8/drivers/net/tg3.c
@@ -3036,7 +3036,11 @@
 	 * So we really do need to disable interrupts when taking
 	 * tx_lock here.
 	 */
-	spin_lock_irqsave(&tp->tx_lock, flags);
+	local_irq_save(flags);
+	if (!spin_trylock(&tp->tx_lock)) { 
+		local_irq_restore(flags);
+		return -1; 
+	} 
 
 	/* This is a hard error, log it. */
 	if (unlikely(TX_BUFFS_AVAIL(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
@@ -8255,6 +8259,7 @@
 
 	if (pci_using_dac)
 		dev->features |= NETIF_F_HIGHDMA;
+	dev->features |= NETIF_F_LLTX;
 #if TG3_VLAN_TAG_USED
 	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
 	dev->vlan_rx_register = tg3_vlan_rx_register;
#1 lore raw
Quoted 2 lines

Add LLTX suppor to tg3. Locking was already safe for it, so only trivial changes.

tg3_set_rx_mode() (dev->set_multicast_list()) and tg3_start_xmit() used to synchronise thanks to dev->lock_xmit. With your LLTX patches they don't synchronise anymore (I don't think tg3_set_rx_mode() grabs tp->tx_lock) ; isn't it an issue?

PS: your LLTX patch to e1000 seems to have the needed extra locking in e1000_set_multi() though.

Quoted 28 lines

Depends on the LLTX patch sent earlier.

diff -u linux-2.6.8/drivers/net/tg3.c-o linux-2.6.8/drivers/net/tg3.c

drivers/net/tg3.c 2004-09-07 08:17:36.000000000 +0000 renamed from drivers/net/tg3.c-o 2004-09-04 13:10:46.000000000 +0000 +6 -1
@@ -3036,7 +3036,11 @@
         * So we really do need to disable interrupts when taking
         * tx_lock here.
         */
-       spin_lock_irqsave(&tp->tx_lock, flags);
+       local_irq_save(flags);
+       if (!spin_trylock(&tp->tx_lock)) {
+               local_irq_restore(flags);
+               return -1;
+       }
 
        /* This is a hard error, log it. */
        if (unlikely(TX_BUFFS_AVAIL(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
@@ -8255,6 +8259,7 @@
 
        if (pci_using_dac)
                dev->features |= NETIF_F_HIGHDMA;
+       dev->features |= NETIF_F_LLTX;
 #if TG3_VLAN_TAG_USED
        dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
        dev->vlan_rx_register = tg3_vlan_rx_register;
Signature · 1 line
Eric
maintainer
#2 lore raw
Quoted 7 lines · On Sun, 12 Sep 2004 19:06:26 +0200 Eric Lemoine <[email protected]> wrote:
Quoted 2 lines

Add LLTX suppor to tg3. Locking was already safe for it, so only trivial changes.

tg3_set_rx_mode() (dev->set_multicast_list()) and tg3_start_xmit() used to synchronise thanks to dev->lock_xmit. With your LLTX patches they don't synchronise anymore (I don't think tg3_set_rx_mode() grabs tp->tx_lock) ; isn't it an issue?

You're absolutely right, I've made tg3_set_rx_mode() grab the tx_lock now too.

Good spotting Eric.