cisco airo_cs scheduling while atomic

hey there,
I have a longer version of this patch from Tom that i applied to
2.6-test1, that fixes the bad scheduling problem..

I have appended his patch - will this qualify for a sucessful test?

cheers

sven

--------

Message: 19
Date: Fri, 18 Jul 2003 14:04:14 -0700
From: Andrew Morton <[email protected]>
To: James Bourne <[email protected]>
Cc: [email protected], [email protected]
Subject: Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[airo]

[airo]

PC4500_accessrid+0x41/0x80 [airo]

PC4500_readrid+0x63/0x130 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18c95d9>] readStatsRid+0x29/0x50

[airo]

airo_get_stats+0x2a/0xe0 [airo]

I've been waiting months for someone to test this patch. Can you please do
so?

diff -puN drivers/net/wireless/airo.c~airo-schedule-fix drivers/net/wireless/airo.c
--- 25/drivers/net/wireless/airo.c~airo-schedule-fix 2003-06-26
17:37:47.000000000 -0700
+++ 25-akpm/drivers/net/wireless/airo.c 2003-06-26 17:37:47.000000000 -0700
@@ -44,6 +44,7 @@
#include <linux/ioport.h>
#include <linux/config.h>
#include <linux/pci.h>
+#include <linux/delay.h>
#include <asm/uaccess.h>

#ifdef CONFIG_PCI
@@ -2379,20 +2380,26 @@ static u16 setup_card(struct airo_info *

 static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp) {
         // Im really paranoid about letting it run forever!
        int max_tries = 600000;

+ static int max = 0;
+ int count = 0;

        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();
+               udelay(1);
+               count++;
        }

- if ( max_tries == -1 ) {
+ if (max_tries == -1) {
printk( KERN_ERR
"airo: Max tries exceeded waiting for command\n"

);
                 return ERROR;
        }
        completecommand(ai, pRsp);

+ if (count > max) {
+ max = count;
+ printk("%s: max delay = %d usec\n", __FUNCTION__, max);
+ }

        return SUCCESS;

}

_

-
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/

-----------------------------------------------------------------

Tom's patch:

@@ -44,6 +44,7 @@
 #include <linux/ioport.h>
 #include <linux/config.h>
 #include <linux/pci.h>
+#include <linux/delay.h>
 #include <asm/uaccess.h>
 
 #ifdef CONFIG_PCI

@@ -1762,6 +1763,8 @@
        int i;
        struct airo_info *ai = dev->priv;
 
+       if (down_interruptible(&ai->sem))
+               return -1;
        waitbusy (ai);
        OUT4500(ai,COMMAND,CMD_SOFTRESET);
        set_current_state (TASK_UNINTERRUPTIBLE);

@@ -1771,6 +1774,7 @@
        schedule_timeout (HZ/5);
        if ( setup_card(ai, dev->dev_addr ) != SUCCESS ) {
                printk( KERN_ERR "airo: MAC could not be enabled\n" );
+               up(&ai->sem);
                return -1;
        } else {
                printk( KERN_INFO "airo: MAC enabled %s

%x:%x:%x:%x:%x:%x\n",
@@ -1788,6 +1792,7 @@

        }
        enable_interrupts( ai );
        netif_wake_queue(dev);
+       up(&ai->sem);
        return 0;

}

@@ -1866,6 +1871,7 @@

                if ( status & EV_MIC ) {
                        OUT4500( apriv, EVACK, EV_MIC );
+                       if (apriv->flags & FLAG_MIC_CAPABLE)
                        airo_read_mic( apriv );
                }
                if ( status & EV_LINK ) {
@@ -2379,20 +2385,26 @@
 static u16 issuecommand(struct airo_info *ai, Cmd *pCmd, Resp *pRsp) {
         // Im really paranoid about letting it run forever!
        int max_tries = 600000;

+ static int max = 0;
+ int count = 0;

        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();
+               udelay(1);
+               count++;
        }

- if ( max_tries == -1 ) {
+ if (max_tries == -1) {
printk( KERN_ERR
"airo: Max tries exceeded waiting for command\n"

);
                 return ERROR;
        }
        completecommand(ai, pRsp);

+ if (count > max) {
+ max = count;
+ printk("%s: max delay = %d usec\n", __FUNCTION__, max);
+ }

        return SUCCESS;

}

@@ -2653,11 +2665,11 @@

        if (down_interruptible(&ai->sem))
                return ERROR;
        if (issuecommand(ai, &cmd, &rsp) != SUCCESS) {
-               txFid = 0;
+               txFid = ERROR;
                goto done;
        }
        if ( (rsp.status & 0xFF00) != 0) {
-               txFid = 0;
+               txFid = ERROR;
                goto done;
        }
        /* wait for the allocate event/indication

@@ -2704,7 +2716,7 @@

        len >>= 16;

- if (len < ETH_ALEN * 2) {

+       if (len <= ETH_ALEN * 2) {
                printk( KERN_WARNING "Short packet %d\n", len );
                return ERROR;
        }
@@ -4838,7 +4850,7 @@
        readCapabilityRid(local, &cap_rid);
 
        dwrq->length = sizeof(struct iw_range);
-       memset(range, 0, sizeof(*range));
+       memset(range, 0, sizeof(range));
        range->min_nwid = 0x0000;
        range->max_nwid = 0x0000;
        range->num_channels = 14;

This has already been pointed out in a previous email. I had been maintaining my own patch for 2.5 for several months now that basically included fixes from the official CVS 2.4 airo driver, scheduling fixes to make the driver at least work, ref counting fixes, and several other minor fixes. Most of these were patches I collected off of LKML and with my own forward port of a few fixes from CVS.

The current CVS version of the driver at airo-linux.sourceforge.net use the sizeof(range) so somehow that managed to sneak into one of the patches I sent out. It wasn't intended to be part of the patch.

Most of these seperate fixes have since made their way into the -mm kernels and I personally like Daniel's fixes much better as, for me at least, the code is easier to follow without all the semaphore locking all over the place. As of this weekend I've moved all of my aironet systems to using Daniels driver and they are working great. I do understand the issues of the driver possibly holding a spinlock for an extended period of time using these patches, but my understanding is that there are only a few commands that are very slow, and they're typically only run for card setup (I could be wrong about that). For me, a solid driver is far more important that some spinlocks being held for a couple hundred milliseconds during card initialization.

Anyway, I'm no longer maintaining my patches as most of the fixes are now in the kernel tree. I was mainly keeping them for my own use and occasionally sent them out to people who were hitting bugs.

Later,
Tom