Thread (7 messages) 7 messages, 4 authors, 2011-01-12

Re: rndis gadget: Inconsistent locking

From: Eric Dumazet <hidden>
Date: 2011-01-12 13:07:12

Possibly related (same subject, not in this thread)

Le mercredi 12 janvier 2011 à 12:28 +0000, Jarek Poplawski a écrit :
quoted hunk
On 2011-01-10 13:14, David Brownell wrote:
quoted
quoted
I have just retested Michals patch but I have found another
lockdep failure.

It looks like rndis_msg_parser() can call dev_get_stats> too:
Can someone provide a fully working patch then?
Or maybe just revert the one that caused all the
breakage??

Rememeber that this driver was working fine for
years before netdev changes added multiple bugs
because (at least) they didn't update all callers.
Maybe I miss something, but if it's like Michal described something
like this should be enough (not tested nor compiled).

Jarek P.
---

 drivers/usb/gadget/f_phonet.c |    6 ++++++
 drivers/usb/gadget/u_ether.c  |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/gadget/f_phonet.c b/drivers/usb/gadget/f_phonet.c
index 3c6e1a0..bef4622 100644
--- a/drivers/usb/gadget/f_phonet.c
+++ b/drivers/usb/gadget/f_phonet.c
@@ -207,6 +207,11 @@ static int pn_net_close(struct net_device *dev)
 	return 0;
 }
 
+static struct net_device_stats *pn_net_stats(struct net_device *dev)
+{
+	return &dev->stats;
+}
+
 static void pn_tx_complete(struct usb_ep *ep, struct usb_request *req)
 {
 	struct f_phonet *fp = ep->driver_data;
@@ -279,6 +284,7 @@ static int pn_net_mtu(struct net_device *dev, int new_mtu)
 static const struct net_device_ops pn_netdev_ops = {
 	.ndo_open	= pn_net_open,
 	.ndo_stop	= pn_net_close,
+	.ndo_get_stats	= pn_net_stats,
 	.ndo_start_xmit	= pn_net_xmit,
 	.ndo_change_mtu	= pn_net_mtu,
 };
diff --git a/drivers/usb/gadget/u_ether.c b/drivers/usb/gadget/u_ether.c
index 1eda968..f21520f 100644
--- a/drivers/usb/gadget/u_ether.c
+++ b/drivers/usb/gadget/u_ether.c
@@ -702,6 +702,11 @@ static int eth_stop(struct net_device *net)
 	return 0;
 }
 
+static struct net_device_stats *eth_get_stats(struct net_device *net)
+{
+	return &net->stats;
+}
+
 /*-------------------------------------------------------------------------*/
 
 /* initial value, changed by "ifconfig usb0 hw ether xx:xx:xx:xx:xx:xx" */
@@ -740,6 +745,7 @@ static struct eth_dev *the_dev;
 static const struct net_device_ops eth_netdev_ops = {
 	.ndo_open		= eth_open,
 	.ndo_stop		= eth_stop,
+	.ndo_get_stats		= eth_get_stats,
 	.ndo_start_xmit		= eth_start_xmit,
 	.ndo_change_mtu		= ueth_change_mtu,
 	.ndo_set_mac_address 	= eth_mac_addr,
--
Hmm... 

So all net devices in gen_ndis_query_resp() should have a
ndo_get_stats() or ndo_get_stats64() method, not allowed to use
spin_lock_bh() / spin_unlock_bh()

If yes, we should add big fat comments to pn_net_stats()/eth_get_stats() so that nobody
tries to revert your patch ;)



--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help