[PATCH|[NET]: migrate HARD_TX_LOCK to header file

[PATCH|[NET]: migrate HARD_TX_LOCK to header file

[jamal]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

I wanted to get rid of the extrenous cpu arguement and ended moving this
to the header files since it looks common enough an operation that could
be used elsewhere.
It is a trivial change - i could resend with leaving it in dev.c and
just getting rid of the cpu arguement.


cheers,
jamal


[-- Attachment #2: htxp1 --]
[-- Type: text/plain, Size: 2050 bytes --]

[NET]: migrate HARD_TX_LOCK to header file

HARD_TX_LOCK micro is a nice aggregation that could be used
in other spots. move it to netdevice.h
Also get rid of superflous cpu arguement while doing this ..

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

---
commit 1bc3a7393737ab1f5239bd8dc2f2953dcee5391e
tree 83a7f39b61fe45282eee825286996ba4bf72c0f6
parent 1f08657fc9b0b56039a9378ca030c2c8ed7bd8ac
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 16 Sep 2007 11:29:48 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 16 Sep 2007 11:29:48 -0400

 include/linux/netdevice.h |   12 ++++++++++++
 net/core/dev.c            |   14 +-------------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dc5e35f..c83e667 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1297,6 +1297,18 @@ static inline void netif_tx_unlock_bh(struct net_device *dev)
 	spin_unlock_bh(&dev->_xmit_lock);
 }
 
+#define HARD_TX_LOCK(dev) {			\
+	if ((dev->features & NETIF_F_LLTX) == 0) {	\
+		netif_tx_lock(dev);			\
+	}						\
+}
+
+#define HARD_TX_UNLOCK(dev) {				\
+	if ((dev->features & NETIF_F_LLTX) == 0) {	\
+		netif_tx_unlock(dev);			\
+	}						\
+}
+
 static inline void netif_tx_disable(struct net_device *dev)
 {
 	netif_tx_lock_bh(dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 2897352..7934d28 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1574,18 +1574,6 @@ out_kfree_skb:
 	return 0;
 }
 
-#define HARD_TX_LOCK(dev, cpu) {			\
-	if ((dev->features & NETIF_F_LLTX) == 0) {	\
-		netif_tx_lock(dev);			\
-	}						\
-}
-
-#define HARD_TX_UNLOCK(dev) {				\
-	if ((dev->features & NETIF_F_LLTX) == 0) {	\
-		netif_tx_unlock(dev);			\
-	}						\
-}
-
 /**
  *	dev_queue_xmit - transmit a buffer
  *	@skb: buffer to transmit
@@ -1710,7 +1698,7 @@ gso:
 
 		if (dev->xmit_lock_owner != cpu) {
 
-			HARD_TX_LOCK(dev, cpu);
+			HARD_TX_LOCK(dev);
 
 			if (!netif_queue_stopped(dev) &&
 			    !netif_subqueue_stopped(dev, skb->queue_mapping)) {

Re: [PATCH|[NET]: migrate HARD_TX_LOCK to header file

[David Miller]

From: jamal <hadi@cyberus.ca>
Date: Sun, 16 Sep 2007 11:48:45 -0400

> I wanted to get rid of the extrenous cpu arguement and ended moving this
> to the header files since it looks common enough an operation that could
> be used elsewhere.
> It is a trivial change - i could resend with leaving it in dev.c and
> just getting rid of the cpu arguement.

The only reason the cpu argument is superfluous is because
we don't provide a way to pass it on down to netif_tx_lock().

So instead netif_tx_lock() recomputes that value in this case which is
extra unnecessary work.

I would instead suggest, in netdevice.h:

static inline void __netif_tx_lock(struct net_device *dev, int cpu)
{
	spin_lock(&dev->_xmit_lock);
	dev->xmit_lock_owner = cpu;
}

static inline void netif_tx_lock(struct net_device *dev)
{
	__netif_tx_lock(dev, smp_processor_id());
}

And make the HARD_TX_LOCK() call __netif_tx_lock() and pass in
the already computed 'cpu' parameter.

Thanks.

Re: [PATCH|[NET]: migrate HARD_TX_LOCK to header file

[jamal]

On Sun, 2007-16-09 at 12:28 -0700, David Miller wrote:

> The only reason the cpu argument is superfluous is because
> we don't provide a way to pass it on down to netif_tx_lock().
> 
> So instead netif_tx_lock() recomputes that value in this case which is
> extra unnecessary work.
> 
> I would instead suggest ..

sounds much better - will resend after a simple test.

cheers,
jamal

Re: [PATCH|[NET]: migrate HARD_TX_LOCK to header file

[jamal]

[-- Attachment #1: Type: text/plain, Size: 155 bytes --]

On Sun, 2007-16-09 at 16:28 -0400, jamal wrote:

> 
> sounds much better - will resend after a simple test.

Ok, heres the revised version

cheers,
jamal


[-- Attachment #2: htxp2 --]
[-- Type: text/plain, Size: 2433 bytes --]

[NET]: migrate HARD_TX_LOCK to header file
HARD_TX_LOCK micro is a nice aggregation that could be used
in other spots. move it to netdevice.h
Also makes sure the previously superflous cpu arguement is used.
Thanks to DaveM for the suggestions.

Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>

---
commit e467e3cb7fca9b533543aa749395547b7ade4980
tree 5e03a405e32968cc8e9e875ecdaeec4e798b6809
parent f55ad5bb4809bdd07720387c62788fad5359d41c
author Jamal Hadi Salim <hadi@cyberus.ca> Sun, 16 Sep 2007 16:54:44 -0400
committer Jamal Hadi Salim <hadi@cyberus.ca> Sun, 16 Sep 2007 16:54:44 -0400

 include/linux/netdevice.h |   21 +++++++++++++++++++--
 net/core/dev.c            |   12 ------------
 2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index dc5e35f..d529a0c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1265,10 +1265,15 @@ static inline void netif_rx_complete(struct net_device *dev,
  *
  * Get network device transmit lock
  */
-static inline void netif_tx_lock(struct net_device *dev)
+static inline void __netif_tx_lock(struct net_device *dev, int cpu)
 {
 	spin_lock(&dev->_xmit_lock);
-	dev->xmit_lock_owner = smp_processor_id();
+	dev->xmit_lock_owner = cpu;
+}
+
+static inline void netif_tx_lock(struct net_device *dev)
+{
+	__netif_tx_lock(dev, smp_processor_id());
 }
 
 static inline void netif_tx_lock_bh(struct net_device *dev)
@@ -1297,6 +1302,18 @@ static inline void netif_tx_unlock_bh(struct net_device *dev)
 	spin_unlock_bh(&dev->_xmit_lock);
 }
 
+#define HARD_TX_LOCK(dev, cpu) {			\
+	if ((dev->features & NETIF_F_LLTX) == 0) {	\
+		__netif_tx_lock(dev, cpu);			\
+	}						\
+}
+
+#define HARD_TX_UNLOCK(dev) {				\
+	if ((dev->features & NETIF_F_LLTX) == 0) {	\
+		netif_tx_unlock(dev);			\
+	}						\
+}
+
 static inline void netif_tx_disable(struct net_device *dev)
 {
 	netif_tx_lock_bh(dev);
diff --git a/net/core/dev.c b/net/core/dev.c
index 2897352..a1f6ca6 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1574,18 +1574,6 @@ out_kfree_skb:
 	return 0;
 }
 
-#define HARD_TX_LOCK(dev, cpu) {			\
-	if ((dev->features & NETIF_F_LLTX) == 0) {	\
-		netif_tx_lock(dev);			\
-	}						\
-}
-
-#define HARD_TX_UNLOCK(dev) {				\
-	if ((dev->features & NETIF_F_LLTX) == 0) {	\
-		netif_tx_unlock(dev);			\
-	}						\
-}
-
 /**
  *	dev_queue_xmit - transmit a buffer
  *	@skb: buffer to transmit

Re: [PATCH|[NET]: migrate HARD_TX_LOCK to header file

[David Miller]

From: jamal <hadi@cyberus.ca>
Date: Sun, 16 Sep 2007 16:57:14 -0400

> On Sun, 2007-16-09 at 16:28 -0400, jamal wrote:
> 
> > 
> > sounds much better - will resend after a simple test.
> 
> Ok, heres the revised version

Applied, thanks Jamal.