[PATCH] airo driver: fix races, oops, etc..

[PATCH] airo driver: fix races, oops, etc..

[Benjamin Herrenschmidt]

Hi !

Here's a patch against Linus current airo.c, it adds back some fixes I
did during OLS on the previous version of this driver. I couldn't test
this new 'fixed' version though as I don't have the airo card anymore:

 - Initialize the work_struct structures used by the driver
 - Change most of schedule_work() to schedule_delayed_work(). The
   problem with schedule_work() is that the worker_thread will never
   schedule() if the work keeps getting added back to the list by the
   callback, which typically happened with this driver when the xmit
   work gets scheduled while the semaphore was used by a pending
   command. Note that -ac tree has a modified version of this driver
   that gets rid of this "over-smart" work queue stuff and uses normal
   spinlock instead, probably at the expense of some latency...
 - Fix a small signed vs. unsigned char issue
 - Remove bogus pci_module_init(), use pci_register_driver() instead and
   add missing pci_unregister_driver() so the module can now be removed
   without leaving stale references (and thus avoid an oops next time
   the driver list is walked by the device core).

Jeff, if you are ok with these, please send to Linus,

Ben


diff -urN linux-2.5/drivers/net/wireless/airo.c linuxppc-2.5-benh/drivers/net/wireless/airo.c
--- linux-2.5/drivers/net/wireless/airo.c	2003-07-29 08:51:06.000000000 -0400
+++ linuxppc-2.5-benh/drivers/net/wireless/airo.c	2003-07-29 08:54:26.000000000 -0400
@@ -633,7 +633,7 @@
 	u16 SSIDlen;
 	char SSID[32];
 	char apName[16];
-	char bssid[4][ETH_ALEN];
+	unsigned char bssid[4][ETH_ALEN];
 	u16 beaconPeriod;
 	u16 dimPeriod;
 	u16 atimDuration;
@@ -1031,7 +1031,7 @@
 	struct work_struct promisc_task;
 	struct {
 		struct sk_buff *skb;
-		int fid;
+		int fid, hardirq;
 		struct work_struct task;
 	} xmit, xmit11;
 	struct net_device *wifidev;
@@ -1348,7 +1348,12 @@
 		netif_stop_queue(dev);
 		priv->xmit.task.func = (void (*)(void *))airo_do_xmit;
 		priv->xmit.task.data = (void *)dev;
-		schedule_work(&priv->xmit.task);
+		if (priv->xmit.hardirq) {
+			priv->xmit.hardirq = 0;
+			schedule_work(&priv->xmit.task);
+			return;
+		}
+		schedule_delayed_work(&priv->xmit.task, 1);
 		return;
 	}
 	status = transmit_802_3_packet (priv, fids[fid], skb->data);
@@ -1393,6 +1398,7 @@
 		fids[i] |= (len << 16);
 		priv->xmit.skb = skb;
 		priv->xmit.fid = i;
+		priv->xmit.hardirq = 1;
 		airo_do_xmit(dev);
 	}
 	return 0;
@@ -1410,7 +1416,12 @@
 		netif_stop_queue(dev);
 		priv->xmit11.task.func = (void (*)(void *))airo_do_xmit11;
 		priv->xmit11.task.data = (void *)dev;
-		schedule_work(&priv->xmit11.task);
+		if (priv->xmit11.hardirq) {
+			priv->xmit11.hardirq = 0;
+			schedule_work(&priv->xmit11.task);
+			return;
+		}
+		schedule_delayed_work(&priv->xmit11.task, 1);
 		return;
 	}
 	status = transmit_802_11_packet (priv, fids[fid], skb->data);
@@ -1485,7 +1496,7 @@
 	} else {
 		ai->stats_task.func = (void (*)(void *))airo_read_stats;
 		ai->stats_task.data = (void *)ai;
-		schedule_work(&ai->stats_task);
+		schedule_delayed_work(&ai->stats_task, 1);
 	}
 }
 
@@ -1508,7 +1519,7 @@
 	} else {
 		ai->promisc_task.func = (void (*)(void *))airo_end_promisc;
 		ai->promisc_task.data = (void *)ai;
-		schedule_work(&ai->promisc_task);
+		schedule_delayed_work(&ai->promisc_task, 1);
 	}
 }
 
@@ -1524,7 +1535,7 @@
 	} else {
 		ai->promisc_task.func = (void (*)(void *))airo_set_promisc;
 		ai->promisc_task.data = (void *)ai;
-		schedule_work(&ai->promisc_task);
+		schedule_delayed_work(&ai->promisc_task, 1);
 	}
 }
 
@@ -1710,6 +1721,14 @@
 	sema_init(&ai->sem, 1);
 	ai->need_commit = 0;
 	ai->config.len = 0;
+	INIT_WORK(&ai->stats_task, NULL, NULL);
+	INIT_WORK(&ai->promisc_task, NULL, NULL);
+	INIT_WORK(&ai->xmit.task, NULL, NULL);
+	INIT_WORK(&ai->xmit11.task, NULL, NULL);
+	INIT_WORK(&ai->mic_task, NULL, NULL);
+#ifdef WIRELESS_EXT
+	INIT_WORK(&ai->event_task, NULL, NULL);
+#endif
 	rc = add_airo_dev( dev );
 	if (rc)
 		goto err_out_free;
@@ -1859,7 +1878,7 @@
 	} else {
 		ai->event_task.func = (void (*)(void *))airo_send_event;
 		ai->event_task.data = (void *)dev;
-		schedule_work(&ai->event_task);
+		schedule_delayed_work(&ai->event_task, 1);
 	}
 }
 #endif
@@ -1876,7 +1895,7 @@
 	} else {
 		ai->mic_task.func = (void (*)(void *))airo_read_mic;
 		ai->mic_task.data = (void *)ai;
-		schedule_work(&ai->mic_task);
+		schedule_delayed_work(&ai->mic_task, 1);
 	}
 }
 
@@ -4090,7 +4109,7 @@
 
 #ifdef CONFIG_PCI
 	printk( KERN_INFO "airo:  Probing for PCI adapters\n" );
-	rc = pci_module_init(&airo_driver);
+	rc = pci_register_driver(&airo_driver);
 	printk( KERN_INFO "airo:  Finished probing for PCI adapters\n" );
 #endif
 
@@ -4102,6 +4121,7 @@
 
 static void __exit airo_cleanup_module( void )
 {
+	pci_unregister_driver(&airo_driver);
 	while( airo_devices ) {
 		printk( KERN_INFO "airo: Unregistering %s\n", airo_devices->dev->name );
 		stop_airo_card( airo_devices->dev, 1 );
@@ -5160,7 +5180,7 @@
 			      & status_rid.bssid[i][2]
 			      & status_rid.bssid[i][3]
 			      & status_rid.bssid[i][4]
-			      & status_rid.bssid[i][5])!=-1 &&
+			      & status_rid.bssid[i][5])!=0xff &&
 			     (status_rid.bssid[i][0]
 			      | status_rid.bssid[i][1]
 			      | status_rid.bssid[i][2]

Re: [PATCH] airo driver: fix races, oops, etc..

[Javier Achirica]

I've integrated this patch in my code. I've done a major change: Instead
of using schedule_delayed_work(), I create a new workqueue and use
queue_work() on that queue. As all tasks sleep in the same lock, I can
queue them there and make them sleep instead of requeueing them.

I haven't sent them to Jeff yet, as I want to do more testing. If you want
to help testing them, please tell me.

Javier Achirica

On 29 Jul 2003, Benjamin Herrenschmidt wrote:

> Hi !
>
> Here's a patch against Linus current airo.c, it adds back some fixes I
> did during OLS on the previous version of this driver. I couldn't test
> this new 'fixed' version though as I don't have the airo card anymore:
>
>  - Initialize the work_struct structures used by the driver
>  - Change most of schedule_work() to schedule_delayed_work(). The
>    problem with schedule_work() is that the worker_thread will never
>    schedule() if the work keeps getting added back to the list by the
>    callback, which typically happened with this driver when the xmit
>    work gets scheduled while the semaphore was used by a pending
>    command. Note that -ac tree has a modified version of this driver
>    that gets rid of this "over-smart" work queue stuff and uses normal
>    spinlock instead, probably at the expense of some latency...
>  - Fix a small signed vs. unsigned char issue
>  - Remove bogus pci_module_init(), use pci_register_driver() instead and
>    add missing pci_unregister_driver() so the module can now be removed
>    without leaving stale references (and thus avoid an oops next time
>    the driver list is walked by the device core).
>
> Jeff, if you are ok with these, please send to Linus,
>
> Ben
>
>
> diff -urN linux-2.5/drivers/net/wireless/airo.c linuxppc-2.5-benh/drivers/net/wireless/airo.c
> --- linux-2.5/drivers/net/wireless/airo.c	2003-07-29 08:51:06.000000000 -0400
> +++ linuxppc-2.5-benh/drivers/net/wireless/airo.c	2003-07-29 08:54:26.000000000 -0400
> @@ -633,7 +633,7 @@
>  	u16 SSIDlen;
>  	char SSID[32];
>  	char apName[16];
> -	char bssid[4][ETH_ALEN];
> +	unsigned char bssid[4][ETH_ALEN];
>  	u16 beaconPeriod;
>  	u16 dimPeriod;
>  	u16 atimDuration;
> @@ -1031,7 +1031,7 @@
>  	struct work_struct promisc_task;
>  	struct {
>  		struct sk_buff *skb;
> -		int fid;
> +		int fid, hardirq;
>  		struct work_struct task;
>  	} xmit, xmit11;
>  	struct net_device *wifidev;
> @@ -1348,7 +1348,12 @@
>  		netif_stop_queue(dev);
>  		priv->xmit.task.func = (void (*)(void *))airo_do_xmit;
>  		priv->xmit.task.data = (void *)dev;
> -		schedule_work(&priv->xmit.task);
> +		if (priv->xmit.hardirq) {
> +			priv->xmit.hardirq = 0;
> +			schedule_work(&priv->xmit.task);
> +			return;
> +		}
> +		schedule_delayed_work(&priv->xmit.task, 1);
>  		return;
>  	}
>  	status = transmit_802_3_packet (priv, fids[fid], skb->data);
> @@ -1393,6 +1398,7 @@
>  		fids[i] |= (len << 16);
>  		priv->xmit.skb = skb;
>  		priv->xmit.fid = i;
> +		priv->xmit.hardirq = 1;
>  		airo_do_xmit(dev);
>  	}
>  	return 0;
> @@ -1410,7 +1416,12 @@
>  		netif_stop_queue(dev);
>  		priv->xmit11.task.func = (void (*)(void *))airo_do_xmit11;
>  		priv->xmit11.task.data = (void *)dev;
> -		schedule_work(&priv->xmit11.task);
> +		if (priv->xmit11.hardirq) {
> +			priv->xmit11.hardirq = 0;
> +			schedule_work(&priv->xmit11.task);
> +			return;
> +		}
> +		schedule_delayed_work(&priv->xmit11.task, 1);
>  		return;
>  	}
>  	status = transmit_802_11_packet (priv, fids[fid], skb->data);
> @@ -1485,7 +1496,7 @@
>  	} else {
>  		ai->stats_task.func = (void (*)(void *))airo_read_stats;
>  		ai->stats_task.data = (void *)ai;
> -		schedule_work(&ai->stats_task);
> +		schedule_delayed_work(&ai->stats_task, 1);
>  	}
>  }
>
> @@ -1508,7 +1519,7 @@
>  	} else {
>  		ai->promisc_task.func = (void (*)(void *))airo_end_promisc;
>  		ai->promisc_task.data = (void *)ai;
> -		schedule_work(&ai->promisc_task);
> +		schedule_delayed_work(&ai->promisc_task, 1);
>  	}
>  }
>
> @@ -1524,7 +1535,7 @@
>  	} else {
>  		ai->promisc_task.func = (void (*)(void *))airo_set_promisc;
>  		ai->promisc_task.data = (void *)ai;
> -		schedule_work(&ai->promisc_task);
> +		schedule_delayed_work(&ai->promisc_task, 1);
>  	}
>  }
>
> @@ -1710,6 +1721,14 @@
>  	sema_init(&ai->sem, 1);
>  	ai->need_commit = 0;
>  	ai->config.len = 0;
> +	INIT_WORK(&ai->stats_task, NULL, NULL);
> +	INIT_WORK(&ai->promisc_task, NULL, NULL);
> +	INIT_WORK(&ai->xmit.task, NULL, NULL);
> +	INIT_WORK(&ai->xmit11.task, NULL, NULL);
> +	INIT_WORK(&ai->mic_task, NULL, NULL);
> +#ifdef WIRELESS_EXT
> +	INIT_WORK(&ai->event_task, NULL, NULL);
> +#endif
>  	rc = add_airo_dev( dev );
>  	if (rc)
>  		goto err_out_free;
> @@ -1859,7 +1878,7 @@
>  	} else {
>  		ai->event_task.func = (void (*)(void *))airo_send_event;
>  		ai->event_task.data = (void *)dev;
> -		schedule_work(&ai->event_task);
> +		schedule_delayed_work(&ai->event_task, 1);
>  	}
>  }
>  #endif
> @@ -1876,7 +1895,7 @@
>  	} else {
>  		ai->mic_task.func = (void (*)(void *))airo_read_mic;
>  		ai->mic_task.data = (void *)ai;
> -		schedule_work(&ai->mic_task);
> +		schedule_delayed_work(&ai->mic_task, 1);
>  	}
>  }
>
> @@ -4090,7 +4109,7 @@
>
>  #ifdef CONFIG_PCI
>  	printk( KERN_INFO "airo:  Probing for PCI adapters\n" );
> -	rc = pci_module_init(&airo_driver);
> +	rc = pci_register_driver(&airo_driver);
>  	printk( KERN_INFO "airo:  Finished probing for PCI adapters\n" );
>  #endif
>
> @@ -4102,6 +4121,7 @@
>
>  static void __exit airo_cleanup_module( void )
>  {
> +	pci_unregister_driver(&airo_driver);
>  	while( airo_devices ) {
>  		printk( KERN_INFO "airo: Unregistering %s\n", airo_devices->dev->name );
>  		stop_airo_card( airo_devices->dev, 1 );
> @@ -5160,7 +5180,7 @@
>  			      & status_rid.bssid[i][2]
>  			      & status_rid.bssid[i][3]
>  			      & status_rid.bssid[i][4]
> -			      & status_rid.bssid[i][5])!=-1 &&
> +			      & status_rid.bssid[i][5])!=0xff &&
>  			     (status_rid.bssid[i][0]
>  			      | status_rid.bssid[i][1]
>  			      | status_rid.bssid[i][2]
>
>
>

Re: [PATCH] airo driver: fix races, oops, etc..

[Benjamin Herrenschmidt]

On Tue, 2003-08-05 at 10:53, Javier Achirica wrote:
> I've integrated this patch in my code. I've done a major change: Instead
> of using schedule_delayed_work(), I create a new workqueue and use
> queue_work() on that queue. As all tasks sleep in the same lock, I can
> queue them there and make them sleep instead of requeueing them.
> 
> I haven't sent them to Jeff yet, as I want to do more testing. If you want
> to help testing them, please tell me.

Well... creating a work queue means you create one thread per CPU, that
sucks a bit don't think ? Maybe using a single thread for the driver
with your own queuing primitives...

Ben.

Re: [PATCH] airo driver: fix races, oops, etc..

[Javier Achirica]

On 5 Aug 2003, Benjamin Herrenschmidt wrote:

> On Tue, 2003-08-05 at 10:53, Javier Achirica wrote:
> > I've integrated this patch in my code. I've done a major change: Instead
> > of using schedule_delayed_work(), I create a new workqueue and use
> > queue_work() on that queue. As all tasks sleep in the same lock, I can
> > queue them there and make them sleep instead of requeueing them.
> >
> > I haven't sent them to Jeff yet, as I want to do more testing. If you want
> > to help testing them, please tell me.
>
> Well... creating a work queue means you create one thread per CPU, that
> sucks a bit don't think ? Maybe using a single thread for the driver
> with your own queuing primitives...

You're right. In my PC, with just one CPU, it doesn't make a difference
:-)

Anyway, I took a look at the workqueue code and looks like I only need a
"create_workqueue()" that only creates one thread. This is what
create_workqueue_thread() does, but it's static. Oh, well. I can also
destroy all threads but one or, as you say, write my own code. I have to
think about it....

Javier Achirica

Re: [PATCH] airo driver: fix races, oops, etc..

[Javier Achirica]

On 5 Aug 2003, Benjamin Herrenschmidt wrote:

> On Tue, 2003-08-05 at 10:53, Javier Achirica wrote:
> > I've integrated this patch in my code. I've done a major change: Instead
> > of using schedule_delayed_work(), I create a new workqueue and use
> > queue_work() on that queue. As all tasks sleep in the same lock, I can
> > queue them there and make them sleep instead of requeueing them.
> >
> > I haven't sent them to Jeff yet, as I want to do more testing. If you want
> > to help testing them, please tell me.
>
> Well... creating a work queue means you create one thread per CPU, that
> sucks a bit don't think ? Maybe using a single thread for the driver
> with your own queuing primitives...

I've been studying the problem for a while and I've implemented a solution
using a single kernel thread and a wait queue for synchronization. I've
tested it and looks like it works fine. It can be used both in 2.4
and 2.6 kernels. Before submitting a patch with it I'd like someone with
experience in this kind of code to take a look at it just in case I'm
doing something dumb. Jeff? :-)

Javier Achirica

Re: [PATCH] airo driver: fix races, oops, etc..

[Jeff Garzik]

Javier Achirica wrote:
> I've been studying the problem for a while and I've implemented a solution
> using a single kernel thread and a wait queue for synchronization. I've
> tested it and looks like it works fine. It can be used both in 2.4
> and 2.6 kernels. Before submitting a patch with it I'd like someone with
> experience in this kind of code to take a look at it just in case I'm
> doing something dumb. Jeff? :-)


Unless the patch is huge (100K or more), post it to linux-kernel, and CC 
it to me and Benjamin Herrenschmidt.

	Jeff

Re: [PATCH] airo driver: fix races, oops, etc..

[Javier Achirica]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 739 bytes --]



On Thu, 7 Aug 2003, Jeff Garzik wrote:

> Javier Achirica wrote:
> > I've been studying the problem for a while and I've implemented a solution
> > using a single kernel thread and a wait queue for synchronization. I've
> > tested it and looks like it works fine. It can be used both in 2.4
> > and 2.6 kernels. Before submitting a patch with it I'd like someone with
> > experience in this kind of code to take a look at it just in case I'm
> > doing something dumb. Jeff? :-)
>
> Unless the patch is huge (100K or more), post it to linux-kernel, and CC
> it to me and Benjamin Herrenschmidt.

Here it goes. This version should be applied to the latest patches I sent
you in the 2.4 tree. Comments are welcome.

Thanks,
Javier Achirica

[-- Attachment #2: Type: TEXT/PLAIN, Size: 32243 bytes --]

--- linux/drivers/net/wireless/airo.c.orig	2003-08-06 22:28:40 +0200
+++ linux/drivers/net/wireless/airo.c	2003-08-06 22:30:31 +0200
@@ -32,7 +32,6 @@
 #include <linux/timer.h>
 #include <linux/interrupt.h>
 #include <linux/in.h>
-#include <linux/tqueue.h>
 #include <asm/io.h>
 #include <asm/system.h>
 #include <asm/bitops.h>
@@ -965,8 +964,6 @@
 static void enable_interrupts(struct airo_info*);
 static void disable_interrupts(struct airo_info*);
 static u16 issuecommand(struct airo_info*, Cmd *pCmd, Resp *pRsp);
-static u16 sendcommand(struct airo_info *ai, Cmd *pCmd);
-static void completecommand(struct airo_info *ai, Resp *pRsp);
 static int bap_setup(struct airo_info*, u16 rid, u16 offset, int whichbap);
 static int aux_bap_read(struct airo_info*, u16 *pu16Dst, int bytelen,
 			int whichbap);
@@ -986,6 +983,8 @@
 
 static void airo_interrupt( int irq, void* dev_id, struct pt_regs
 			    *regs);
+static int airo_thread(void *data);
+static void timer_func( struct net_device *dev );
 static int airo_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 #ifdef WIRELESS_EXT
 struct iw_statistics *airo_get_wireless_stats (struct net_device *dev);
@@ -996,8 +995,8 @@
 int flashcard(struct net_device *dev, aironet_ioctl *comp);
 #endif /* CISCO_EXT */
 #ifdef MICSUPPORT
-static void micinit(struct airo_info *ai, MICRid *micr);
-static void micsetup(struct airo_info *ai);
+static void micinit(struct airo_info *ai);
+static int micsetup(struct airo_info *ai);
 static int encapsulate(struct airo_info *ai, etherHead *pPacket, MICBuffer *buffer, int len);
 static int decapsulate(struct airo_info *ai, MICBuffer *mic, etherHead *pPacket, u16 payLen);
 #endif
@@ -1015,41 +1014,50 @@
 	int need_commit;	// Need to set config
 	char keyindex; // Used with auto wep
 	char defindex; // Used with auto wep
-	struct timer_list timer;
 	struct proc_dir_entry *proc_entry;
 	struct airo_info *next;
         spinlock_t aux_lock;
         unsigned long flags;
-#define FLAG_PROMISC   IFF_PROMISC	/* 0x100 - include/linux/if.h */
-#define FLAG_RADIO_OFF 0x02		/* User disabling of MAC */
-#define FLAG_RADIO_DOWN 0x08		/* ifup/ifdown disabling of MAC */
-#define FLAG_FLASHING  0x10
-#define FLAG_ADHOC        0x01 /* Needed by MIC */
-#define FLAG_MIC_CAPABLE  0x20
-#define FLAG_UPDATE_MULTI 0x40
-#define FLAG_UPDATE_UNI   0x80
-#define FLAG_802_11    0x200
-#define FLAG_PENDING_XMIT   0x400
-#define FLAG_PENDING_XMIT11 0x800
-#define FLAG_PCI    0x1000
+#define FLAG_PROMISC	8	/* IFF_PROMISC 0x100 - include/linux/if.h */
+#define FLAG_RADIO_OFF	0	/* User disabling of MAC */
+#define FLAG_RADIO_DOWN	1	/* ifup/ifdown disabling of MAC */
+#define FLAG_RADIO_MASK 0x03
+#define FLAG_FLASHING	2
+#define FLAG_ADHOC	3	/* Needed by MIC */
+#define FLAG_MIC_CAPABLE 4
+#define FLAG_UPDATE_MULTI 5
+#define FLAG_UPDATE_UNI 6
+#define FLAG_802_11	7
+#define FLAG_PENDING_XMIT 9
+#define FLAG_PENDING_XMIT11 10
+#define FLAG_PCI	11
+#define JOB_MASK	0xff0000
+#define JOB_DIE		16
+#define JOB_XMIT	17
+#define JOB_XMIT11	18
+#define JOB_STATS	19
+#define JOB_PROMISC	20
+#define JOB_MIC		21
+#define JOB_EVENT	22
+#define JOB_AUTOWEP	23
 	int (*bap_read)(struct airo_info*, u16 *pu16Dst, int bytelen,
 			int whichbap);
 	unsigned short *flash;
 	tdsRssiEntry *rssi;
-	struct semaphore sem;
 	struct task_struct *task;
-	struct tq_struct stats_task;
-	struct tq_struct promisc_task;
+	struct semaphore sem;
+	pid_t thr_pid;
+	wait_queue_head_t thr_wait;
+	struct completion thr_exited;
+	unsigned long expires;
 	struct {
 		struct sk_buff *skb;
 		int fid;
-		struct tq_struct task;
 	} xmit, xmit11;
 	struct net_device *wifidev;
 #ifdef WIRELESS_EXT
 	struct iw_statistics	wstats;		// wireless stats
 	unsigned long		scan_timestamp;	/* Time started to scan */
-	struct tq_struct	event_task;
 #if WIRELESS_EXT > 15
 	struct iw_spy_data	spy_data;
 #else /* WIRELESS_EXT > 15 */
@@ -1063,7 +1071,6 @@
 	/* MIC stuff */
 	mic_module		mod[2];
 	mic_statistics		micstats;
-	struct tq_struct 	mic_task;
 };
 
 static inline int bap_read(struct airo_info *ai, u16 *pu16Dst, int bytelen,
@@ -1213,9 +1220,9 @@
 	cfgr = ai->config;
 
 	if ((cfgr.opmode & 0xFF) == MODE_STA_IBSS)
-		ai->flags |= FLAG_ADHOC;
+		set_bit(FLAG_ADHOC, &ai->flags);
 	else
-		ai->flags &= ~FLAG_ADHOC;
+		clear_bit(FLAG_ADHOC, &ai->flags);
 
 	for(s = &cfgr.len; s <= &cfgr.rtsThres; s++) *s = cpu_to_le16(*s);
 
@@ -1279,7 +1286,7 @@
 	struct airo_info *info = dev->priv;
 	Resp rsp;
 
-	if (info->flags & FLAG_FLASHING)
+	if (test_bit(FLAG_FLASHING, &info->flags))
 		return -EIO;
 
 	/* Make sure the card is configured.
@@ -1293,7 +1300,7 @@
 
 	if (info->wifidev != dev) {
 		/* Power on the MAC controller (which may have been disabled) */
-		info->flags &= ~FLAG_RADIO_DOWN;
+		clear_bit(FLAG_RADIO_DOWN, &info->flags);
 		enable_interrupts(info);
 	}
 	enable_MAC(info, &rsp, 1);
@@ -1351,7 +1358,7 @@
 	}
 }
 
-static void airo_do_xmit(struct net_device *dev) {
+static void airo_end_xmit(struct net_device *dev) {
 	u16 status;
 	int i;
 	struct airo_info *priv = dev->priv;
@@ -1359,17 +1366,10 @@
 	int fid = priv->xmit.fid;
 	u32 *fids = priv->fids;
 
-	if (down_trylock(&priv->sem) != 0) {
-		priv->flags |= FLAG_PENDING_XMIT;
-		netif_stop_queue(dev);
-		priv->xmit.task.routine = (void (*)(void *))airo_do_xmit;
-		priv->xmit.task.data = (void *)dev;
-		schedule_task(&priv->xmit.task);
-		return;
-	}
+	clear_bit(JOB_XMIT, &priv->flags);
+	clear_bit(FLAG_PENDING_XMIT, &priv->flags);
 	status = transmit_802_3_packet (priv, fids[fid], skb->data);
 	up(&priv->sem);
-	priv->flags &= ~FLAG_PENDING_XMIT;
 
 	i = 0;
 	if ( status == SUCCESS ) {
@@ -1413,11 +1413,17 @@
 	fids[i] |= (len << 16);
 	priv->xmit.skb = skb;
 	priv->xmit.fid = i;
-	airo_do_xmit(dev);
+	if (down_trylock(&priv->sem) != 0) {
+		set_bit(FLAG_PENDING_XMIT, &priv->flags);
+		netif_stop_queue(dev);
+		set_bit(JOB_XMIT, &priv->flags);
+		wake_up_interruptible(&priv->thr_wait);
+	} else
+		airo_end_xmit(dev);
 	return 0;
 }
 
-static void airo_do_xmit11(struct net_device *dev) {
+static void airo_end_xmit11(struct net_device *dev) {
 	u16 status;
 	int i;
 	struct airo_info *priv = dev->priv;
@@ -1425,17 +1431,10 @@
 	int fid = priv->xmit11.fid;
 	u32 *fids = priv->fids;
 
-	if (down_trylock(&priv->sem) != 0) {
-		priv->flags |= FLAG_PENDING_XMIT11;
-		netif_stop_queue(dev);
-		priv->xmit11.task.routine = (void (*)(void *))airo_do_xmit11;
-		priv->xmit11.task.data = (void *)dev;
-		schedule_task(&priv->xmit11.task);
-		return;
-	}
+	clear_bit(JOB_XMIT11, &priv->flags);
+	clear_bit(FLAG_PENDING_XMIT11, &priv->flags);
 	status = transmit_802_11_packet (priv, fids[fid], skb->data);
 	up(&priv->sem);
-	priv->flags &= ~FLAG_PENDING_XMIT11;
 
 	i = MAX_FIDS / 2;
 	if ( status == SUCCESS ) {
@@ -1479,7 +1478,13 @@
 	fids[i] |= (len << 16);
 	priv->xmit11.skb = skb;
 	priv->xmit11.fid = i;
-	airo_do_xmit11(dev);
+	if (down_trylock(&priv->sem) != 0) {
+		set_bit(FLAG_PENDING_XMIT11, &priv->flags);
+		netif_stop_queue(dev);
+		set_bit(JOB_XMIT11, &priv->flags);
+		wake_up_interruptible(&priv->thr_wait);
+	} else
+		airo_end_xmit11(dev);
 	return 0;
 }
 
@@ -1487,29 +1492,24 @@
 	StatsRid stats_rid;
 	u32 *vals = stats_rid.vals;
 
-	if (down_trylock(&ai->sem) == 0) {
-		readStatsRid(ai, &stats_rid, RID_STATS, 0);
-		up(&ai->sem);
+	clear_bit(JOB_STATS, &ai->flags);
+	readStatsRid(ai, &stats_rid, RID_STATS, 0);
+	up(&ai->sem);
 
-		ai->stats.rx_packets = vals[43] + vals[44] + vals[45];
-		ai->stats.tx_packets = vals[39] + vals[40] + vals[41];
-		ai->stats.rx_bytes = vals[92];
-		ai->stats.tx_bytes = vals[91];
-		ai->stats.rx_errors = vals[0] + vals[2] + vals[3] + vals[4];
-		ai->stats.tx_errors = vals[42] + ai->stats.tx_fifo_errors;
-		ai->stats.multicast = vals[43];
-		ai->stats.collisions = vals[89];
-
-		/* detailed rx_errors: */
-		ai->stats.rx_length_errors = vals[3];
-		ai->stats.rx_crc_errors = vals[4];
-		ai->stats.rx_frame_errors = vals[2];
-		ai->stats.rx_fifo_errors = vals[0];
-	} else {
-		ai->stats_task.routine = (void (*)(void *))airo_read_stats;
-		ai->stats_task.data = (void *)ai;
-		schedule_task(&ai->stats_task);
-	}
+	ai->stats.rx_packets = vals[43] + vals[44] + vals[45];
+	ai->stats.tx_packets = vals[39] + vals[40] + vals[41];
+	ai->stats.rx_bytes = vals[92];
+	ai->stats.tx_bytes = vals[91];
+	ai->stats.rx_errors = vals[0] + vals[2] + vals[3] + vals[4];
+	ai->stats.tx_errors = vals[42] + ai->stats.tx_fifo_errors;
+	ai->stats.multicast = vals[43];
+	ai->stats.collisions = vals[89];
+
+	/* detailed rx_errors: */
+	ai->stats.rx_length_errors = vals[3];
+	ai->stats.rx_crc_errors = vals[4];
+	ai->stats.rx_frame_errors = vals[2];
+	ai->stats.rx_fifo_errors = vals[0];
 }
 
 struct net_device_stats *airo_get_stats(struct net_device *dev)
@@ -1517,46 +1517,37 @@
 	struct airo_info *local =  dev->priv;
 
 	/* Get stats out of the card if available */
-	airo_read_stats(local);
+	if (down_trylock(&local->sem) != 0) {
+		set_bit(JOB_STATS, &local->flags);
+		wake_up_interruptible(&local->thr_wait);
+	} else
+		airo_read_stats(local);
 
 	return &local->stats;
 }
 
-static void airo_end_promisc(struct airo_info *ai) {
-	Resp rsp;
-
-	if ((IN4500(ai, EVSTAT) & EV_CMD) != 0) {
-		completecommand(ai, &rsp);
-		up(&ai->sem);
-	} else {
-		ai->promisc_task.routine = (void (*)(void *))airo_end_promisc;
-		ai->promisc_task.data = (void *)ai;
-		schedule_task(&ai->promisc_task);
-	}
-}
-
 static void airo_set_promisc(struct airo_info *ai) {
 	Cmd cmd;
+	Resp rsp;
 
-	if (down_trylock(&ai->sem) == 0) {
-		memset(&cmd, 0, sizeof(cmd));
-		cmd.cmd=CMD_SETMODE;
-		cmd.parm0=(ai->flags&IFF_PROMISC) ? PROMISC : NOPROMISC;
-		sendcommand(ai, &cmd);
-		airo_end_promisc(ai);
-	} else {
-		ai->promisc_task.routine = (void (*)(void *))airo_set_promisc;
-		ai->promisc_task.data = (void *)ai;
-		schedule_task(&ai->promisc_task);
-	}
+	memset(&cmd, 0, sizeof(cmd));
+	cmd.cmd=CMD_SETMODE;
+	clear_bit(JOB_PROMISC, &ai->flags);
+	cmd.parm0=(ai->flags&IFF_PROMISC) ? PROMISC : NOPROMISC;
+	issuecommand(ai, &cmd, &rsp);
+	up(&ai->sem);
 }
 
 static void airo_set_multicast_list(struct net_device *dev) {
 	struct airo_info *ai = dev->priv;
 
 	if ((dev->flags ^ ai->flags) & IFF_PROMISC) {
-		ai->flags ^= IFF_PROMISC;
-		airo_set_promisc(ai);
+		change_bit(FLAG_PROMISC, &ai->flags);
+		if (down_trylock(&ai->sem) != 0) {
+			set_bit(JOB_PROMISC, &ai->flags);
+			wake_up_interruptible(&ai->thr_wait);
+		} else
+			airo_set_promisc(ai);
 	}
 
 	if ((dev->flags&IFF_ALLMULTI)||dev->mc_count>0) {
@@ -1602,7 +1593,7 @@
 		 * That's the method that is most friendly towards the network
 		 * stack (i.e. the network stack won't try to broadcast
 		 * anything on the interface and routes are gone. Jean II */
-		ai->flags |= FLAG_RADIO_DOWN;
+		set_bit(FLAG_RADIO_DOWN, &ai->flags);
 		disable_MAC(ai, 1);
 #endif
 		disable_interrupts( ai );
@@ -1617,8 +1608,6 @@
 	struct airo_info *ai = dev->priv;
 	disable_interrupts(ai);
 	free_irq( dev->irq, dev );
-	if (auto_wep)
-		del_timer_sync(&ai->timer);
 	takedown_proc_entry( dev, ai );
 	if (ai->registered) {
 		unregister_netdev( dev );
@@ -1629,7 +1618,9 @@
 		}
 		ai->registered = 0;
 	}
-	flush_scheduled_tasks();
+	set_bit(JOB_DIE, &ai->flags);
+	kill_proc(ai->thr_pid, SIGTERM, 1);
+	wait_for_completion(&ai->thr_exited);
 	if (ai->flash)
 		kfree(ai->flash);
 	if (ai->rssi)
@@ -1734,9 +1725,14 @@
 	sema_init(&ai->sem, 1);
 	ai->need_commit = 0;
 	ai->config.len = 0;
+	init_waitqueue_head (&ai->thr_wait);
+	init_completion (&ai->thr_exited);
+	ai->thr_pid = kernel_thread(airo_thread, dev, CLONE_FS | CLONE_FILES);
+	if (ai->thr_pid < 0)
+		goto err_out_free;
 	rc = add_airo_dev( dev );
 	if (rc)
-		goto err_out_free;
+		goto err_out_thr;
 
 	/* The Airo-specific entries in the device structure. */
 	dev->hard_start_xmit = &airo_start_xmit;
@@ -1776,7 +1772,7 @@
 		}
 	} else {
 		ai->bap_read = fast_bap_read;
-		ai->flags |= FLAG_FLASHING;
+		set_bit(FLAG_FLASHING, &ai->flags);
 	}
 
 	rc = register_netdev(dev);
@@ -1807,6 +1803,10 @@
 	free_irq(dev->irq, dev);
 err_out_unlink:
 	del_airo_dev(dev);
+err_out_thr:
+	set_bit(JOB_DIE, &ai->flags);
+	kill_proc(ai->thr_pid, SIGTERM, 1);
+	wait_for_completion(&ai->thr_exited);
 err_out_free:
 	kfree(dev);
 	return NULL;
@@ -1870,38 +1870,103 @@
 	union iwreq_data wrqu;
 	StatusRid status_rid;
 
-	if (down_trylock(&ai->sem) == 0) {
-		PC4500_readrid(ai, RID_STATUS, &status_rid, sizeof(status_rid), 0);
-		up(&ai->sem);
-		wrqu.data.length = 0;
-		wrqu.data.flags = 0;
-		memcpy(wrqu.ap_addr.sa_data, status_rid.bssid[0], ETH_ALEN);
-		wrqu.ap_addr.sa_family = ARPHRD_ETHER;
+	clear_bit(JOB_EVENT, &ai->flags);
+	PC4500_readrid(ai, RID_STATUS, &status_rid, sizeof(status_rid), 0);
+	up(&ai->sem);
+	wrqu.data.length = 0;
+	wrqu.data.flags = 0;
+	memcpy(wrqu.ap_addr.sa_data, status_rid.bssid[0], ETH_ALEN);
+	wrqu.ap_addr.sa_family = ARPHRD_ETHER;
 
-		/* Send event to user space */
-		wireless_send_event(dev, SIOCGIWAP, &wrqu, NULL);
-	} else {
-		ai->event_task.routine = (void (*)(void *))airo_send_event;
-		ai->event_task.data = (void *)dev;
-		schedule_task(&ai->event_task);
-	}
+	/* Send event to user space */
+	wireless_send_event(dev, SIOCGIWAP, &wrqu, NULL);
 }
 #endif
 
-static void airo_read_mic(struct airo_info *ai) {
-	MICRid mic_rid;
+static int airo_thread(void *data) {
+	struct net_device *dev = data;
+	struct airo_info *ai = dev->priv;
+	int locked;
+	
+	daemonize();
+	reparent_to_init();
+	sigemptyset(&current->blocked);
+	recalc_sigpending(current);
+
+	strncpy (current->comm, dev->name, sizeof(current->comm) - 1);
+	current->comm[sizeof(current->comm) - 1] = '\0';
+
+	while(1) {
+		if (signal_pending(current))
+			flush_signals(current);
 
-	if (down_trylock(&ai->sem) == 0) {
-		PC4500_readrid(ai, RID_MIC, &mic_rid, sizeof(mic_rid), 0);
-		up(&ai->sem);
+		if (test_bit(JOB_DIE, &ai->flags))
+			break;
+
+		if (ai->flags & JOB_MASK) {
+			locked = down_interruptible(&ai->sem);
+		} else {
+			wait_queue_t wait;
+
+			init_waitqueue_entry(&wait, current);
+			add_wait_queue(&ai->thr_wait, &wait);
+			for (;;) {
+				set_current_state(TASK_INTERRUPTIBLE);
+				if (ai->flags & JOB_MASK)
+					break;
+				if (ai->expires) {
+					if (time_after_eq(jiffies,ai->expires)){
+						set_bit(JOB_AUTOWEP,&ai->flags);
+						break;
+					}
+					if (!signal_pending(current)) {
+						schedule_timeout(ai->expires - jiffies);
+						continue;
+					}
+				} else if (!signal_pending(current)) {
+					schedule();
+					continue;
+				}
+				break;
+			}
+			current->state = TASK_RUNNING;
+			remove_wait_queue(&ai->thr_wait, &wait);
+			locked = 1;
+		}
+
+		if (locked)
+			continue;
+
+		if (test_bit(JOB_DIE, &ai->flags)) {
+			up(&ai->sem);
+			break;
+		}
+
+		if (test_bit(FLAG_FLASHING, &ai->flags)) {
+			up(&ai->sem);
+			continue;
+		}
+
+		if (test_bit(JOB_XMIT, &ai->flags))
+			airo_end_xmit(dev);
+		else if (test_bit(JOB_XMIT11, &ai->flags))
+			airo_end_xmit11(dev);
+		else if (test_bit(JOB_STATS, &ai->flags))
+			airo_read_stats(ai);
+		else if (test_bit(JOB_PROMISC, &ai->flags))
+			airo_set_promisc(ai);
 #ifdef MICSUPPORT
-		micinit (ai, &mic_rid);
+		else if (test_bit(JOB_MIC, &ai->flags))
+			micinit(ai);
 #endif
-	} else {
-		ai->mic_task.routine = (void (*)(void *))airo_read_mic;
-		ai->mic_task.data = (void *)ai;
-		schedule_task(&ai->mic_task);
+#if WIRELESS_EXT > 13
+		else if (test_bit(JOB_EVENT, &ai->flags))
+			airo_send_event(dev);
+#endif
+		else if (test_bit(JOB_AUTOWEP, &ai->flags))
+			timer_func(dev);
 	}
+	complete_and_exit (&ai->thr_exited, 0);
 }
 
 static void airo_interrupt ( int irq, void* dev_id, struct pt_regs *regs) {
@@ -1930,8 +1995,15 @@
 
 		if ( status & EV_MIC ) {
 			OUT4500( apriv, EVACK, EV_MIC );
-			if (apriv->flags & FLAG_MIC_CAPABLE)
-				airo_read_mic( apriv );
+#ifdef MICSUPPORT
+			if (test_bit(FLAG_MIC_CAPABLE, &apriv->flags)) {
+				if (down_trylock(&apriv->sem) != 0) {
+					set_bit(JOB_MIC, &apriv->flags);
+					wake_up_interruptible(&apriv->thr_wait);
+				} else
+					micinit (apriv);
+			}
+#endif
 		}
 		if ( status & EV_LINK ) {
 #if WIRELESS_EXT > 13
@@ -1973,15 +2045,18 @@
 #define RC_NOAUTH 9 /* Station requesting (Re)Association is not
 		       Authenticated with the responding station */
 			if (newStatus != ASSOCIATED) {
-				if (auto_wep && !timer_pending(&apriv->timer)) {
-					apriv->timer.expires = RUN_AT(HZ*3);
-		      			add_timer(&apriv->timer);
+				if (auto_wep && !apriv->expires) {
+					apriv->expires = RUN_AT(3*HZ);
+					wake_up_interruptible(&apriv->thr_wait);
 				}
 			} else {
 				struct task_struct *task = apriv->task;
+				if (auto_wep)
+					apriv->expires = 0;
 				if (task)
 					wake_up_process (task);
-				apriv->flags|=FLAG_UPDATE_UNI|FLAG_UPDATE_MULTI;
+				set_bit(FLAG_UPDATE_UNI, &apriv->flags);
+				set_bit(FLAG_UPDATE_MULTI, &apriv->flags);
 			}
 #if WIRELESS_EXT > 13
 			/* Question : is ASSOCIATED the only status
@@ -2002,7 +2077,11 @@
 					wireless_send_event(dev, SIOCGIWSCAN, &wrqu, NULL);
 					apriv->scan_timestamp = 0;
 				}
-				airo_send_event(dev);
+				if (down_trylock(&apriv->sem) != 0) {
+					set_bit(JOB_EVENT, &apriv->flags);
+					wake_up_interruptible(&apriv->thr_wait);
+				} else
+					airo_send_event(dev);
 			} else {
 				memset(wrqu.ap_addr.sa_data, '\0', ETH_ALEN);
 				wrqu.ap_addr.sa_family = ARPHRD_ETHER;
@@ -2033,7 +2112,7 @@
 			fid = IN4500( apriv, RXFID );
 
 			/* Get the packet length */
-			if (apriv->flags & FLAG_802_11) {
+			if (test_bit(FLAG_802_11, &apriv->flags)) {
 				bap_setup (apriv, fid, 4, BAP0);
 				bap_read (apriv, (u16*)&hdr, sizeof(hdr), BAP0);
 				/* Bad CRC. Ignore packet */
@@ -2052,7 +2131,7 @@
 				len = 0;
 			}
 			if (len) {
-				if (apriv->flags & FLAG_802_11) {
+				if (test_bit(FLAG_802_11, &apriv->flags)) {
 					bap_read (apriv, (u16*)&fc, sizeof(fc), BAP0);
 					fc = le16_to_cpu(fc);
 					switch (fc & 0xc) {
@@ -2081,7 +2160,7 @@
 			}
 			if (len) {
 				buffer = (u16*)skb_put (skb, len + hdrlen);
-				if (apriv->flags & FLAG_802_11) {
+				if (test_bit(FLAG_802_11, &apriv->flags)) {
 					buffer[0] = fc;
 					bap_read (apriv, buffer + 1, hdrlen - 2, BAP0);
 					if (hdrlen == 24)
@@ -2128,11 +2207,12 @@
 					char *sa;
 					struct iw_quality wstats;
 					/* Prepare spy data : addr + qual */
-					sa = (char*)buffer + ((apriv->flags & FLAG_802_11) ? 10 : 6);
-					if (!(apriv->flags & FLAG_802_11)) {
+					if (!test_bit(FLAG_802_11, &apriv->flags)) {
+						sa = (char*)buffer + 6;
 						bap_setup (apriv, fid, 8, BAP0);
 						bap_read (apriv, (u16*)hdr.rssi, 2, BAP0);
-					}
+					} else
+						sa = (char*)buffer + 10;
 					wstats.qual = hdr.rssi[0];
 					if (apriv->rssi)
 						wstats.level = 0x100 - apriv->rssi[hdr.rssi[1]].rssidBm;
@@ -2149,12 +2229,12 @@
 					int i;
 					char *sa;
 
-					sa = (char*)buffer + ((apriv->flags & FLAG_802_11) ? 10 : 6);
+					sa = (char*)buffer + (test_bit(FLAG_802_11, &apriv->flags) ? 10 : 6);
 
 					for (i=0; i<apriv->spy_number; i++)
 						if (!memcmp(sa,apriv->spy_address[i],ETH_ALEN))
 						{
-							if (!(apriv->flags & FLAG_802_11)) {
+							if (!test_bit(FLAG_802_11, &apriv->flags)) {
 								bap_setup (apriv, fid, 8, BAP0);
 								bap_read (apriv, (u16*)hdr.rssi, 2, BAP0);
 							}
@@ -2172,7 +2252,7 @@
 #endif /* WIRELESS_EXT > 15 */
 				OUT4500( apriv, EVACK, EV_RX);
 
-				if (apriv->flags & FLAG_802_11) {
+				if (test_bit(FLAG_802_11, &apriv->flags)) {
 					skb->mac.raw = skb->data;
 					skb->pkt_type = PACKET_OTHERHOST;
 					skb->dev = apriv->wifidev;
@@ -2210,10 +2290,10 @@
 				/* Set up to be used again */
 				apriv->fids[index] &= 0xffff;
 				if (index < MAX_FIDS / 2) {
-					if (!(apriv->flags & FLAG_PENDING_XMIT))
+					if (!test_bit(FLAG_PENDING_XMIT, &apriv->flags))
 						netif_wake_queue(dev);
 				} else {
-					if (!(apriv->flags & FLAG_PENDING_XMIT11))
+					if (!test_bit(FLAG_PENDING_XMIT11, &apriv->flags))
 						netif_wake_queue(apriv->wifidev);
 				}
 			} else {
@@ -2273,7 +2353,7 @@
 	 * instead of this flag, but I don't trust it *within* the
 	 * open/close functions, and testing both flags together is
 	 * "cheaper" - Jean II */
-	if (ai->flags & (FLAG_RADIO_OFF|FLAG_RADIO_DOWN)) return SUCCESS;
+	if (ai->flags & FLAG_RADIO_MASK) return SUCCESS;
 	memset(&cmd, 0, sizeof(cmd));
 	cmd.cmd = MAC_ENABLE;
 	if (!lock)
@@ -2394,10 +2474,10 @@
 		ai->config.opmode = adhoc ? MODE_STA_IBSS : MODE_STA_ESS;
 
 #ifdef MICSUPPORT
-		if ((cap_rid.len>=sizeof(cap_rid)) && (cap_rid.extSoftCap&1)) {
+		if ((cap_rid.len>=sizeof(cap_rid)) && (cap_rid.extSoftCap&1) &&
+		    (micsetup(ai) == SUCCESS)) {
 			ai->config.opmode |= MODE_MIC;
-			ai->flags |= FLAG_MIC_CAPABLE;
-			micsetup(ai);
+			set_bit(FLAG_MIC_CAPABLE, &ai->flags);
 		}
 #endif
 
@@ -2463,34 +2543,15 @@
 		rc = readWepKeyRid(ai, &wkr, 0);
 	} while(lastindex != wkr.kindex);
 
-	if (auto_wep && !timer_pending(&ai->timer)) {
-		ai->timer.expires = RUN_AT(HZ*3);
-		add_timer(&ai->timer);
+	if (auto_wep) {
+		ai->expires = RUN_AT(3*HZ);
+		wake_up_interruptible(&ai->thr_wait);
 	}
-	return SUCCESS;
-}
-
-static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp) {
-        // Im really paranoid about letting it run forever!
-	int max_tries = 600000;
-
-	if (sendcommand(ai, pCmd) == (u16)ERROR)
-		return ERROR;
 
-	while (max_tries-- && (IN4500(ai, EVSTAT) & EV_CMD) == 0) {
-		if (!in_interrupt() && (max_tries & 255) == 0)
-			schedule();
-	}
-	if ( max_tries == -1 ) {
-		printk( KERN_ERR
-			"airo: Max tries exceeded waiting for command\n" );
-                return ERROR;
-	}
-	completecommand(ai, pRsp);
 	return SUCCESS;
 }
 
-static u16 sendcommand(struct airo_info *ai, Cmd *pCmd) {
+static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp) {
         // Im really paranoid about letting it run forever!
 	int max_tries = 600000;
 	u16 cmd;
@@ -2509,10 +2570,16 @@
 			"airo: Max tries exceeded when issueing command\n" );
                 return ERROR;
 	}
-	return SUCCESS;
-}
 
-static void completecommand(struct airo_info *ai, Resp *pRsp) {
+	while (max_tries-- && (IN4500(ai, EVSTAT) & EV_CMD) == 0) {
+		if (!in_interrupt() && (max_tries & 255) == 0)
+			schedule();
+	}
+	if ( max_tries == -1 ) {
+		printk( KERN_ERR
+			"airo: Max tries exceeded waiting for command\n" );
+                return ERROR;
+	}
 	// command completed
 	pRsp->status = IN4500(ai, STATUS);
 	pRsp->rsp0 = IN4500(ai, RESP0);
@@ -2525,6 +2592,8 @@
 	}
 	// acknowledge processing the status/response
 	OUT4500(ai, EVACK, EV_CMD);
+
+	return SUCCESS;
 }
 
 /* Sets up the bap to start exchange data.  whichbap should
@@ -2811,7 +2880,7 @@
 	len -= ETH_ALEN * 2;
 
 #ifdef MICSUPPORT
-	if ((ai->flags & FLAG_MIC_CAPABLE) && ai->micstats.enabled && 
+	if (test_bit(FLAG_MIC_CAPABLE, &ai->flags) && ai->micstats.enabled && 
 	    (ntohs(((u16 *)pPacket)[6]) != 0x888E)) {
 		if (encapsulate(ai,(etherHead *)pPacket,&pMic,len) != SUCCESS)
 			return ERROR;
@@ -3314,7 +3383,7 @@
 			if ((ai->config.rmode & 0xff) >= RXMODE_RFMON)
 					ai->need_commit = 2;
 			ai->config.rmode &= 0xfe00;
-			ai->flags &= ~FLAG_802_11;
+			clear_bit (FLAG_802_11, &ai->flags);
 			ai->config.opmode &= 0xFF00;
 			ai->config.scanMode = SCANMODE_ACTIVE;
 			if ( line[0] == 'a' ) {
@@ -3324,11 +3393,11 @@
 				if ( line[0] == 'r' ) {
 					ai->config.rmode |= RXMODE_RFMON | RXMODE_DISABLE_802_3_HEADER;
 					ai->config.scanMode = SCANMODE_PASSIVE;
-					ai->flags |= FLAG_802_11;
+					set_bit (FLAG_802_11, &ai->flags);
 				} else if ( line[0] == 'y' ) {
 					ai->config.rmode |= RXMODE_RFMON_ANYBSS | RXMODE_DISABLE_802_3_HEADER;
 					ai->config.scanMode = SCANMODE_PASSIVE;
-					ai->flags |= FLAG_802_11;
+					set_bit (FLAG_802_11, &ai->flags);
 				} else if ( line[0] == 'l' )
 					ai->config.rmode |= RXMODE_LANMON;
 			}
@@ -3339,9 +3408,9 @@
 		else if (!strncmp(line,"Radio: ", 7)) {
 			line += 7;
 			if (!strncmp(line,"off",3)) {
-				ai->flags |= FLAG_RADIO_OFF;
+				set_bit (FLAG_RADIO_OFF, &ai->flags);
 			} else {
-				ai->flags &= ~FLAG_RADIO_OFF;
+				clear_bit (FLAG_RADIO_OFF, &ai->flags);
 			}
 		}
 /*** NodeName processing */
@@ -3545,7 +3614,7 @@
 		     (ai->config.opmode & 0xFF) == 1 ? get_rmode(ai->config.rmode):
 		     (ai->config.opmode & 0xFF) == 2 ? "AP" :
 		     (ai->config.opmode & 0xFF) == 3 ? "AP RPTR" : "Error",
-		     ai->flags&FLAG_RADIO_OFF ? "off" : "on",
+		     test_bit(FLAG_RADIO_OFF, &ai->flags) ? "off" : "on",
 		     ai->config.nodeName,
 		     ai->config.powerSaveMode == 0 ? "CAM" :
 		     ai->config.powerSaveMode == 1 ? "PSP" :
@@ -4012,23 +4081,14 @@
    will switch WEP modes to see if that will help.  If the card is
    associated we will check every minute to see if anything has
    changed. */
-static void timer_func( u_long data ) {
-	struct net_device *dev = (struct net_device*)data;
+static void timer_func( struct net_device *dev ) {
 	struct airo_info *apriv = dev->priv;
-	u16 linkstat = IN4500(apriv, LINKSTAT);
 	Resp rsp;
 
-	if (!(apriv->flags & FLAG_FLASHING) && (linkstat != 0x400)) {
 /* We don't have a link so try changing the authtype */
-		if (down_trylock(&apriv->sem) != 0) {
-			apriv->timer.expires = RUN_AT(1);
-			add_timer(&apriv->timer);
-			return;
-		}
-
-		readConfigRid(apriv, 0);
-		disable_MAC(apriv, 0);
-		switch(apriv->config.authType) {
+	readConfigRid(apriv, 0);
+	disable_MAC(apriv, 0);
+	switch(apriv->config.authType) {
 		case AUTH_ENCRYPT:
 /* So drop to OPEN */
 			apriv->config.authType = AUTH_OPEN;
@@ -4047,16 +4107,15 @@
 			break;
 		default:  /* We'll escalate to SHAREDKEY */
 			apriv->config.authType = AUTH_SHAREDKEY;
-		}
-		apriv->need_commit = 1;
-		writeConfigRid(apriv, 0);
-		enable_MAC(apriv, &rsp, 0);
-		up(&apriv->sem);
+	}
+	apriv->need_commit = 1;
+	writeConfigRid(apriv, 0);
+	enable_MAC(apriv, &rsp, 0);
+	up(&apriv->sem);
 
 /* Schedule check to see if the change worked */
-		apriv->timer.expires = RUN_AT(HZ*3);
-		add_timer(&apriv->timer);
-	}
+	clear_bit(JOB_AUTOWEP, &apriv->flags);
+	apriv->expires = RUN_AT(HZ*3);
 }
 
 static int add_airo_dev( struct net_device *dev ) {
@@ -4064,15 +4123,6 @@
 	if ( !node )
 		return -ENOMEM;
 
-	if ( auto_wep ) {
-		struct airo_info *apriv=dev->priv;
-		struct timer_list *timer = &apriv->timer;
-
-		timer->function = timer_func;
-		timer->data = (u_long)dev;
-		init_timer(timer);
-	}
-
 	node->dev = dev;
 	node->next = airo_devices;
 	airo_devices = node;
@@ -4103,7 +4153,7 @@
 		return -ENODEV;
 
 	pci_set_drvdata(pdev, dev);
-	((struct airo_info *)dev->priv)->flags |= FLAG_PCI;
+	clear_bit (FLAG_PCI, &((struct airo_info *)dev->priv)->flags);
 	return 0;
 }
 
@@ -4149,7 +4199,7 @@
 	while( airo_devices ) {
 		printk( KERN_INFO "airo: Unregistering %s\n", airo_devices->dev->name );
 #ifdef CONFIG_PCI
-		if (((struct airo_info *)airo_devices->dev->priv)->flags & FLAG_PCI)
+		if (test_bit(FLAG_PCI, &((struct airo_info *)airo_devices->dev->priv)->flags))
 			is_pci = 1;
 #endif
 		stop_airo_card( airo_devices->dev, 1 );
@@ -4623,28 +4673,28 @@
 			local->config.opmode |= MODE_STA_IBSS;
 			local->config.rmode &= 0xfe00;
 			local->config.scanMode = SCANMODE_ACTIVE;
-			local->flags &= ~FLAG_802_11;
+			clear_bit (FLAG_802_11, &local->flags);
 			break;
 		case IW_MODE_INFRA:
 			local->config.opmode &= 0xFF00;
 			local->config.opmode |= MODE_STA_ESS;
 			local->config.rmode &= 0xfe00;
 			local->config.scanMode = SCANMODE_ACTIVE;
-			local->flags &= ~FLAG_802_11;
+			clear_bit (FLAG_802_11, &local->flags);
 			break;
 		case IW_MODE_MASTER:
 			local->config.opmode &= 0xFF00;
 			local->config.opmode |= MODE_AP;
 			local->config.rmode &= 0xfe00;
 			local->config.scanMode = SCANMODE_ACTIVE;
-			local->flags &= ~FLAG_802_11;
+			clear_bit (FLAG_802_11, &local->flags);
 			break;
 		case IW_MODE_REPEAT:
 			local->config.opmode &= 0xFF00;
 			local->config.opmode |= MODE_AP_RPTR;
 			local->config.rmode &= 0xfe00;
 			local->config.scanMode = SCANMODE_ACTIVE;
-			local->flags &= ~FLAG_802_11;
+			clear_bit (FLAG_802_11, &local->flags);
 			break;
 		case IW_MODE_MONITOR:
 			local->config.opmode &= 0xFF00;
@@ -4652,7 +4702,7 @@
 			local->config.rmode &= 0xfe00;
 			local->config.rmode |= RXMODE_RFMON | RXMODE_DISABLE_802_3_HEADER;
 			local->config.scanMode = SCANMODE_PASSIVE;
-			local->flags |= FLAG_802_11;
+			set_bit (FLAG_802_11, &local->flags);
 			break;
 		default:
 			return -EINVAL;
@@ -4841,14 +4891,14 @@
 	readCapabilityRid(local, &cap_rid);
 
 	if (vwrq->disabled) {
-		local->flags |= FLAG_RADIO_OFF;
+		set_bit (FLAG_RADIO_OFF, &local->flags);
 		local->need_commit = 1;
 		return -EINPROGRESS;		/* Call commit handler */
 	}
 	if (vwrq->flags != IW_TXPOW_MWATT) {
 		return -EINVAL;
 	}
-	local->flags &= ~FLAG_RADIO_OFF;
+	clear_bit (FLAG_RADIO_OFF, &local->flags);
 	for (i = 0; cap_rid.txPowerLevels[i] && (i < 8); i++)
 		if ((vwrq->value==cap_rid.txPowerLevels[i])) {
 			local->config.txPower = vwrq->value;
@@ -4872,7 +4922,7 @@
 
 	vwrq->value = local->config.txPower;
 	vwrq->fixed = 1;	/* No power control */
-	vwrq->disabled = (local->flags & FLAG_RADIO_OFF);
+	vwrq->disabled = test_bit(FLAG_RADIO_OFF, &local->flags);
 	vwrq->flags = IW_TXPOW_MWATT;
 
 	return 0;
@@ -5522,10 +5572,14 @@
 		writeSsidRid(local, &SSID_rid);
 		writeAPListRid(local, &APList_rid);
 	}
-	writeConfigRid(local, 1);
-	enable_MAC(local, &rsp, 1);
+	if (down_interruptible(&local->sem))
+		return -ERESTARTSYS;
+	writeConfigRid(local, 0);
+	enable_MAC(local, &rsp, 0);
 	if (local->need_commit > 1)
 		airo_set_promisc(local);
+	else
+		up(&local->sem);
 
 	return 0;
 }
@@ -6063,7 +6117,7 @@
 	unsigned char *iobuf;
 	struct airo_info *ai = dev->priv;
 
-	if (ai->flags & FLAG_FLASHING)
+	if (test_bit(FLAG_FLASHING, &ai->flags))
 		return -EIO;
 
 	switch(comp->command)
@@ -6131,7 +6185,7 @@
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;
 
-	if (ai->flags & FLAG_FLASHING)
+	if (test_bit(FLAG_FLASHING, &ai->flags))
 		return -EIO;
 
 	ridcode = 0;
@@ -6205,13 +6259,13 @@
 	if (comp->command == AIROPCFG) {
 		ConfigRid *cfg = (ConfigRid *)iobuf;
 
-		if (ai->flags & FLAG_MIC_CAPABLE)
+		if (test_bit(FLAG_MIC_CAPABLE, &ai->flags))
 			cfg->opmode |= MODE_MIC;
 
 		if ((cfg->opmode & 0xFF) == MODE_STA_IBSS)
-			ai->flags |= FLAG_ADHOC;
+			set_bit (FLAG_ADHOC, &ai->flags);
 		else
-			ai->flags &= ~FLAG_ADHOC;
+			clear_bit (FLAG_ADHOC, &ai->flags);
 	}
 
 	if((*writer)(ai, ridcode, iobuf,comp->len,1)) {
@@ -6322,7 +6376,7 @@
  */
 
 int setflashmode (struct airo_info *ai) {
-	ai->flags |= FLAG_FLASHING;
+	set_bit (FLAG_FLASHING, &ai->flags);
 
 	OUT4500(ai, SWS0, FLASH_COMMAND);
 	OUT4500(ai, SWS1, FLASH_COMMAND);
@@ -6338,7 +6392,7 @@
 	schedule_timeout (HZ/2); /* 500ms delay */
 
 	if(!waitbusy(ai)) {
-		ai->flags &= ~FLAG_FLASHING;
+		clear_bit (FLAG_FLASHING, &ai->flags);
 		printk(KERN_INFO "Waitbusy hang after setflash mode\n");
 		return -EIO;
 	}
@@ -6444,7 +6498,7 @@
 
 	set_current_state (TASK_UNINTERRUPTIBLE);
 	schedule_timeout (HZ);          /* Added 12/7/00 */
-	ai->flags &= ~FLAG_FLASHING;
+	clear_bit (FLAG_FLASHING, &ai->flags);
 	status = setup_card(ai, dev->dev_addr);
 
 	for( i = 0; i < MAX_FIDS; i++ ) {

Re: [PATCH] airo driver: fix races, oops, etc..

[Jean Tourrilhes]

Benjamin Herrenschmidt wrote :
> 
> Here's a patch against Linus current airo.c, it adds back some fixes I
> did during OLS on the previous version of this driver. I couldn't test
> this new 'fixed' version though as I don't have the airo card anymore:
> 
>  - Initialize the work_struct structures used by the driver
>  - Change most of schedule_work() to schedule_delayed_work(). The
>    problem with schedule_work() is that the worker_thread will never
>    schedule() if the work keeps getting added back to the list by the
>    callback, which typically happened with this driver when the xmit
>    work gets scheduled while the semaphore was used by a pending
>    command. Note that -ac tree has a modified version of this driver
>    that gets rid of this "over-smart" work queue stuff and uses normal
>    spinlock instead, probably at the expense of some latency...
>  - Fix a small signed vs. unsigned char issue
>  - Remove bogus pci_module_init(), use pci_register_driver() instead and
>    add missing pci_unregister_driver() so the module can now be removed
>    without leaving stale references (and thus avoid an oops next time
>    the driver list is walked by the device core).
> 
> Jeff, if you are ok with these, please send to Linus,

	Ben,

	Would you mind sending your patch to Javier (who is the
current maintainer). Javier did some work lately to fix some of those
problems, and I think your patch collides with it.
	Thanks...

	Jean