diff mbox

Phonet: improve GPRS variable names

Message ID 1228808759-8016-1-git-send-email-remi.denis-courmont@nokia.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Rémi Denis-Courmont Dec. 9, 2008, 7:45 a.m. UTC 
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
 net/phonet/pep-gprs.c |  170 +++++++++++++++++++++++++------------------------
 1 files changed, 86 insertions(+), 84 deletions(-)

Comments

Rémi Denis-Courmont Dec. 9, 2008, 7:51 a.m. UTC | #1 
That one depends on the first patch though.
David Miller Dec. 9, 2008, 8:05 a.m. UTC | #2 
From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Tue, 9 Dec 2008 09:51:16 +0200

> That one depends on the first patch though.

No problem, thanks for mentioning.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Dec. 9, 2008, 11:29 p.m. UTC | #3 
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Tue,  9 Dec 2008 09:45:59 +0200

> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>

Since this depends upon the buggy refcount change, I'm
tossing this and will wait for an updated version once
everything is sorted out.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rémi Denis-Courmont Dec. 10, 2008, 1:14 p.m. UTC | #4 
On Wednesday 10 December 2008 01:29:11 ext David Miller, you wrote:
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> Date: Tue,  9 Dec 2008 09:45:59 +0200
>
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> Since this depends upon the buggy refcount change, I'm
> tossing this and will wait for an updated version once
> everything is sorted out.

Unfortunately, I don't think there is a way around this dependency.

I believe there is a -theoretical- race condition between the deferred work, 
and the device unregistering, but I could not produce it this far. The fixed 
patches follow.
diff mbox

Patch

diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index a24d01e..7461a61 100644
--- a/net/phonet/pep-gprs.c
+++ b/net/phonet/pep-gprs.c
@@ -40,7 +40,7 @@  struct gprs_dev {
 	void			(*old_data_ready)(struct sock *, int);
 	void			(*old_write_space)(struct sock *);
 
-	struct net_device	*net;
+	struct net_device	*dev;
 
 	struct sk_buff_head	tx_queue;
 	struct work_struct	tx_work;
@@ -72,17 +72,19 @@  static __be16 gprs_type_trans(struct sk_buff *skb)
 
 static void gprs_state_change(struct sock *sk)
 {
-	struct gprs_dev *dev = sk->sk_user_data;
+	struct gprs_dev *gp = sk->sk_user_data;
 
 	if (sk->sk_state == TCP_CLOSE_WAIT) {
-		netif_stop_queue(dev->net);
-		netif_carrier_off(dev->net);
+		struct net_device *dev = gp->dev;
+
+		netif_stop_queue(dev);
+		netif_carrier_off(dev);
 	}
 }
 
-static int gprs_recv(struct gprs_dev *dev, struct sk_buff *skb)
+static int gprs_recv(struct gprs_dev *gp, struct sk_buff *skb)
 {
-	struct net_device *net = dev->net;
+	struct net_device *dev = gp->dev;
 	int err = 0;
 	__be16 protocol = gprs_type_trans(skb);
 
@@ -99,7 +101,7 @@  static int gprs_recv(struct gprs_dev *dev, struct sk_buff *skb)
 		 * so wrap the IP packet as a single fragment of an head-less
 		 * socket buffer. The network stack will pull what it needs,
 		 * but at least, the whole IP payload is not memcpy'd. */
-		rskb = netdev_alloc_skb(net, 0);
+		rskb = netdev_alloc_skb(dev, 0);
 		if (!rskb) {
 			err = -ENOBUFS;
 			goto drop;
@@ -123,11 +125,11 @@  static int gprs_recv(struct gprs_dev *dev, struct sk_buff *skb)
 
 	skb->protocol = protocol;
 	skb_reset_mac_header(skb);
-	skb->dev = net;
+	skb->dev = dev;
 
-	if (likely(net->flags & IFF_UP)) {
-		net->stats.rx_packets++;
-		net->stats.rx_bytes += skb->len;
+	if (likely(dev->flags & IFF_UP)) {
+		dev->stats.rx_packets++;
+		dev->stats.rx_bytes += skb->len;
 		netif_rx(skb);
 		skb = NULL;
 	} else
@@ -136,41 +138,41 @@  static int gprs_recv(struct gprs_dev *dev, struct sk_buff *skb)
 drop:
 	if (skb) {
 		dev_kfree_skb(skb);
-		net->stats.rx_dropped++;
+		dev->stats.rx_dropped++;
 	}
 	return err;
 }
 
 static void gprs_data_ready(struct sock *sk, int len)
 {
-	struct gprs_dev *dev = sk->sk_user_data;
+	struct gprs_dev *gp = sk->sk_user_data;
 	struct sk_buff *skb;
 
 	while ((skb = pep_read(sk)) != NULL) {
 		skb_orphan(skb);
-		gprs_recv(dev, skb);
+		gprs_recv(gp, skb);
 	}
 }
 
 static void gprs_write_space(struct sock *sk)
 {
-	struct gprs_dev *dev = sk->sk_user_data;
+	struct gprs_dev *gp = sk->sk_user_data;
 	unsigned credits = pep_writeable(sk);
 
-	spin_lock_bh(&dev->tx_lock);
-	dev->tx_max = credits;
-	if (credits > skb_queue_len(&dev->tx_queue))
-		netif_wake_queue(dev->net);
-	spin_unlock_bh(&dev->tx_lock);
+	spin_lock_bh(&gp->tx_lock);
+	gp->tx_max = credits;
+	if (credits > skb_queue_len(&gp->tx_queue))
+		netif_wake_queue(gp->dev);
+	spin_unlock_bh(&gp->tx_lock);
 }
 
 /*
  * Network device callbacks
  */
 
-static int gprs_xmit(struct sk_buff *skb, struct net_device *net)
+static int gprs_xmit(struct sk_buff *skb, struct net_device *dev)
 {
-	struct gprs_dev *dev = netdev_priv(net);
+	struct gprs_dev *gp = netdev_priv(dev);
 
 	switch (skb->protocol) {
 	case  htons(ETH_P_IP):
@@ -181,16 +183,16 @@  static int gprs_xmit(struct sk_buff *skb, struct net_device *net)
 		return 0;
 	}
 
-	spin_lock(&dev->tx_lock);
-	if (likely(skb_queue_len(&dev->tx_queue) < dev->tx_max)) {
-		skb_queue_tail(&dev->tx_queue, skb);
+	spin_lock(&gp->tx_lock);
+	if (likely(skb_queue_len(&gp->tx_queue) < gp->tx_max)) {
+		skb_queue_tail(&gp->tx_queue, skb);
 		skb = NULL;
 	}
-	if (skb_queue_len(&dev->tx_queue) >= dev->tx_max)
-		netif_stop_queue(net);
-	spin_unlock(&dev->tx_lock);
+	if (skb_queue_len(&gp->tx_queue) >= gp->tx_max)
+		netif_stop_queue(dev);
+	spin_unlock(&gp->tx_lock);
 
-	schedule_work(&dev->tx_work);
+	schedule_work(&gp->tx_work);
 	if (unlikely(skb))
 		dev_kfree_skb(skb);
 	return 0;
@@ -198,16 +200,16 @@  static int gprs_xmit(struct sk_buff *skb, struct net_device *net)
 
 static void gprs_tx(struct work_struct *work)
 {
-	struct gprs_dev *dev = container_of(work, struct gprs_dev, tx_work);
-	struct net_device *net = dev->net;
-	struct sock *sk = dev->sk;
+	struct gprs_dev *gp = container_of(work, struct gprs_dev, tx_work);
+	struct net_device *dev = gp->dev;
+	struct sock *sk = gp->sk;
 	struct sk_buff *skb;
 
-	while ((skb = skb_dequeue(&dev->tx_queue)) != NULL) {
+	while ((skb = skb_dequeue(&gp->tx_queue)) != NULL) {
 		int err;
 
-		net->stats.tx_bytes += skb->len;
-		net->stats.tx_packets++;
+		dev->stats.tx_bytes += skb->len;
+		dev->stats.tx_packets++;
 
 		skb_orphan(skb);
 		skb_set_owner_w(skb, sk);
@@ -216,9 +218,9 @@  static void gprs_tx(struct work_struct *work)
 		err = pep_write(sk, skb);
 		if (err) {
 			LIMIT_NETDEBUG(KERN_WARNING"%s: TX error (%d)\n",
-					net->name, err);
-			net->stats.tx_aborted_errors++;
-			net->stats.tx_errors++;
+					dev->name, err);
+			dev->stats.tx_aborted_errors++;
+			dev->stats.tx_errors++;
 		}
 		release_sock(sk);
 	}
@@ -228,28 +230,28 @@  static void gprs_tx(struct work_struct *work)
 	release_sock(sk);
 }
 
-static int gprs_set_mtu(struct net_device *net, int new_mtu)
+static int gprs_set_mtu(struct net_device *dev, int new_mtu)
 {
 	if ((new_mtu < 576) || (new_mtu > (PHONET_MAX_MTU - 11)))
 		return -EINVAL;
 
-	net->mtu = new_mtu;
+	dev->mtu = new_mtu;
 	return 0;
 }
 
-static void gprs_setup(struct net_device *net)
+static void gprs_setup(struct net_device *dev)
 {
-	net->features		= NETIF_F_FRAGLIST;
-	net->type		= ARPHRD_NONE;
-	net->flags		= IFF_POINTOPOINT | IFF_NOARP;
-	net->mtu		= GPRS_DEFAULT_MTU;
-	net->hard_header_len	= 0;
-	net->addr_len		= 0;
-	net->tx_queue_len	= 10;
-
-	net->destructor		= free_netdev;
-	net->hard_start_xmit	= gprs_xmit; /* mandatory */
-	net->change_mtu		= gprs_set_mtu;
+	dev->features		= NETIF_F_FRAGLIST;
+	dev->type		= ARPHRD_NONE;
+	dev->flags		= IFF_POINTOPOINT | IFF_NOARP;
+	dev->mtu		= GPRS_DEFAULT_MTU;
+	dev->hard_header_len	= 0;
+	dev->addr_len		= 0;
+	dev->tx_queue_len	= 10;
+
+	dev->destructor		= free_netdev;
+	dev->hard_start_xmit	= gprs_xmit; /* mandatory */
+	dev->change_mtu		= gprs_set_mtu;
 }
 
 /*
@@ -263,28 +265,28 @@  static void gprs_setup(struct net_device *net)
 int gprs_attach(struct sock *sk)
 {
 	static const char ifname[] = "gprs%d";
-	struct gprs_dev *dev;
-	struct net_device *net;
+	struct gprs_dev *gp;
+	struct net_device *dev;
 	int err;
 
 	if (unlikely(sk->sk_type == SOCK_STREAM))
 		return -EINVAL; /* need packet boundaries */
 
 	/* Create net device */
-	net = alloc_netdev(sizeof(*dev), ifname, gprs_setup);
-	if (!net)
+	dev = alloc_netdev(sizeof(*gp), ifname, gprs_setup);
+	if (!dev)
 		return -ENOMEM;
-	dev = netdev_priv(net);
-	dev->net = net;
-	dev->tx_max = 0;
-	spin_lock_init(&dev->tx_lock);
-	skb_queue_head_init(&dev->tx_queue);
-	INIT_WORK(&dev->tx_work, gprs_tx);
-
-	netif_stop_queue(net);
-	err = register_netdev(net);
+	gp = netdev_priv(dev);
+	gp->dev = dev;
+	gp->tx_max = 0;
+	spin_lock_init(&gp->tx_lock);
+	skb_queue_head_init(&gp->tx_queue);
+	INIT_WORK(&gp->tx_work, gprs_tx);
+
+	netif_stop_queue(dev);
+	err = register_netdev(dev);
 	if (err) {
-		free_netdev(net);
+		free_netdev(dev);
 		return err;
 	}
 
@@ -298,45 +300,45 @@  int gprs_attach(struct sock *sk)
 		err = -EINVAL;
 		goto out_rel;
 	}
-	sk->sk_user_data	= dev;
-	dev->old_state_change	= sk->sk_state_change;
-	dev->old_data_ready	= sk->sk_data_ready;
-	dev->old_write_space	= sk->sk_write_space;
+	sk->sk_user_data	= gp;
+	gp->old_state_change	= sk->sk_state_change;
+	gp->old_data_ready	= sk->sk_data_ready;
+	gp->old_write_space	= sk->sk_write_space;
 	sk->sk_state_change	= gprs_state_change;
 	sk->sk_data_ready	= gprs_data_ready;
 	sk->sk_write_space	= gprs_write_space;
 	release_sock(sk);
 
 	sock_hold(sk);
-	dev->sk = sk;
+	gp->sk = sk;
 
-	printk(KERN_DEBUG"%s: attached\n", net->name);
+	printk(KERN_DEBUG"%s: attached\n", dev->name);
 	gprs_write_space(sk); /* kick off TX */
-	return net->ifindex;
+	return dev->ifindex;
 
 out_rel:
 	release_sock(sk);
-	unregister_netdev(net);
+	unregister_netdev(dev);
 	return err;
 }
 
 void gprs_detach(struct sock *sk)
 {
-	struct gprs_dev *dev = sk->sk_user_data;
-	struct net_device *net = dev->net;
+	struct gprs_dev *gp = sk->sk_user_data;
+	struct net_device *dev = gp->dev;
 
 	lock_sock(sk);
 	sk->sk_user_data	= NULL;
-	sk->sk_state_change	= dev->old_state_change;
-	sk->sk_data_ready	= dev->old_data_ready;
-	sk->sk_write_space	= dev->old_write_space;
+	sk->sk_state_change	= gp->old_state_change;
+	sk->sk_data_ready	= gp->old_data_ready;
+	sk->sk_write_space	= gp->old_write_space;
 	release_sock(sk);
 
-	printk(KERN_DEBUG"%s: detached\n", net->name);
-	dev_hold(net); /* TX work still needs it */
-	unregister_netdev(net);
+	printk(KERN_DEBUG"%s: detached\n", dev->name);
+	dev_hold(dev); /* TX work still needs it */
+	unregister_netdev(dev);
 	flush_scheduled_work();
-	dev_put(net);
+	dev_put(dev);
 	sock_put(sk);
-	skb_queue_purge(&dev->tx_queue);
+	skb_queue_purge(&gp->tx_queue);
 }