diff mbox

fix hostap registration order

Message ID alpine.SOC.1.00.1012100020110.26229@math.ut.ee
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Meelis Roos Dec. 9, 2010, 11:19 p.m. UTC
In 2.6.37-rc4, hostap_pci init gives a WARNING with backtrace telling 
that netif_stop_queue is called before register_netdev. Fix it by moving 
this call after register_netdev. Removes the warning and seems to work, 
but why is the call to netif_stop_queue needed at all after 
register_netdev?

Signed-off-by: Meelis Roos <mroos@linux.ee>

Comments

David Miller Dec. 10, 2010, 3:53 a.m. UTC | #1
From: Meelis Roos <mroos@linux.ee>
Date: Fri, 10 Dec 2010 01:19:23 +0200 (EET)

> In 2.6.37-rc4, hostap_pci init gives a WARNING with backtrace telling 
> that netif_stop_queue is called before register_netdev. Fix it by moving 
> this call after register_netdev. Removes the warning and seems to work, 
> but why is the call to netif_stop_queue needed at all after 
> register_netdev?
> 
> Signed-off-by: Meelis Roos <mroos@linux.ee>

It should simply not touch the queue state at all at this point.

Your change would add a race.  At the moment the device is registered
it can be brought up, and then your code will eroneously modify
the queue state.
--
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
Meelis Roos Dec. 10, 2010, 7:19 a.m. UTC | #2
> > In 2.6.37-rc4, hostap_pci init gives a WARNING with backtrace telling 
> > that netif_stop_queue is called before register_netdev. Fix it by moving 
> > this call after register_netdev. Removes the warning and seems to work, 
> > but why is the call to netif_stop_queue needed at all after 
> > register_netdev?
> 
> It should simply not touch the queue state at all at this point.
> 
> Your change would add a race.  At the moment the device is registered
> it can be brought up, and then your code will eroneously modify
> the queue state.

OK, I can make a simpler patch for that but I would like to understand 
the original reasons for having netif_stop_queue there. Just a relict or 
still somehow important?

Jouni?
David Miller Dec. 10, 2010, 5:47 p.m. UTC | #3
From: Meelis Roos <mroos@linux.ee>
Date: Fri, 10 Dec 2010 09:19:51 +0200 (EET)

>> > In 2.6.37-rc4, hostap_pci init gives a WARNING with backtrace telling 
>> > that netif_stop_queue is called before register_netdev. Fix it by moving 
>> > this call after register_netdev. Removes the warning and seems to work, 
>> > but why is the call to netif_stop_queue needed at all after 
>> > register_netdev?
>> 
>> It should simply not touch the queue state at all at this point.
>> 
>> Your change would add a race.  At the moment the device is registered
>> it can be brought up, and then your code will eroneously modify
>> the queue state.
> 
> OK, I can make a simpler patch for that but I would like to understand 
> the original reasons for having netif_stop_queue there. Just a relict or 
> still somehow important?

I think it was neither.

The queue state is "don't care" at this point, and always has been,
and a lot of drivers had this unnecessary netif_stop_queue() call
there.
--
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
diff mbox

Patch

diff --git a/drivers/net/wireless/hostap/hostap_hw.c b/drivers/net/wireless/hostap/hostap_hw.c
index b7cb165..cc760c5 100644
--- a/drivers/net/wireless/hostap/hostap_hw.c
+++ b/drivers/net/wireless/hostap/hostap_hw.c
@@ -3253,6 +3253,8 @@  while (0)
 	}
 	printk(KERN_INFO "%s: Registered netdevice %s\n", dev_info, dev->name);
 
+	netif_stop_queue(dev);
+
 	hostap_init_data(local);
 	return dev;
 
diff --git a/drivers/net/wireless/hostap/hostap_main.c b/drivers/net/wireless/hostap/hostap_main.c
index 25a2722..0f1b202 100644
--- a/drivers/net/wireless/hostap/hostap_main.c
+++ b/drivers/net/wireless/hostap/hostap_main.c
@@ -100,6 +100,7 @@  struct net_device * hostap_add_interface(struct local_info *local,
 	printk(KERN_DEBUG "%s: registered netdevice %s\n",
 	       mdev->name, dev->name);
 
+	netif_stop_queue(dev);
 	return dev;
 }
 
@@ -891,7 +892,6 @@  void hostap_setup_dev(struct net_device *dev, local_info_t *local,
 
 	SET_ETHTOOL_OPS(dev, &prism2_ethtool_ops);
 
-	netif_stop_queue(dev);
 }
 
 static int hostap_enable_hostapd(local_info_t *local, int rtnl_locked)