On Thu, Jul 13, 2006 at 01:53:19PM -0700, Sukadev Bhattiprolu wrote:
> Andrew,
>
> Javier Achirica, one of the major contributors to drivers/net/wireless/airo.c
> took a look at this patch, and doesn't have any problems with it. It doesn't
> fix any bugs and is just a cleanup, so it certainly isn't a candidate
> for this mainline cycle
I'm not sure it's that easy. I think it needs some more love:
- switch to wake_uo_process
- kill JOB_DIE
- cleanup a the convoluted mess in airo_thread a bit
Note that it's still reimplementing the single threaded workqueue
functionality quite badly. So if someone could switch it over and while
we're at it try to kill the idiociy of doing the trylock in the calling
context and only then calling the thread by always calling the thread
(which also solves the synchronization problem).
Anywhy, here's a small incremental patch ontop of yours to implement my
above items:
Index: linux-2.6/drivers/net/wireless/airo.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/airo.c 2006-07-13 23:23:28.000000000 +0200
+++ linux-2.6/drivers/net/wireless/airo.c 2006-07-13 23:25:38.000000000 +0200
@@ -1173,7 +1173,6 @@
#define FLAG_FLASHING 15
#define FLAG_WPA_CAPABLE 16
unsigned long flags;
-#define JOB_DIE 0
#define JOB_XMIT 1
#define JOB_XMIT11 2
#define JOB_STATS 3
@@ -1191,7 +1190,6 @@
struct task_struct *list_bss_task;
struct task_struct *airo_thread_task;
struct semaphore sem;
- wait_queue_head_t thr_wait;
unsigned long expires;
struct {
struct sk_buff *skb;
@@ -2182,7 +2180,7 @@
set_bit(FLAG_PENDING_XMIT, &priv->flags);
netif_stop_queue(dev);
set_bit(JOB_XMIT, &priv->jobs);
- wake_up_interruptible(&priv->thr_wait);
+ wake_up_process(priv->airo_thread_task);
} else
airo_end_xmit(dev);
return 0;
@@ -2253,7 +2251,7 @@
set_bit(FLAG_PENDING_XMIT11, &priv->flags);
netif_stop_queue(dev);
set_bit(JOB_XMIT11, &priv->jobs);
- wake_up_interruptible(&priv->thr_wait);
+ wake_up_process(priv->airo_thread_task);
} else
airo_end_xmit11(dev);
return 0;
@@ -2295,7 +2293,7 @@
/* Get stats out of the card if available */
if (down_trylock(&local->sem) != 0) {
set_bit(JOB_STATS, &local->jobs);
- wake_up_interruptible(&local->thr_wait);
+ wake_up_process(local->airo_thread_task);
} else
airo_read_stats(local);
}
@@ -2322,7 +2320,7 @@
change_bit(FLAG_PROMISC, &ai->flags);
if (down_trylock(&ai->sem) != 0) {
set_bit(JOB_PROMISC, &ai->jobs);
- wake_up_interruptible(&ai->thr_wait);
+ wake_up_process(ai->airo_thread_task);
} else
airo_set_promisc(ai);
}
@@ -2399,7 +2397,6 @@
}
clear_bit(FLAG_REGISTERED, &ai->flags);
}
- set_bit(JOB_DIE, &ai->jobs);
kthread_stop(ai->airo_thread_task);
/*
@@ -2809,7 +2806,6 @@
sema_init(&ai->sem, 1);
ai->config.len = 0;
ai->pci = pci;
- init_waitqueue_head (&ai->thr_wait);
ai->airo_thread_task = kthread_run(airo_thread, dev, dev->name);
if (IS_ERR(ai->airo_thread_task))
goto err_out_free;
@@ -2927,7 +2923,6 @@
err_out_unlink:
del_airo_dev(dev);
err_out_thr:
- set_bit(JOB_DIE, &ai->jobs);
kthread_stop(ai->airo_thread_task);
err_out_free:
free_netdev(dev);
@@ -3055,70 +3050,52 @@
wireless_send_event(ai->dev, SIOCGIWSCAN, &wrqu, NULL);
}
-static int airo_thread(void *data) {
+static int airo_thread(void *data)
+{
struct net_device *dev = data;
struct airo_info *ai = dev->priv;
- int locked;
- while(1) {
+ while (1) {
/* make swsusp happy with our thread */
try_to_freeze();
- if (test_bit(JOB_DIE, &ai->jobs))
- break;
+ for (;;) {
+ unsigned long wake_at;
- if (ai->jobs) {
- locked = down_interruptible(&ai->sem);
- } else {
- wait_queue_t wait;
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (kthread_should_stop())
+ break;
+ if (ai->jobs)
+ break;
+ if (ai->scan_timeout &&
+ time_after_eq(jiffies, ai->scan_timeout)) {
+ set_bit(JOB_SCAN_RESULTS, &ai->jobs);
+ break;
+ }
- init_waitqueue_entry(&wait, current);
- add_wait_queue(&ai->thr_wait, &wait);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- if (ai->jobs)
- break;
- if (ai->expires || ai->scan_timeout) {
- if (ai->scan_timeout &&
- time_after_eq(jiffies,ai->scan_timeout)){
- set_bit(JOB_SCAN_RESULTS, &ai->jobs);
- break;
- } else if (ai->expires &&
- time_after_eq(jiffies,ai->expires)){
- set_bit(JOB_AUTOWEP, &ai->jobs);
- break;
- }
- if (!kthread_should_stop()) {
- unsigned long wake_at;
- if (!ai->expires || !ai->scan_timeout) {
- wake_at = max(ai->expires,
- ai->scan_timeout);
- } else {
- wake_at = min(ai->expires,
- ai->scan_timeout);
- }
- schedule_timeout(wake_at - jiffies);
- continue;
- }
- } else if (!kthread_should_stop()) {
- schedule();
- continue;
- }
+ if (ai->expires &&
+ time_after_eq(jiffies, ai->expires)){
+ set_bit(JOB_AUTOWEP, &ai->jobs);
break;
}
- current->state = TASK_RUNNING;
- remove_wait_queue(&ai->thr_wait, &wait);
- locked = 1;
- }
- if (locked)
- continue;
+ if (ai->expires && ai->scan_timeout)
+ wake_at = min(ai->expires, ai->scan_timeout);
+ else if (ai->expires || ai->scan_timeout)
+ wake_at = max(ai->expires, ai->scan_timeout);
+ else
+ wake_at = MAX_SCHEDULE_TIMEOUT;
- if (test_bit(JOB_DIE, &ai->jobs)) {
- up(&ai->sem);
- break;
+ schedule_timeout(wake_at - jiffies);
}
+ __set_current_state(TASK_RUNNING);
+ if (kthread_should_stop())
+ break;
+
+ if (down_interruptible(&ai->sem))
+ continue;
+
if (ai->power.event || test_bit(FLAG_FLASHING, &ai->flags)) {
up(&ai->sem);
continue;
@@ -3180,7 +3157,7 @@
OUT4500( apriv, EVACK, EV_MIC );
if (test_bit(FLAG_MIC_CAPABLE, &apriv->flags)) {
set_bit(JOB_MIC, &apriv->jobs);
- wake_up_interruptible(&apriv->thr_wait);
+ wake_up_process(apriv->airo_thread_task);
}
}
if ( status & EV_LINK ) {
@@ -3234,13 +3211,13 @@
if (down_trylock(&apriv->sem) != 0) {
set_bit(JOB_EVENT, &apriv->jobs);
- wake_up_interruptible(&apriv->thr_wait);
+ wake_up_process(apriv->airo_thread_task);
} else
airo_send_event(dev);
} else if (!scan_forceloss) {
if (auto_wep && !apriv->expires) {
apriv->expires = RUN_AT(3*HZ);
- wake_up_interruptible(&apriv->thr_wait);
+ wake_up_process(apriv->airo_thread_task);
}
/* Send event to user space */
@@ -3903,7 +3880,7 @@
if (auto_wep) {
ai->expires = RUN_AT(3*HZ);
- wake_up_interruptible(&ai->thr_wait);
+ wake_up_process(ai->airo_thread_task);
}
return SUCCESS;
@@ -7179,7 +7156,7 @@
out:
up(&ai->sem);
if (wake)
- wake_up_interruptible(&ai->thr_wait);
+ wake_up_process(ai->airo_thread_task);
return 0;
}
@@ -7677,7 +7654,7 @@
/* Get stats out of the card if available */
if (down_trylock(&local->sem) != 0) {
set_bit(JOB_WSTATS, &local->jobs);
- wake_up_interruptible(&local->thr_wait);
+ wake_up_process(local->airo_thread_task);
} else
airo_read_wireless_stats(local);
}
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
