[BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[James Bourne]

The Cisco Airo card driver calls schedule while atomic in the function
issuecommand in drivers/net/wireless/airo.c line 2388.


Jul 17 15:27:10 localhost kernel: bad: scheduling while atomic!
Jul 17 15:27:10 localhost kernel: Call Trace:
Jul 17 15:27:10 localhost kernel:  [<c0119754>] schedule+0x3c4/0x3d0
Jul 17 15:27:10 localhost kernel:  [<d18cbb51>] sendcommand+0xa1/0xe0 [airo]
Jul 17 15:27:10 localhost kernel:  [<d18cba80>] issuecommand+0x60/0x90 [airo]
Jul 17 15:27:10 localhost kernel:  [<d18cc001>] PC4500_accessrid+0x41/0x80 [airo]
Jul 17 15:27:10 localhost kernel:  [<d18cc0a3>] PC4500_readrid+0x63/0x130 [airo]
Jul 17 15:27:10 localhost kernel:  [<d18c95d9>] readStatsRid+0x29/0x50 [airo]
Jul 17 15:27:10 localhost kernel:  [<d18c9c0a>] airo_get_stats+0x2a/0xe0 [airo]
Jul 17 15:27:10 localhost kernel:  [<c013e194>] check_poison_obj+0x54/0x1d0
Jul 17 15:27:10 localhost kernel:  [<c013e194>] check_poison_obj+0x54/0x1d0
Jul 17 15:27:10 localhost kernel:  [<c013fa94>] kmem_cache_alloc+0x114/0x160
Jul 17 15:27:10 localhost kernel:  [<c01848ce>] proc_alloc_inode+0x1e/0x80
Jul 17 15:27:10 localhost kernel:  [<c01848fd>] proc_alloc_inode+0x4d/0x80
Jul 17 15:27:10 localhost kernel:  [<c016def7>] alloc_inode+0xd7/0x190
Jul 17 15:27:10 localhost kernel:  [<c0184887>] proc_read_inode+0x17/0x40
Jul 17 15:27:10 localhost kernel:  [<c016f941>] wake_up_inode+0x11/0x30
Jul 17 15:27:10 localhost kernel:  [<c01f070a>] vsnprintf+0x21a/0x440
Jul 17 15:27:10 localhost kernel:  [<c0173bd5>] seq_printf+0x45/0x60
Jul 17 15:27:10 localhost kernel:  [<c02846eb>] dev_seq_printf_stats+0xeb/0x100
Jul 17 15:27:10 localhost kernel:  [<c0284726>] dev_seq_show+0x26/0x80
Jul 17 15:27:10 localhost kernel:  [<c0173689>] seq_read+0x1c9/0x300
Jul 17 15:27:10 localhost kernel:  [<c0154bd3>] vfs_read+0xd3/0x140
Jul 17 15:27:10 localhost kernel:  [<c0154e7f>] sys_read+0x3f/0x60
Jul 17 15:27:10 localhost kernel:  [<c01094fb>] syscall_call+0x7/0xb

Regards
James Bourne

-- 
James Bourne                  | Email:            jbourne@hardrock.org          
Unix Systems Administrator    | WWW:           http://www.hardrock.org
Custom Unix Programming       | Linux:  The choice of a GNU generation
----------------------------------------------------------------------
 "All you need's an occasional kick in the philosophy." Frank Herbert  

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Andrew Morton]

James Bourne <jbourne@hardrock.org> wrote:
>
> The Cisco Airo card driver calls schedule while atomic in the function
> issuecommand in drivers/net/wireless/airo.c line 2388.
> 
> 
> Jul 17 15:27:10 localhost kernel: bad: scheduling while atomic!
> Jul 17 15:27:10 localhost kernel: Call Trace:
> Jul 17 15:27:10 localhost kernel:  [<c0119754>] schedule+0x3c4/0x3d0
> Jul 17 15:27:10 localhost kernel:  [<d18cbb51>] sendcommand+0xa1/0xe0 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18cba80>] issuecommand+0x60/0x90 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18cc001>] PC4500_accessrid+0x41/0x80 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18cc0a3>] PC4500_readrid+0x63/0x130 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18c95d9>] readStatsRid+0x29/0x50 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18c9c0a>] 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;
 }
 

_

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[James Bourne]

On Fri, 18 Jul 2003, Andrew Morton wrote:

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

Well, the error is gone, unfortunately I won't have anything for the card to
talk to until monday (or if I take my laptop for a car ride...).

Thanks!

James Bourne

-- 
James Bourne                  | Email:            jbourne@hardrock.org          
Unix Systems Administrator    | WWW:           http://www.hardrock.org
Custom Unix Programming       | Linux:  The choice of a GNU generation
----------------------------------------------------------------------
 "All you need's an occasional kick in the philosophy." Frank Herbert  

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Andrew Morton]

James Bourne <jbourne@hardrock.org> wrote:
>
> > I've been waiting months for someone to test this patch.  Can you please do
>  > so?
> 
>  Well, the error is gone, unfortunately I won't have anything for the card to
>  talk to until monday (or if I take my laptop for a car ride...).

Well Daniel Ritz has posted a big fix to the driver so I threw mine away. 
I'll include it in the next -mm, so please test that.

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Tom Sightler]

On Fri, 2003-07-18 at 23:00, Andrew Morton wrote:
> James Bourne <jbourne@hardrock.org> wrote:
> >
> > > I've been waiting months for someone to test this patch.  Can you please do
> >  > so?
> > 
> >  Well, the error is gone, unfortunately I won't have anything for the card to
> >  talk to until monday (or if I take my laptop for a car ride...).
> 
> Well Daniel Ritz has posted a big fix to the driver so I threw mine away. 
> I'll include it in the next -mm, so please test that.

I've applied Daniel's patch to my 2.6.0-test1-mm1 tree on two of my test
systems (a PCMCIA and PCI version of the Aironet 350 series) and both
are working great.  His patches look pretty obviously correct to me and
are much cleaner than the hacked up patches I've been sending out to
people to get the card working on recent 2.5.7x kernels.  Just wanted to
report the success.

Later,
Tom

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Tom Sightler]

On Mon, 2003-07-21 at 13:15, Georg Nikodym wrote:
> On 19 Jul 2003 22:58:57 +1000
> Sven Dowideit <svenud@ozemail.com.au> wrote:
> 
> > @@ -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;
> 
> I suspect that this part of the patch to airo.c is incorrect.  The
> intent seems to be to clear a section of memory pointed to by range that
> contains (or will soon contain) a struct iw_range.  The sizeof(*range)
> is equivalent of the sizeof(struct iw_range) above.  The patch reduces
> the size of the memset to the size of the pointer (which I'm assuming is
> smaller than the structure [/me goes and looks]).
> 
> Of course, the range pointer is derived from the char *extra
> parameter...  this could mean that we're actually getting a pre-filled
> iw_range and the memset is only supposed to clear the first member.  If
> that's the case, I would hope that the author could come up with a
> cleaner way of expressing that.

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

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Georg Nikodym]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

On 19 Jul 2003 22:58:57 +1000
Sven Dowideit <svenud@ozemail.com.au> wrote:

> @@ -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;

I suspect that this part of the patch to airo.c is incorrect.  The
intent seems to be to clear a section of memory pointed to by range that
contains (or will soon contain) a struct iw_range.  The sizeof(*range)
is equivalent of the sizeof(struct iw_range) above.  The patch reduces
the size of the memset to the size of the pointer (which I'm assuming is
smaller than the structure [/me goes and looks]).

Of course, the range pointer is derived from the char *extra
parameter...  this could mean that we're actually getting a pre-filled
iw_range and the memset is only supposed to clear the first member.  If
that's the case, I would hope that the author could come up with a
cleaner way of expressing that.

-g

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Sven Dowideit]

[-- Attachment #1: Type: text/plain, Size: 6627 bytes --]

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 <akpm@osdl.org>
To: James Bourne <jbourne@hardrock.org>
Cc: breed@users.sourceforge.net, linux-kernel@vger.kernel.org
Subject: Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

James Bourne <jbourne@hardrock.org> wrote:
>
> The Cisco Airo card driver calls schedule while atomic in the function
> issuecommand in drivers/net/wireless/airo.c line 2388.
> 
> 
> Jul 17 15:27:10 localhost kernel: bad: scheduling while atomic!
> Jul 17 15:27:10 localhost kernel: Call Trace:
> Jul 17 15:27:10 localhost kernel:  [<c0119754>] schedule+0x3c4/0x3d0
> Jul 17 15:27:10 localhost kernel:  [<d18cbb51>] sendcommand+0xa1/0xe0
[airo]
> Jul 17 15:27:10 localhost kernel:  [<d18cba80>] issuecommand+0x60/0x90
[airo]
> Jul 17 15:27:10 localhost kernel:  [<d18cc001>]
PC4500_accessrid+0x41/0x80 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18cc0a3>]
PC4500_readrid+0x63/0x130 [airo]
> Jul 17 15:27:10 localhost kernel:  [<d18c95d9>] readStatsRid+0x29/0x50
[airo]
> Jul 17 15:27:10 localhost kernel:  [<d18c9c0a>]
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 majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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


Tom's patch:

--- airo.c      2003-05-31 09:26:12.000000000 -0400
+++ airo.c.tom  2003-06-19 15:37:07.100811000 -0400
@@ -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;



[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

[Daniel Ritz]

> > Well Daniel Ritz has posted a big fix to the driver so I threw mine away. 
> > I'll include it in the next -mm, so please test that.
>
> I've applied Daniel's patch to my 2.6.0-test1-mm1 tree on two of my test
> systems (a PCMCIA and PCI version of the Aironet 350 series) and both
> are working great.  His patches look pretty obviously correct to me and
> are much cleaner than the hacked up patches I've been sending out to
> people to get the card working on recent 2.5.7x kernels.  Just wanted to
> report the success.
>

thanx...nice to hear that it works...

> Later,
> Tom
>

-daniel