Re: [Bugme-new] [Bug 15091] New: starfire causes kernel BUG when interface goes up

Re: [Bugme-new] [Bug 15091] New: starfire causes kernel BUG when interface goes up

[Andrew Morton]

(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Wed, 20 Jan 2010 04:29:20 GMT
bugzilla-daemon@bugzilla.kernel.org wrote:

> http://bugzilla.kernel.org/show_bug.cgi?id=15091
> 
>            Summary: starfire causes kernel BUG when interface goes up
>            Product: Drivers
>            Version: 2.5
>     Kernel Version: 2.6.32
>           Platform: All
>         OS/Version: Linux
>               Tree: Mainline
>             Status: NEW
>           Severity: normal
>           Priority: P1
>          Component: Network
>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>         ReportedBy: michael@moffatt.org.nz
>         Regression: No
> 
> 
> Created an attachment (id=24651)
>  --> (http://bugzilla.kernel.org/attachment.cgi?id=24651)
> ls -l /dev (before crash)
> 
> I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> ethX up" (where X is a starfire port).
> 
> Sometimes the exception causes the whole kernel to freeze. Sometimes the kernel
> keeps going. On the occasion that the kernel kept going I was able to retrieve
> syslog, which has the full kernel information.
> 
> Note that in syslog, you can see that I inserted a USB memory stick in order to
> copy off the attached files. The kernel oops happens without the USB memory
> stick inserted.
> 
> I can reproduce this at will. At the moment I simply can't use my two four port
> starfire network cards.
> 
> This PC is a root-over-NFS system.
> 

Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
&n->state)); in napi_enable().

This is a regression somewhere between 2.6.24 and 2.6.32(!).

Re: [Bugme-new] [Bug 15091] New: starfire causes kernel BUG when interface goes up

[Michael]

Hi Andrew,

I believe that this is a regression, yes.

I will attempt to compile up some kernels this week and provide more 
info. Should I start at 26 and go up or at 31 and go down?

I can't use anything lower than 26 according to udev. I was running 24 
but compiled 32 when I upgraded udev.

Regards,
Michael.

Andrew Morton wrote:
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
>
> On Wed, 20 Jan 2010 04:29:20 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>   
>> http://bugzilla.kernel.org/show_bug.cgi?id=15091
>>
>>            Summary: starfire causes kernel BUG when interface goes up
>>            Product: Drivers
>>            Version: 2.5
>>     Kernel Version: 2.6.32
>>           Platform: All
>>         OS/Version: Linux
>>               Tree: Mainline
>>             Status: NEW
>>           Severity: normal
>>           Priority: P1
>>          Component: Network
>>         AssignedTo: drivers_network@kernel-bugs.osdl.org
>>         ReportedBy: michael@moffatt.org.nz
>>         Regression: No
>>
>>
>> Created an attachment (id=24651)
>>  --> (http://bugzilla.kernel.org/attachment.cgi?id=24651)
>> ls -l /dev (before crash)
>>
>> I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
>> cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
>> ethX up" (where X is a starfire port).
>>
>> Sometimes the exception causes the whole kernel to freeze. Sometimes the kernel
>> keeps going. On the occasion that the kernel kept going I was able to retrieve
>> syslog, which has the full kernel information.
>>
>> Note that in syslog, you can see that I inserted a USB memory stick in order to
>> copy off the attached files. The kernel oops happens without the USB memory
>> stick inserted.
>>
>> I can reproduce this at will. At the moment I simply can't use my two four port
>> starfire network cards.
>>
>> This PC is a root-over-NFS system.
>>
>>     
>
> Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> &n->state)); in napi_enable().
>
> This is a regression somewhere between 2.6.24 and 2.6.32(!).
>
>
>   

Re: [Bugme-new] [Bug 15091] New: starfire causes kernel BUG when interface goes up

[Andrew Morton]

On Tue, 26 Jan 2010 14:44:31 +1300 Michael <michael@moffatt.org.nz> wrote:

> Hi Andrew,
> 
> I believe that this is a regression, yes.
> 
> I will attempt to compile up some kernels this week and provide more 
> info. Should I start at 26 and go up or at 31 and go down?
> 
> I can't use anything lower than 26 according to udev. I was running 24 
> but compiled 32 when I upgraded udev.
> 

Thanks.

Starfire is a pretty rarely-used driver, I suspect.  Hopefully someone
who understands the NAPI stuff can look at the code and go "ah-hah",
and save you all that work.

But if that doesn't happen then yup, a bisection would be good, thanks.
 The best way to do it really is with git. 
http://landley.net/writing/git-quick.html has an explanation.

[PATCH] starfire: Clean up properly if firmware loading fails

[Ben Hutchings]

netdev_open() will return without cleaning up net device or hardware
state if firmware loading fails.  This results in a BUG() on a second
attempt to bring the interface up, reported in
<http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
even worse effects if the driver is removed afterwards.

Call netdev_close() to clean up on failure.
---
On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Wed, 20 Jan 2010 04:29:20 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=15091
> > 
> >            Summary: starfire causes kernel BUG when interface goes up
[...] 
> > I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> > cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> > ethX up" (where X is a starfire port).
[...] 
> Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> &n->state)); in napi_enable().
> 
> This is a regression somewhere between 2.6.24 and 2.6.32(!).

This driver now attempts to load firmware when an interface is brought
up, *after* calling napi_enable().  If that fails, it will return
without calling napi_disable().  On the second attempt to bring the
interface it calls napi_enable() a second time and triggers this
assertion.

As a workaround, try installing the necessary firmware. :-)

Ben.

 drivers/net/starfire.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/starfire.c b/drivers/net/starfire.c
index 95db60a..f952113 100644
--- a/drivers/net/starfire.c
+++ b/drivers/net/starfire.c
@@ -1063,7 +1063,7 @@ static int netdev_open(struct net_device *dev)
 	if (retval) {
 		printk(KERN_ERR "starfire: Failed to load firmware \"%s\"\n",
 		       FIRMWARE_RX);
-		return retval;
+		goto out_init;
 	}
 	if (fw_rx->size % 4) {
 		printk(KERN_ERR "starfire: bogus length %zu in \"%s\"\n",
@@ -1108,6 +1108,9 @@ out_tx:
 	release_firmware(fw_tx);
 out_rx:
 	release_firmware(fw_rx);
+out_init:
+	if (retval)
+		netdev_close(dev);
 	return retval;
 }
 
-- 
1.6.6

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Andrew Morton]

On Tue, 26 Jan 2010 02:02:12 +0000 Ben Hutchings <bhutchings@solarflare.com> wrote:

> netdev_open() will return without cleaning up net device or hardware
> state if firmware loading fails.  This results in a BUG() on a second
> attempt to bring the interface up, reported in
> <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
> even worse effects if the driver is removed afterwards.

(That's the wrong bugzilla URL)

> 
> Call netdev_close() to clean up on failure.

OK, thanks.

> ---
> On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
> > (switched to email.  Please respond via emailed reply-to-all, not via the
> > bugzilla web interface).
> > 
> > On Wed, 20 Jan 2010 04:29:20 GMT
> > bugzilla-daemon@bugzilla.kernel.org wrote:
> > 
> > > http://bugzilla.kernel.org/show_bug.cgi?id=15091
> > > 
> > >            Summary: starfire causes kernel BUG when interface goes up
> [...] 
> > > I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> > > cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> > > ethX up" (where X is a starfire port).
> [...] 
> > Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> > &n->state)); in napi_enable().
> > 
> > This is a regression somewhere between 2.6.24 and 2.6.32(!).
> 
> This driver now attempts to load firmware when an interface is brought
> up, *after* calling napi_enable().  If that fails, it will return
> without calling napi_disable().  On the second attempt to bring the
> interface it calls napi_enable() a second time and triggers this
> assertion.
> 
> As a workaround, try installing the necessary firmware. :-)
> 

Missing signed-off-by.  I added it, OK?  Also added a Cc:stable.


From: Ben Hutchings <bhutchings@solarflare.com>

netdev_open() will return without cleaning up net device or hardware state
if firmware loading fails.  This results in a BUG() on a second attempt to
bring the interface up, reported in
<http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has even
worse effects if the driver is removed afterwards.

Call netdev_close() to clean up on failure.

Addresses http://bugzilla.kernel.org/show_bug.cgi?id=15091

Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
Reported-by: Michael Moffatt <michael@moffatt.org.nz>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: <stable@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/net/starfire.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff -puN drivers/net/starfire.c~starfire-clean-up-properly-if-firmware-loading-fails drivers/net/starfire.c
--- a/drivers/net/starfire.c~starfire-clean-up-properly-if-firmware-loading-fails
+++ a/drivers/net/starfire.c
@@ -1063,7 +1063,7 @@ static int netdev_open(struct net_device
 	if (retval) {
 		printk(KERN_ERR "starfire: Failed to load firmware \"%s\"\n",
 		       FIRMWARE_RX);
-		return retval;
+		goto out_init;
 	}
 	if (fw_rx->size % 4) {
 		printk(KERN_ERR "starfire: bogus length %zu in \"%s\"\n",
@@ -1108,6 +1108,9 @@ out_tx:
 	release_firmware(fw_tx);
 out_rx:
 	release_firmware(fw_rx);
+out_init:
+	if (retval)
+		netdev_close(dev);
 	return retval;
 }
 
_

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Ben Hutchings]

On Mon, 2010-01-25 at 18:15 -0800, Andrew Morton wrote:
> On Tue, 26 Jan 2010 02:02:12 +0000 Ben Hutchings <bhutchings@solarflare.com> wrote:
> 
> > netdev_open() will return without cleaning up net device or hardware
> > state if firmware loading fails.  This results in a BUG() on a second
> > attempt to bring the interface up, reported in
> > <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
> > even worse effects if the driver is removed afterwards.
> 
> (That's the wrong bugzilla URL)

Hah, well spotted.  Must have copied from the wrong tab.

> > Call netdev_close() to clean up on failure.
> 
> OK, thanks.
> 
> > ---
> > On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
> > > (switched to email.  Please respond via emailed reply-to-all, not via the
> > > bugzilla web interface).
> > > 
> > > On Wed, 20 Jan 2010 04:29:20 GMT
> > > bugzilla-daemon@bugzilla.kernel.org wrote:
> > > 
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=15091
> > > > 
> > > >            Summary: starfire causes kernel BUG when interface goes up
> > [...] 
> > > > I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
> > > > cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
> > > > ethX up" (where X is a starfire port).
> > [...] 
> > > Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
> > > &n->state)); in napi_enable().
> > > 
> > > This is a regression somewhere between 2.6.24 and 2.6.32(!).
> > 
> > This driver now attempts to load firmware when an interface is brought
> > up, *after* calling napi_enable().  If that fails, it will return
> > without calling napi_disable().  On the second attempt to bring the
> > interface it calls napi_enable() a second time and triggers this
> > assertion.
> > 
> > As a workaround, try installing the necessary firmware. :-)
> > 
> 
> Missing signed-off-by.  I added it, OK?  Also added a Cc:stable.
[...]

Well I was going to wait and see what the submitter says first, but I'm
happy for you to add that in your patch set if it's a positive result.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Michael]

Hi guys,

I think I'm the submitter that Ben is referring to.

So that could be the answer to the kernel BUG I have reported, but I 
don't think that it will answer why the interface doesn't come up... or 
does it?

Cheers,
Michael.

Ben Hutchings wrote:
> On Mon, 2010-01-25 at 18:15 -0800, Andrew Morton wrote:
>   
>> On Tue, 26 Jan 2010 02:02:12 +0000 Ben Hutchings <bhutchings@solarflare.com> wrote:
>>
>>     
>>> netdev_open() will return without cleaning up net device or hardware
>>> state if firmware loading fails.  This results in a BUG() on a second
>>> attempt to bring the interface up, reported in
>>> <http://bugzilla.kernel.org/show_bug.cgi?id=15117>, and probably has
>>> even worse effects if the driver is removed afterwards.
>>>       
>> (That's the wrong bugzilla URL)
>>     
>
> Hah, well spotted.  Must have copied from the wrong tab.
>
>   
>>> Call netdev_close() to clean up on failure.
>>>       
>> OK, thanks.
>>
>>     
>>> ---
>>> On Mon, 2010-01-25 at 17:08 -0800, Andrew Morton wrote: 
>>>       
>>>> (switched to email.  Please respond via emailed reply-to-all, not via the
>>>> bugzilla web interface).
>>>>
>>>> On Wed, 20 Jan 2010 04:29:20 GMT
>>>> bugzilla-daemon@bugzilla.kernel.org wrote:
>>>>
>>>>         
>>>>> http://bugzilla.kernel.org/show_bug.cgi?id=15091
>>>>>
>>>>>            Summary: starfire causes kernel BUG when interface goes up
>>>>>           
>>> [...] 
>>>       
>>>>> I formerly used 2.6.20 and 2.6.24 with a couple of starfire 4 port ethernet
>>>>> cards. On 2.6.32 the interfaces don't start on boot and when I issue "ifconfig
>>>>> ethX up" (where X is a starfire port).
>>>>>           
>>> [...] 
>>>       
>>>> Starfire is triggering the BUG_ON(!test_bit(NAPI_STATE_SCHED,
>>>> &n->state)); in napi_enable().
>>>>
>>>> This is a regression somewhere between 2.6.24 and 2.6.32(!).
>>>>         
>>> This driver now attempts to load firmware when an interface is brought
>>> up, *after* calling napi_enable().  If that fails, it will return
>>> without calling napi_disable().  On the second attempt to bring the
>>> interface it calls napi_enable() a second time and triggers this
>>> assertion.
>>>
>>> As a workaround, try installing the necessary firmware. :-)
>>>
>>>       
>> Missing signed-off-by.  I added it, OK?  Also added a Cc:stable.
>>     
> [...]
>
> Well I was going to wait and see what the submitter says first, but I'm
> happy for you to add that in your patch set if it's a positive result.
>
> Ben.
>
>   

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Andrew Morton]

On Tue, 26 Jan 2010 15:58:39 +1300 Michael <michael@moffatt.org.nz> wrote:

> Hi guys,
> 
> I think I'm the submitter that Ben is referring to.
> 
> So that could be the answer to the kernel BUG I have reported, but I 
> don't think that it will answer why the interface doesn't come up... or 
> does it?

>From this:

Jan 21 05:08:26 172 kernel: starfire: Failed to load firmware "adaptec/starfire_rx.bin"
Jan 21 05:08:26 172 kernel: device eth4 entered promiscuous mode
Jan 21 05:08:26 172 kernel: starfire 0000:03:06.0: firmware: requesting adaptec/starfire_rx.bin
Jan 21 05:08:26 172 kernel: starfire: Failed to load firmware "adaptec/starfire_rx.bin"
Jan 21 05:08:26 172 kernel: device eth5 entered promiscuous mode
Jan 21 05:08:26 172 kernel: starfire 0000:03:07.0: firmware: requesting adaptec/starfire_rx.bin
Jan 21 05:08:26 172 kernel: starfire: Failed to load firmware "adaptec/starfire_rx.bin"
Jan 21 05:08:26 172 kernel: device eth6 entered promiscuous mode
Jan 21 05:08:26 172 kernel: starfire 0000:04:04.0: firmware: requesting adaptec/starfire_rx.bin

I assume that it can't find the firmware?

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Michael]

Hi Andrew,

Yep, OK, I hadn't seen that log in dmesg.

That driver is new to me, it's never turned up (or been required) before 
so that must be new between 2.6.24 and 2.6.32.

As this is a root over nfs system, the kernel is compiled elsewhere and 
then installed manually. What I was missing was that there is a new 
adaptec directory that needed to be copied from 
/usr/src/linux-2.6.32/firmware/ to /lib/2.6.32. Actually, I am not sure 
that this is the right place for it (rather than say 
/lib/firmware/2.6.32|, but it seems to work anyway.

Quite a gotchya. After compiling a kernel on a separate compiling 
system, I don't actually run the 'make install' on the nfs system. 
Previously I used to run the 'make install' on a dedicated compiling 
server and then just copy the modules from that system into the root 
over nfs exported /lib/modules directory. That'd worked fine up until now.

I will have to find a cleverer way to copy over the new firmware libs 
for future compiles. The 'make install' seems to copy firmware objects 
into the compiling system's /lib/firmware/ directory without 
distinguishing the kernel version. So I can't easily tell which ones I'm 
supposed to be copying into the nfs export.

Many thanks for all your help. The interfaces are up and apparently you 
understand where the kernel BUG came from.

So does that complete the story now? In other words, is there anything 
further you need from me.

Many thanks,
Michael.
|

Andrew Morton wrote:
> On Tue, 26 Jan 2010 15:58:39 +1300 Michael <michael@moffatt.org.nz> wrote:
>
>   
>> Hi guys,
>>
>> I think I'm the submitter that Ben is referring to.
>>
>> So that could be the answer to the kernel BUG I have reported, but I 
>> don't think that it will answer why the interface doesn't come up... or 
>> does it?
>>     
>
> >From this:
>
> Jan 21 05:08:26 172 kernel: starfire: Failed to load firmware "adaptec/starfire_rx.bin"
> Jan 21 05:08:26 172 kernel: device eth4 entered promiscuous mode
> Jan 21 05:08:26 172 kernel: starfire 0000:03:06.0: firmware: requesting adaptec/starfire_rx.bin
> Jan 21 05:08:26 172 kernel: starfire: Failed to load firmware "adaptec/starfire_rx.bin"
> Jan 21 05:08:26 172 kernel: device eth5 entered promiscuous mode
> Jan 21 05:08:26 172 kernel: starfire 0000:03:07.0: firmware: requesting adaptec/starfire_rx.bin
> Jan 21 05:08:26 172 kernel: starfire: Failed to load firmware "adaptec/starfire_rx.bin"
> Jan 21 05:08:26 172 kernel: device eth6 entered promiscuous mode
> Jan 21 05:08:26 172 kernel: starfire 0000:04:04.0: firmware: requesting adaptec/starfire_rx.bin
>
> I assume that it can't find the firmware?
>   

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Andrew Morton]

On Tue, 26 Jan 2010 18:51:07 +1300 Michael <michael@moffatt.org.nz> wrote:

> Yep, OK, I hadn't seen that log in dmesg.
> 
> That driver is new to me, it's never turned up (or been required) before 
> so that must be new between 2.6.24 and 2.6.32.
> 
> As this is a root over nfs system, the kernel is compiled elsewhere and 
> then installed manually. What I was missing was that there is a new 
> adaptec directory that needed to be copied from 
> /usr/src/linux-2.6.32/firmware/ to /lib/2.6.32. Actually, I am not sure 
> that this is the right place for it (rather than say 
> /lib/firmware/2.6.32|, but it seems to work anyway.
> 
> Quite a gotchya. After compiling a kernel on a separate compiling 
> system, I don't actually run the 'make install' on the nfs system. 
> Previously I used to run the 'make install' on a dedicated compiling 
> server and then just copy the modules from that system into the root 
> over nfs exported /lib/modules directory. That'd worked fine up until now.
> 
> I will have to find a cleverer way to copy over the new firmware libs 
> for future compiles. The 'make install' seems to copy firmware objects 
> into the compiling system's /lib/firmware/ directory without 
> distinguishing the kernel version. So I can't easily tell which ones I'm 
> supposed to be copying into the nfs export.
> 
> Many thanks for all your help. The interfaces are up and apparently you 
> understand where the kernel BUG came from.
> 
> So does that complete the story now? In other words, is there anything 
> further you need from me.

Well, it'd be great if you could test Ben's patch with the firmware
file inaccessible, and confirm that the kernel doesn't crash/freeze/etc.

Thanks.

Re: [PATCH] starfire: Clean up properly if firmware loading fails

[Ben Hutchings]

On Tue, 2010-01-26 at 18:51 +1300, Michael wrote:
[...]
> I will have to find a cleverer way to copy over the new firmware libs 
> for future compiles. The 'make install' seems to copy firmware objects 
> into the compiling system's /lib/firmware/ directory without 
> distinguishing the kernel version. So I can't easily tell which ones I'm 
> supposed to be copying into the nfs export.
[...]

You should be setting INSTALL_MOD_PATH.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.