* [PATCH net] Phonet: hold GPRS device while cleaning up
@ 2008-12-08 10:09 Rémi Denis-Courmont
2008-12-08 10:21 ` David Miller
2008-12-09 8:15 ` [PATCH net] Phonet: hold GPRS device while cleaning up David Miller
0 siblings, 2 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-08 10:09 UTC (permalink / raw)
To: netdev
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/pep-gprs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index 9978afb..9602bfc 100644
--- a/net/phonet/pep-gprs.c
+++ b/net/phonet/pep-gprs.c
@@ -340,8 +340,10 @@ void gprs_detach(struct sock *sk)
release_sock(sk);
printk(KERN_DEBUG"%s: detached\n", net->name);
+ dev_hold(net); /* TX work still needs it */
unregister_netdev(net);
flush_scheduled_work();
+ dev_put(net);
sock_put(sk);
skb_queue_purge(&dev->tx_queue);
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: hold GPRS device while cleaning up
2008-12-08 10:09 [PATCH net] Phonet: hold GPRS device while cleaning up Rémi Denis-Courmont
@ 2008-12-08 10:21 ` David Miller
2008-12-08 11:03 ` Rémi Denis-Courmont
2008-12-09 8:15 ` [PATCH net] Phonet: hold GPRS device while cleaning up David Miller
1 sibling, 1 reply; 22+ messages in thread
From: David Miller @ 2008-12-08 10:21 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Mon, 8 Dec 2008 12:09:25 +0200
This is just a side-comment about the phonet stack, not directly
related to this patch.
> diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
> index 9978afb..9602bfc 100644
> --- a/net/phonet/pep-gprs.c
> +++ b/net/phonet/pep-gprs.c
> @@ -340,8 +340,10 @@ void gprs_detach(struct sock *sk)
> release_sock(sk);
>
> printk(KERN_DEBUG"%s: detached\n", net->name);
> + dev_hold(net); /* TX work still needs it */
> unregister_netdev(net);
> flush_scheduled_work();
> + dev_put(net);
> sock_put(sk);
> skb_queue_purge(&dev->tx_queue);
> }
Can we get the phonet stack using more sensible local
variable names for netdev pointers?
We have network namespace objects, and everything says
"net" as the variable name to hold pointers to such
objects.
Using the same local variable name for "struct netdev"
pointers breeds confusion and makes phonet patches and
code difficult to read for people familiar with the
rest of the Linux networking.
Since you use "dev", which is the typical way to name
netdev pointers in networking code, for the phonet et al.
private stuff, one thing you can do is adopt the technique
I usually use:
1) Driver/subsystem private objects get a two letter local
variable name, the second letter is 'p' for pointer.
So a TG3 driver private pointer variable is "tp". You'll
see this in pretty much every single driver I ever wrote.
2) struct netdev pointer variables are "dev"
3) Network namespace pointer variables are "net"
With this we'll achieve consistency and much easier to read
and validate patches.
Just a suggestion :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: hold GPRS device while cleaning up
2008-12-08 10:21 ` David Miller
@ 2008-12-08 11:03 ` Rémi Denis-Courmont
2008-12-08 11:45 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-08 11:03 UTC (permalink / raw)
To: ext David Miller; +Cc: netdev
On Monday 08 December 2008 12:21:53 ext David Miller, you wrote:
> Can we get the phonet stack using more sensible local
> variable names for netdev pointers?
Sure. Should I send a large cosmetic s/net/dev/ et al patch against net-next?
--
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: hold GPRS device while cleaning up
2008-12-08 11:03 ` Rémi Denis-Courmont
@ 2008-12-08 11:45 ` David Miller
2008-12-09 7:45 ` [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
0 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2008-12-08 11:45 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Mon, 8 Dec 2008 13:03:55 +0200
> On Monday 08 December 2008 12:21:53 ext David Miller, you wrote:
> > Can we get the phonet stack using more sensible local
> > variable names for netdev pointers?
>
> Sure. Should I send a large cosmetic s/net/dev/ et al patch against net-next?
That would work just fine, thanks.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Phonet: improve GPRS variable names
2008-12-08 11:45 ` David Miller
@ 2008-12-09 7:45 ` Rémi Denis-Courmont
2008-12-09 7:51 ` Rémi Denis-Courmont
2008-12-09 23:29 ` David Miller
0 siblings, 2 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-09 7:45 UTC (permalink / raw)
To: netdev
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(-)
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);
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Phonet: improve GPRS variable names
2008-12-09 7:45 ` [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
@ 2008-12-09 7:51 ` Rémi Denis-Courmont
2008-12-09 8:05 ` David Miller
2008-12-09 23:29 ` David Miller
1 sibling, 1 reply; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-09 7:51 UTC (permalink / raw)
To: netdev
That one depends on the first patch though.
--
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Phonet: improve GPRS variable names
2008-12-09 7:45 ` [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
2008-12-09 7:51 ` Rémi Denis-Courmont
@ 2008-12-09 23:29 ` David Miller
2008-12-10 13:14 ` Rémi Denis-Courmont
1 sibling, 1 reply; 22+ messages in thread
From: David Miller @ 2008-12-09 23:29 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Phonet: improve GPRS variable names
2008-12-09 23:29 ` David Miller
@ 2008-12-10 13:14 ` Rémi Denis-Courmont
2008-12-10 13:15 ` [PATCH net] Phonet: ensure GPRS device does not go away during TX work Rémi Denis-Courmont
2008-12-10 13:15 ` [PATCH net-next] Phonet: improve GPRS variable names Rémi Denis-Courmont
0 siblings, 2 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-10 13:14 UTC (permalink / raw)
To: ext David Miller, netdev
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.
--
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net] Phonet: ensure GPRS device does not go away during TX work
2008-12-10 13:14 ` Rémi Denis-Courmont
@ 2008-12-10 13:15 ` Rémi Denis-Courmont
2008-12-10 23:28 ` David Miller
2008-12-10 13:15 ` [PATCH net-next] Phonet: improve GPRS variable names Rémi Denis-Courmont
1 sibling, 1 reply; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-10 13:15 UTC (permalink / raw)
To: netdev
We need to hold the device while TX work is pending, as work is flushed
only after the network device is unregistered.
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/pep-gprs.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index e6e8e44..5985bc5 100644
--- a/net/phonet/pep-gprs.c
+++ b/net/phonet/pep-gprs.c
@@ -184,6 +184,7 @@ static int gprs_xmit(struct sk_buff *skb, struct net_device *net)
spin_lock(&dev->tx_lock);
if (likely(skb_queue_len(&dev->tx_queue) < dev->tx_max)) {
skb_queue_tail(&dev->tx_queue, skb);
+ dev_hold(net);
skb = NULL;
}
if (skb_queue_len(&dev->tx_queue) >= dev->tx_max)
@@ -221,6 +222,7 @@ static void gprs_tx(struct work_struct *work)
net->stats.tx_errors++;
}
release_sock(sk);
+ dev_put(net);
}
lock_sock(sk);
--
1.5.4.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: ensure GPRS device does not go away during TX work
2008-12-10 13:15 ` [PATCH net] Phonet: ensure GPRS device does not go away during TX work Rémi Denis-Courmont
@ 2008-12-10 23:28 ` David Miller
2008-12-11 8:11 ` Rémi Denis-Courmont
0 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2008-12-10 23:28 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Wed, 10 Dec 2008 15:15:28 +0200
> We need to hold the device while TX work is pending, as work is flushed
> only after the network device is unregistered.
>
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
This seems unnecessary.
If you simply create proper ->open and ->stop methods for the GPRS net
device you can purge the TX queue in ->stop() and avoid this
refcounting business.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: ensure GPRS device does not go away during TX work
2008-12-10 23:28 ` David Miller
@ 2008-12-11 8:11 ` Rémi Denis-Courmont
2008-12-11 8:33 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-11 8:11 UTC (permalink / raw)
To: ext David Miller, netdev
On Thursday 11 December 2008 01:28:45 ext David Miller, you wrote:
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> Date: Wed, 10 Dec 2008 15:15:28 +0200
>
> > We need to hold the device while TX work is pending, as work is flushed
> > only after the network device is unregistered.
> >
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> This seems unnecessary.
>
> If you simply create proper ->open and ->stop methods for the GPRS net
> device you can purge the TX queue in ->stop() and avoid this
> refcounting business.
Ok. Can I assume that scheduled work will be flushed sometime between ->stop
and ->destructor then?
--
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: ensure GPRS device does not go away during TX work
2008-12-11 8:11 ` Rémi Denis-Courmont
@ 2008-12-11 8:33 ` David Miller
2008-12-11 14:04 ` [PATCH net-2.6] Phonet: keep TX queue disabled when the device is off Rémi Denis-Courmont
0 siblings, 1 reply; 22+ messages in thread
From: David Miller @ 2008-12-11 8:33 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Thu, 11 Dec 2008 10:11:17 +0200
> Can I assume that scheduled work will be flushed sometime between
> ->stop and ->destructor then?
If you have a workqueue you can explicitly flush it in your
->stop() method.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH net-2.6] Phonet: keep TX queue disabled when the device is off
2008-12-11 8:33 ` David Miller
@ 2008-12-11 14:04 ` Rémi Denis-Courmont
2008-12-15 8:54 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-11 14:04 UTC (permalink / raw)
To: netdev
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/pep-gprs.c | 27 ++++++++++++++++++++++-----
1 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index e6e8e44..22848dd 100644
--- a/net/phonet/pep-gprs.c
+++ b/net/phonet/pep-gprs.c
@@ -155,12 +155,13 @@ static void gprs_data_ready(struct sock *sk, int len)
static void gprs_write_space(struct sock *sk)
{
struct gprs_dev *dev = sk->sk_user_data;
+ struct net_device *net = dev->net;
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);
+ if (credits > skb_queue_len(&dev->tx_queue) && netif_running(net))
+ netif_wake_queue(net);
spin_unlock_bh(&dev->tx_lock);
}
@@ -168,6 +169,23 @@ static void gprs_write_space(struct sock *sk)
* Network device callbacks
*/
+static int gprs_open(struct net_device *dev)
+{
+ struct gprs_dev *gp = netdev_priv(dev);
+
+ gprs_write_space(gp->sk);
+ return 0;
+}
+
+static int gprs_close(struct net_device *dev)
+{
+ struct gprs_dev *gp = netdev_priv(dev);
+
+ netif_stop_queue(dev);
+ flush_work(&gp->tx_work);
+ return 0;
+}
+
static int gprs_xmit(struct sk_buff *skb, struct net_device *net)
{
struct gprs_dev *dev = netdev_priv(net);
@@ -248,6 +266,8 @@ static void gprs_setup(struct net_device *net)
net->tx_queue_len = 10;
net->destructor = free_netdev;
+ net->open = gprs_open;
+ net->stop = gprs_close;
net->hard_start_xmit = gprs_xmit; /* mandatory */
net->change_mtu = gprs_set_mtu;
}
@@ -311,7 +331,6 @@ int gprs_attach(struct sock *sk)
dev->sk = sk;
printk(KERN_DEBUG"%s: attached\n", net->name);
- gprs_write_space(sk); /* kick off TX */
return net->ifindex;
out_rel:
@@ -334,7 +353,5 @@ void gprs_detach(struct sock *sk)
printk(KERN_DEBUG"%s: detached\n", net->name);
unregister_netdev(net);
- flush_scheduled_work();
sock_put(sk);
- skb_queue_purge(&dev->tx_queue);
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH net-next] Phonet: improve GPRS variable names
2008-12-10 13:14 ` Rémi Denis-Courmont
2008-12-10 13:15 ` [PATCH net] Phonet: ensure GPRS device does not go away during TX work Rémi Denis-Courmont
@ 2008-12-10 13:15 ` Rémi Denis-Courmont
1 sibling, 0 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-10 13:15 UTC (permalink / raw)
To: netdev
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(-)
diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index 5985bc5..e60ba59 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,17 +183,17 @@ 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);
- dev_hold(net);
+ spin_lock(&gp->tx_lock);
+ if (likely(skb_queue_len(&gp->tx_queue) < gp->tx_max)) {
+ skb_queue_tail(&gp->tx_queue, skb);
+ dev_hold(dev);
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;
@@ -199,16 +201,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);
@@ -217,12 +219,12 @@ 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);
- dev_put(net);
+ dev_put(dev);
}
lock_sock(sk);
@@ -230,28 +232,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;
}
/*
@@ -265,28 +267,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;
}
@@ -300,43 +302,43 @@ 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);
- unregister_netdev(net);
+ printk(KERN_DEBUG"%s: detached\n", dev->name);
+ unregister_netdev(dev);
flush_scheduled_work();
sock_put(sk);
- skb_queue_purge(&dev->tx_queue);
+ skb_queue_purge(&gp->tx_queue);
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: hold GPRS device while cleaning up
2008-12-08 10:09 [PATCH net] Phonet: hold GPRS device while cleaning up Rémi Denis-Courmont
2008-12-08 10:21 ` David Miller
@ 2008-12-09 8:15 ` David Miller
2008-12-09 15:03 ` Rémi Denis-Courmont
1 sibling, 1 reply; 22+ messages in thread
From: David Miller @ 2008-12-09 8:15 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Mon, 8 Dec 2008 12:09:25 +0200
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Applied.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: hold GPRS device while cleaning up
2008-12-09 8:15 ` [PATCH net] Phonet: hold GPRS device while cleaning up David Miller
@ 2008-12-09 15:03 ` Rémi Denis-Courmont
2008-12-09 23:28 ` David Miller
0 siblings, 1 reply; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-09 15:03 UTC (permalink / raw)
To: ext David Miller; +Cc: netdev
On Tuesday 09 December 2008 10:15:11 ext David Miller, you wrote:
> From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> Date: Mon, 8 Dec 2008 12:09:25 +0200
>
> > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
>
> Applied.
Uho :( I ended up not testing what I thought I was :-$ This patch deadlocks
unregister_netdev(), since that waits for the refcount to drop to zero.
--
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH net] Phonet: hold GPRS device while cleaning up
2008-12-09 15:03 ` Rémi Denis-Courmont
@ 2008-12-09 23:28 ` David Miller
0 siblings, 0 replies; 22+ messages in thread
From: David Miller @ 2008-12-09 23:28 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: "Rémi Denis-Courmont" <remi.denis-courmont@nokia.com>
Date: Tue, 9 Dec 2008 17:03:41 +0200
> On Tuesday 09 December 2008 10:15:11 ext David Miller, you wrote:
> > From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> > Date: Mon, 8 Dec 2008 12:09:25 +0200
> >
> > > Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
> >
> > Applied.
>
> Uho :( I ended up not testing what I thought I was :-$ This patch deadlocks
> unregister_netdev(), since that waits for the refcount to drop to zero.
Lucky for you I didn't push this out anywhere yet. I'll
just revert it locally.
You're on double-secret-probation, please test your patches
in the future :)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] Phonet: improve GPRS variable names
@ 2008-12-15 14:02 Rémi Denis-Courmont
2008-12-15 14:06 ` Rémi Denis-Courmont
2008-12-16 9:18 ` David Miller
0 siblings, 2 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-15 14:02 UTC (permalink / raw)
To: netdev
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(-)
diff --git a/net/phonet/pep-gprs.c b/net/phonet/pep-gprs.c
index 22848dd..0b640b0 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,33 +138,33 @@ 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 net_device *net = dev->net;
+ struct gprs_dev *gp = sk->sk_user_data;
+ struct net_device *dev = gp->dev;
unsigned credits = pep_writeable(sk);
- spin_lock_bh(&dev->tx_lock);
- dev->tx_max = credits;
- if (credits > skb_queue_len(&dev->tx_queue) && netif_running(net))
- netif_wake_queue(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_running(dev))
+ netif_wake_queue(dev);
+ spin_unlock_bh(&gp->tx_lock);
}
/*
@@ -186,9 +188,9 @@ static int gprs_close(struct net_device *dev)
return 0;
}
-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):
@@ -199,16 +201,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;
@@ -216,16 +218,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);
@@ -234,9 +236,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);
}
@@ -246,30 +248,30 @@ 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->open = gprs_open;
- net->stop = gprs_close;
- 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->open = gprs_open;
+ dev->stop = gprs_close;
+ dev->hard_start_xmit = gprs_xmit; /* mandatory */
+ dev->change_mtu = gprs_set_mtu;
}
/*
@@ -283,28 +285,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;
}
@@ -318,40 +320,40 @@ 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);
- return net->ifindex;
+ printk(KERN_DEBUG"%s: attached\n", dev->name);
+ 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);
- unregister_netdev(net);
+ printk(KERN_DEBUG"%s: detached\n", dev->name);
+ unregister_netdev(dev);
sock_put(sk);
}
--
1.5.4.3
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] Phonet: improve GPRS variable names
2008-12-15 14:02 [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
@ 2008-12-15 14:06 ` Rémi Denis-Courmont
2008-12-16 9:18 ` David Miller
1 sibling, 0 replies; 22+ messages in thread
From: Rémi Denis-Courmont @ 2008-12-15 14:06 UTC (permalink / raw)
To: netdev
That depends on the earlier Phonet patch in net-2.6.
I resolved the merge this way (but I could be wrong):
diff --cc drivers/net/e1000e/ich8lan.c
index 92f2ace,d115a6d..0000000
--- a/drivers/net/e1000e/ich8lan.c
+++ b/drivers/net/e1000e/ich8lan.c
@@@ -1939,7 -1893,8 +1939,8 @@@ static s32 e1000_reset_hw_ich8lan(struc
ctrl |= E1000_CTRL_PHY_RST;
}
ret_val = e1000_acquire_swflag_ich8lan(hw);
+ /* Whether or not the swflag was acquired, we need to reset the part */
- hw_dbg(hw, "Issuing a global reset to ich8lan");
+ hw_dbg(hw, "Issuing a global reset to ich8lan\n");
ew32(CTRL, (ctrl | E1000_CTRL_RST));
msleep(20);
--
Rémi Denis-Courmont
Maemo Software, Nokia Devices R&D
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] Phonet: improve GPRS variable names
2008-12-15 14:02 [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
2008-12-15 14:06 ` Rémi Denis-Courmont
@ 2008-12-16 9:18 ` David Miller
1 sibling, 0 replies; 22+ messages in thread
From: David Miller @ 2008-12-16 9:18 UTC (permalink / raw)
To: remi.denis-courmont; +Cc: netdev
From: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Date: Mon, 15 Dec 2008 16:02:04 +0200
> Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2008-12-16 9:18 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-12-08 10:09 [PATCH net] Phonet: hold GPRS device while cleaning up Rémi Denis-Courmont
2008-12-08 10:21 ` David Miller
2008-12-08 11:03 ` Rémi Denis-Courmont
2008-12-08 11:45 ` David Miller
2008-12-09 7:45 ` [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
2008-12-09 7:51 ` Rémi Denis-Courmont
2008-12-09 8:05 ` David Miller
2008-12-09 23:29 ` David Miller
2008-12-10 13:14 ` Rémi Denis-Courmont
2008-12-10 13:15 ` [PATCH net] Phonet: ensure GPRS device does not go away during TX work Rémi Denis-Courmont
2008-12-10 23:28 ` David Miller
2008-12-11 8:11 ` Rémi Denis-Courmont
2008-12-11 8:33 ` David Miller
2008-12-11 14:04 ` [PATCH net-2.6] Phonet: keep TX queue disabled when the device is off Rémi Denis-Courmont
2008-12-15 8:54 ` David Miller
2008-12-10 13:15 ` [PATCH net-next] Phonet: improve GPRS variable names Rémi Denis-Courmont
2008-12-09 8:15 ` [PATCH net] Phonet: hold GPRS device while cleaning up David Miller
2008-12-09 15:03 ` Rémi Denis-Courmont
2008-12-09 23:28 ` David Miller
2008-12-15 14:02 [PATCH] Phonet: improve GPRS variable names Rémi Denis-Courmont
2008-12-15 14:06 ` Rémi Denis-Courmont
2008-12-16 9:18 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.