2003-07-18 12:49:21

by James Bourne

[permalink] [raw]
Subject: [BUG REPORT 2.6.0] cisco airo_cs scheduling while atomic

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: [email protected]
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



2003-07-18 20:58:14

by Andrew Morton

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

James Bourne <[email protected]> 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;
}


_

2003-07-19 02:31:18

by James Bourne

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

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: [email protected]
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

2003-07-19 02:44:34

by Andrew Morton

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

James Bourne <[email protected]> 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.

2003-07-19 04:23:29

by Tom Sightler

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

On Fri, 2003-07-18 at 23:00, Andrew Morton wrote:
> James Bourne <[email protected]> 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


2003-07-19 12:01:54

by Daniel Ritz

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

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

2003-07-19 13:11:04

by Sven Dowideit

[permalink] [raw]
Subject: Re: [BUG REPORT 2.6.0] 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

James Bourne <[email protected]> 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;
}


_


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2003-07-21 17:01:36

by Georg Nikodym

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

On 19 Jul 2003 22:58:57 +1000
Sven Dowideit <[email protected]> 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


Attachments:
(No filename) (189.00 B)

2003-07-21 19:39:00

by Tom Sightler

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

On Mon, 2003-07-21 at 13:15, Georg Nikodym wrote:
> On 19 Jul 2003 22:58:57 +1000
> Sven Dowideit <[email protected]> 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