Patchwork [net] Phonet: ensure GPRS device does not go away during TX work

login
register
mail settings
Submitter Rémi Denis-Courmont
Date Dec. 10, 2008, 1:15 p.m.
Message ID <1228914929-26337-1-git-send-email-remi.denis-courmont@nokia.com>
Download mbox | patch
Permalink /patch/13163/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Rémi Denis-Courmont - Dec. 10, 2008, 1:15 p.m.
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(-)
David Miller - Dec. 10, 2008, 11:28 p.m.
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.
--
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. 11, 2008, 8:11 a.m.
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?
David Miller - Dec. 11, 2008, 8:33 a.m.
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.
--
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

Patch

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);