netdev - Re: [PATCH RFC] gianfar: Do not call skb recycling with disabled IRQs

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

Message-ID: <20091105175316.GA27099@oksana.dev.rtsoft.ru>
Date:	Thu, 5 Nov 2009 20:53:16 +0300
From:	Anton Vorontsov <avorontsov@...mvista.com>
To:	Jon Loeliger <jdl@....com>
Cc:	Kumar Gopalpet-B05799 <B05799@...escale.com>,
	linuxppc-dev@...abs.org, Jason Wessel <jason.wessel@...driver.com>,
	Fleming Andy-AFLEMING <afleming@...escale.com>,
	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Lennert Buytenhek <buytenh@...tstofly.org>,
	Stephen Hemminger <shemminger@...tta.com>
Subject: Re: [PATCH RFC] gianfar: Do not call skb recycling with disabled
 IRQs

On Thu, Nov 05, 2009 at 11:40:21AM -0600, Jon Loeliger wrote:
[...]
> > >+			netif_tx_lock_bh(priv->ndev);
> > 
> > Will this not lead to locking all the tx queues even though at this
> > point we are working on a "particular queue" ?
> 
> Similar but different question:  What version is this patch based upon?
> I can't find:
> 
>     > >index 197b358..a0ae604 100644
> 
> My search of 2.6.31 up through current linux head (v2.6.32-rc6-26-g91d3f9b)
> and benh's current head (94a8d5caba74211ec76dac80fc6e2d5c391530df) do not
> have a version of this code with separate txqueues.
> 
> I confess I'm not too familiar with the history here, so I don't know
> if that feature is in flux, or came, or went, or what.

Sorry for not mentioning, it's based on

git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next-2.6.git
+ compiler/sparse warnings fixes that I sent yesterday.

> That boils down to:  I can't directly apply your patch, but I could
> hand-fudge its intent, but only on a single tx queue variant.

Here is the patch on top of the Linus' git tree, if you haven't
already 'back-ported' the previous patch.

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 5bf31f1..5dca99c 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1274,7 +1274,6 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	u32 lstatus;
 	int i;
 	u32 bufaddr;
-	unsigned long flags;
 	unsigned int nr_frags, length;
 
 	base = priv->tx_bd_base;
@@ -1298,14 +1297,11 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* total number of fragments in the SKB */
 	nr_frags = skb_shinfo(skb)->nr_frags;
 
-	spin_lock_irqsave(&priv->txlock, flags);
-
 	/* check if there is space to queue this packet */
 	if ((nr_frags+1) > priv->num_txbdfree) {
 		/* no space, stop the queue */
 		netif_stop_queue(dev);
 		dev->stats.tx_fifo_errors++;
-		spin_unlock_irqrestore(&priv->txlock, flags);
 		return NETDEV_TX_BUSY;
 	}
 
@@ -1403,9 +1399,6 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Tell the DMA to go go go */
 	gfar_write(&priv->regs->tstat, TSTAT_CLEAR_THALT);
 
-	/* Unlock priv */
-	spin_unlock_irqrestore(&priv->txlock, flags);
-
 	return NETDEV_TX_OK;
 }
 
@@ -1915,17 +1908,14 @@ static int gfar_poll(struct napi_struct *napi, int budget)
 	struct net_device *dev = priv->ndev;
 	int tx_cleaned = 0;
 	int rx_cleaned = 0;
-	unsigned long flags;
 
 	/* Clear IEVENT, so interrupts aren't called again
 	 * because of the packets that have already arrived */
 	gfar_write(&priv->regs->ievent, IEVENT_RTX_MASK);
 
-	/* If we fail to get the lock, don't bother with the TX BDs */
-	if (spin_trylock_irqsave(&priv->txlock, flags)) {
-		tx_cleaned = gfar_clean_tx_ring(dev);
-		spin_unlock_irqrestore(&priv->txlock, flags);
-	}
+	netif_tx_lock_bh(priv->ndev);
+	tx_cleaned = gfar_clean_tx_ring(dev);
+	netif_tx_unlock_bh(priv->ndev);
 
 	rx_cleaned = gfar_clean_rx_ring(dev, budget);
 

-- 
Anton Vorontsov
email: cbouatmailru@...il.com
irc://irc.freenode.net/bd2
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html