On Thu, 22 Mar 2007 07:23:06 -0500,
Larry Finger <[email protected]> wrote:
> Cornelia Huck wrote:
> > On Wed, 21 Mar 2007 23:39:17 -0800,
> > Andrew Morton <[email protected]> wrote:
> >
> >> On Wed, 21 Mar 2007 15:22:25 -0500 Matt Mackall <[email protected]> wrote:
> >>
> >>> With the latest -mm, I'm now getting this:
> >>>
> >>> Mar 21 15:06:52 cinder kernel: ipw2200: Detected Intel PRO/Wireless
> >>> 2200BG Network Connection
> >>> Mar 21 15:06:52 cinder kernel: firmware_loading_store: unexpected
> >>> value (0)
> >>> Mar 21 15:06:52 cinder kernel: ipw2200: ipw2200-bss.fw
> >>> request_firmware failed:
> >>> Reason -2
> >>> Mar 21 15:06:52 cinder kernel: ipw2200: Unable to load firmware: -2
> >>> Mar 21 15:06:52 cinder kernel: ipw2200: failed to register network
> >>> device
> >> The firmware loading bug is caused by
> >> driver-core-handles-kobject_uevent-failure-while-device_add.patch
> >
> > Hm, this patch looks sane. It might be a good idea to enable
> > CONFIG_DEBUG_KOBJECT and find out why kobject_uevent() actually fails
> > in this case...
>
> Attached is the appropriate portion of /var/log/messages with Kobject debugging enabled.
> Mar 22 07:01:42 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
> Mar 22 07:01:42 larrylap2 kernel: kobject_uevent_env
> Mar 22 07:01:42 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
> Mar 22 07:01:42 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
> Mar 22 07:01:42 larrylap2 kernel: kobject_uevent_env
> Mar 22 07:01:42 larrylap2 ntpd[3434]: frequency initialized -31.513 PPM from /var/lib/ntp/drift/ntp.drift
> Mar 22 07:01:43 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
> Mar 22 07:01:43 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
> Mar 22 07:01:43 larrylap2 kernel: firmware_loading_store: unexpected value (0)
> Mar 22 07:01:43 larrylap2 kernel: kobject_uevent_env
> Mar 22 07:01:43 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
> Mar 22 07:01:43 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
> Mar 22 07:01:43 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
(Repeating several times)
This would indicate that dev_uevent had been called. But how could
kobject_uevent then return an error without moaning about an uevent()
error code? Maybe the following debug patch could shed some light on
this (all moaning is prefixed with kobject_uevent_env, so it should be
easy to spot)...
---
lib/kobject_uevent.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
--- linux-2.6.orig/lib/kobject_uevent.c
+++ linux-2.6/lib/kobject_uevent.c
@@ -85,11 +85,11 @@ int kobject_uevent_env(struct kobject *k
int retval = 0;
int j;
- pr_debug("%s\n", __FUNCTION__);
+ pr_debug("%s: %s\n", __FUNCTION__, kobject_name(kobj));
action_string = action_to_string(action);
if (!action_string) {
- pr_debug("kobject attempted to send uevent without action_string!\n");
+ pr_debug("%s: kobject attempted to send uevent without action_string!\n", __FUNCTION__);
return -EINVAL;
}
@@ -101,7 +101,7 @@ int kobject_uevent_env(struct kobject *k
} while (!top_kobj->kset && top_kobj->parent);
}
if (!top_kobj->kset) {
- pr_debug("kobject attempted to send uevent without kset!\n");
+ pr_debug("%s: kobject attempted to send uevent without kset!\n", __FUNCTION__);
return -EINVAL;
}
@@ -111,7 +111,7 @@ int kobject_uevent_env(struct kobject *k
/* skip the event, if the filter returns zero. */
if (uevent_ops && uevent_ops->filter)
if (!uevent_ops->filter(kset, kobj)) {
- pr_debug("kobject filter function caused the event to drop!\n");
+ pr_debug("%s: kobject filter function caused the event to drop!\n", __FUNCTION__);
return 0;
}
@@ -121,18 +121,20 @@ int kobject_uevent_env(struct kobject *k
else
subsystem = kobject_name(&kset->kobj);
if (!subsystem) {
- pr_debug("unset subsytem caused the event to drop!\n");
+ pr_debug("%s: unset subsytem caused the event to drop!\n", __FUNCTION__);
return 0;
}
/* environment index */
envp = kzalloc(NUM_ENVP * sizeof (char *), GFP_KERNEL);
- if (!envp)
+ if (!envp) {
+ pr_debug("%s: couldn't alloc envp\n", __FUNCTION__);
return -ENOMEM;
-
+ }
/* environment values */
buffer = kmalloc(BUFFER_SIZE, GFP_KERNEL);
if (!buffer) {
+ pr_debug("%s: couldn't alloc buffer\n", __FUNCTION__);
retval = -ENOMEM;
goto exit;
}
@@ -140,6 +142,7 @@ int kobject_uevent_env(struct kobject *k
/* complete object path */
devpath = kobject_get_path(kobj, GFP_KERNEL);
if (!devpath) {
+ pr_debug("%s: couldn't get kobject path\n", __FUNCTION__);
retval = -ENOENT;
goto exit;
}
@@ -221,6 +224,7 @@ exit:
kfree(devpath);
kfree(buffer);
kfree(envp);
+ pr_debug("%s: returning %d\n", __FUNCTION__, retval);
return retval;
}
On Tue, Mar 27, 2007 at 07:17:49PM +0200, Cornelia Huck wrote:
> On Tue, 27 Mar 2007 11:25:57 +0200,
> "Kay Sievers" <[email protected]> wrote:
> > Checking the uevent return value, will not prevent any malfunction,
> > usually this kind of "error handling" just prevents bringing up a
> > whole subsystem, or booting-up a box, because the needed device does
> > not exist at all.
>
> OK, if we consider uevents to be non-vital to a functioning device.
The reason for that original patch was that it is actually possible for the
uevent functions to return -ENOMEM, the uevent buffer being statically
allocated to BUFFER_SIZE (2048). It used to be 1024 but that was not
always enough and it was doubled a while ago [1]. Using add_uevent_var()
makes this less of a problem as such an overflow should be catched
cleanly [2].
> OTOH, I think using something like uevent_suppress (maybe via
> dev_uevent_filter?) is a saner way to suppress a uevent than to return
> an error code in the uevent function.
That makes sense, I guess. I will try that.
Thanks.
[1] http://marc.info/?t=113797361200002&r=1&w=2
[2] uevent-use-add_uevent_var-instead-of-open-coding-it.patch in rc4-mm1
On Thu, 22 Mar 2007 13:55:51 -0500,
Larry Finger <[email protected]> wrote:
> Cornelia Huck wrote:
> > On Thu, 22 Mar 2007 07:23:06 -0500,
> >
> > This would indicate that dev_uevent had been called. But how could
> > kobject_uevent then return an error without moaning about an uevent()
> > error code? Maybe the following debug patch could shed some light on
> > this (all moaning is prefixed with kobject_uevent_env, so it should be
> > easy to spot)...
>
> I applied the debug patch, but I don't see any error codes being returned. This time I also got the
> General Protection Faults. An excerpt of the log is attached.
Hm, I think I have an idea about what happened.
The firmware class tried to suppress the first KOBJ_ADD uevent by
returning -ENODEV in firmware_uevent if FW_STATUS_READY was not set.
This only worked as long as the return code of kobject_uevent was not
checked in device_add. hack-to-make-wireless-work.patch made that first
uevent return successfully, but this possible triggered some udev rule
too early, leading to firmware load failures.
The following (completely untested) patch uses uevent_suppress to stop
the uevent from being generated during device_add. Does this work for
you?
---
drivers/base/firmware_class.c | 2 ++
1 file changed, 2 insertions(+)
--- linux-2.6.orig/drivers/base/firmware_class.c
+++ linux-2.6/drivers/base/firmware_class.c
@@ -333,6 +333,7 @@ static int fw_register_device(struct dev
f_dev->parent = device;
f_dev->class = &firmware_class;
dev_set_drvdata(f_dev, fw_priv);
+ f_dev->uevent_suppress = 1;
retval = device_register(f_dev);
if (retval) {
printk(KERN_ERR "%s: device_register failed\n",
@@ -385,6 +386,7 @@ static int fw_setup_device(struct firmwa
set_bit(FW_STATUS_READY, &fw_priv->status);
else
set_bit(FW_STATUS_READY_NOHOTPLUG, &fw_priv->status);
+ f_dev->uevent_suppress = 0;
*dev_p = f_dev;
goto out;
On 3/26/07, Eric Rannaud <[email protected]> wrote:
> On Mon, Mar 26, 2007 at 01:22:32AM -0800, Andrew Morton wrote:
> > On Mon, 26 Mar 2007 11:09:49 +0200 Cornelia Huck <[email protected]> wrote:
> > > > If so, do you think I should labour on with
> > > > uevent-improve-error-checking-and-handling.patch plus your fix, or should I
> > > > drop the lot? (I'm inclined toward the latter, but I'm still not
> > > > sure which patch(es) need to be dropped).
> > >
> > > This depends on what semantics uevent returning an error code should
> > > have. The firmware code was using it to suppress uevents, but
> > > uevent_suppress is a better idea now. So if we want uevent returning !=
> > > 0 to imply "something really bad happened", all uevent functions have
> > > to be audited and those that work like firmware_uevent have to be
> > > converted to uevent_suppress. This would be cleaner, but I'm not sure
> > > it's worth the work.
> >
> > We're generally struggling to stay alive amongst all the bugs at present -
> > I'll drop all those patches.
>
> My mistake, I wrote the guilty patch
> uevent-improve-error-checking-and-handling.patch assuming it was safe to
> treat the return value as an error code, since several uevent functions
> returns things like -ENOMEM.
>
> Should I rework the patch as Cornelia suggests and resubmit later, when
> things have settled down a little?
I don't see any point in deregistering a kernel device, if the event
to userspace goes wrong, or a subsytem returns a non-zero value in the
filter.
Checking the uevent return value, will not prevent any malfunction,
usually this kind of "error handling" just prevents bringing up a
whole subsystem, or booting-up a box, because the needed device does
not exist at all.
Thanks,
Kay
On Mon, 26 Mar 2007 11:09:49 +0200 Cornelia Huck <[email protected]> wrote:
> On Fri, 23 Mar 2007 21:06:18 -0800,
> Andrew Morton <[email protected]> wrote:
>
> > Would I be right in guessing that this was all triggered by
> > uevent-improve-error-checking-and-handling.patch?
>
> Looks like it, since it passed the uevent failures to the upper layer.
OK, thanks.
> > If so, do you think I should labour on with
> > uevent-improve-error-checking-and-handling.patch plus your fix, or should I
> > drop the lot? (I'm inclined toward the latter, but I'm still not
> > sure which patch(es) need to be dropped).
>
> This depends on what semantics uevent returning an error code should
> have. The firmware code was using it to suppress uevents, but
> uevent_suppress is a better idea now. So if we want uevent returning !=
> 0 to imply "something really bad happened", all uevent functions have
> to be audited and those that work like firmware_uevent have to be
> converted to uevent_suppress. This would be cleaner, but I'm not sure
> it's worth the work.
We're generally struggling to stay alive amongst all the bugs at present -
I'll drop all those patches.
On Mon, 26 Mar 2007 12:34:33 +0200 Eric Rannaud <[email protected]> wrote:
> On Mon, Mar 26, 2007 at 01:22:32AM -0800, Andrew Morton wrote:
> > On Mon, 26 Mar 2007 11:09:49 +0200 Cornelia Huck <[email protected]> wrote:
> > > > If so, do you think I should labour on with
> > > > uevent-improve-error-checking-and-handling.patch plus your fix, or should I
> > > > drop the lot? (I'm inclined toward the latter, but I'm still not
> > > > sure which patch(es) need to be dropped).
> > >
> > > This depends on what semantics uevent returning an error code should
> > > have. The firmware code was using it to suppress uevents, but
> > > uevent_suppress is a better idea now. So if we want uevent returning !=
> > > 0 to imply "something really bad happened", all uevent functions have
> > > to be audited and those that work like firmware_uevent have to be
> > > converted to uevent_suppress. This would be cleaner, but I'm not sure
> > > it's worth the work.
> >
> > We're generally struggling to stay alive amongst all the bugs at present -
> > I'll drop all those patches.
>
> My mistake, I wrote the guilty patch
> uevent-improve-error-checking-and-handling.patch assuming it was safe to
> treat the return value as an error code, since several uevent functions
> returns things like -ENOMEM.
>
> Should I rework the patch as Cornelia suggests and resubmit later, when
> things have settled down a little?
Sure, when we've fixed all the bugs ;)
I think we now know what to test for - firmware loading simply collapsed
all over the place with these changes.
On Fri, 23 Mar 2007 21:06:18 -0800,
Andrew Morton <[email protected]> wrote:
> Would I be right in guessing that this was all triggered by
> uevent-improve-error-checking-and-handling.patch?
Looks like it, since it passed the uevent failures to the upper layer.
> If so, do you think I should labour on with
> uevent-improve-error-checking-and-handling.patch plus your fix, or should I
> drop the lot? (I'm inclined toward the latter, but I'm still not
> sure which patch(es) need to be dropped).
This depends on what semantics uevent returning an error code should
have. The firmware code was using it to suppress uevents, but
uevent_suppress is a better idea now. So if we want uevent returning !=
0 to imply "something really bad happened", all uevent functions have
to be audited and those that work like firmware_uevent have to be
converted to uevent_suppress. This would be cleaner, but I'm not sure
it's worth the work.
On Fri, 23 Mar 2007 11:10:29 +0100 Cornelia Huck <[email protected]> wrote:
> On Thu, 22 Mar 2007 13:55:51 -0500,
> Larry Finger <[email protected]> wrote:
>
> > Cornelia Huck wrote:
> > > On Thu, 22 Mar 2007 07:23:06 -0500,
> > >
> > > This would indicate that dev_uevent had been called. But how could
> > > kobject_uevent then return an error without moaning about an uevent()
> > > error code? Maybe the following debug patch could shed some light on
> > > this (all moaning is prefixed with kobject_uevent_env, so it should be
> > > easy to spot)...
> >
> > I applied the debug patch, but I don't see any error codes being returned. This time I also got the
> > General Protection Faults. An excerpt of the log is attached.
>
> Hm, I think I have an idea about what happened.
>
> The firmware class tried to suppress the first KOBJ_ADD uevent by
> returning -ENODEV in firmware_uevent if FW_STATUS_READY was not set.
> This only worked as long as the return code of kobject_uevent was not
> checked in device_add. hack-to-make-wireless-work.patch made that first
> uevent return successfully, but this possible triggered some udev rule
> too early, leading to firmware load failures.
>
> The following (completely untested) patch uses uevent_suppress to stop
> the uevent from being generated during device_add. Does this work for
> you?
>
> ---
> drivers/base/firmware_class.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- linux-2.6.orig/drivers/base/firmware_class.c
> +++ linux-2.6/drivers/base/firmware_class.c
> @@ -333,6 +333,7 @@ static int fw_register_device(struct dev
> f_dev->parent = device;
> f_dev->class = &firmware_class;
> dev_set_drvdata(f_dev, fw_priv);
> + f_dev->uevent_suppress = 1;
> retval = device_register(f_dev);
> if (retval) {
> printk(KERN_ERR "%s: device_register failed\n",
> @@ -385,6 +386,7 @@ static int fw_setup_device(struct firmwa
> set_bit(FW_STATUS_READY, &fw_priv->status);
> else
> set_bit(FW_STATUS_READY_NOHOTPLUG, &fw_priv->status);
> + f_dev->uevent_suppress = 0;
> *dev_p = f_dev;
> goto out;
hm.
Would I be right in guessing that this was all triggered by
uevent-improve-error-checking-and-handling.patch?
If so, do you think I should labour on with
uevent-improve-error-checking-and-handling.patch plus your fix, or should I
drop the lot? (I'm inclined toward the latter, but I'm still not
sure which patch(es) need to be dropped).
Thanks.
Cornelia Huck wrote:
> On Thu, 22 Mar 2007 13:55:51 -0500,
> Larry Finger <[email protected]> wrote:
>> I applied the debug patch, but I don't see any error codes being returned. This time I also got the
>> General Protection Faults. An excerpt of the log is attached.
>
> Hm, I think I have an idea about what happened.
>
> The firmware class tried to suppress the first KOBJ_ADD uevent by
> returning -ENODEV in firmware_uevent if FW_STATUS_READY was not set.
> This only worked as long as the return code of kobject_uevent was not
> checked in device_add. hack-to-make-wireless-work.patch made that first
> uevent return successfully, but this possible triggered some udev rule
> too early, leading to firmware load failures.
>
> The following (completely untested) patch uses uevent_suppress to stop
> the uevent from being generated during device_add. Does this work for
> you?
Yes it does. Good job. On my first reboot with the new code, the startup of X hung with the black
screen and the big X cursor, but that seems to have been a single occurrence. With the patch, the
bcm43xx firmware is loaded and the device is working.
Thanks,
Larry
On Wed, 28 Mar 2007 03:26:35 +0200,
Eric Rannaud <[email protected]> wrote:
> The reason for that original patch was that it is actually possible for the
> uevent functions to return -ENOMEM, the uevent buffer being statically
> allocated to BUFFER_SIZE (2048).
So maybe -ENOMEM should still be propagated? We just don't want to fail
device_add because of it.
> It used to be 1024 but that was not
> always enough and it was doubled a while ago [1]. Using add_uevent_var()
> makes this less of a problem as such an overflow should be catched
> cleanly [2].
Reminds me that I need to look into ccw_uevent :)
Mar 22 13:01:21 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:01:21 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:21 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:21 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:21 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:21 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:21 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:21 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:21 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:21 larrylap2 kernel: firmware_loading_store: unexpected value (0)
Mar 22 13:01:21 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:21 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:21 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:21 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:21 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:01:21 larrylap2 kernel: bcm43xx: Error: PCM "bcm43xx_pcm5.fw" not available or load failed.
Mar 22 13:01:21 larrylap2 kernel: bcm43xx: core_up for active 802.11 core failed (-2)
Mar 22 13:01:45 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:01:45 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:45 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:45 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:45 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:45 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:45 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:45 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:45 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:46 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:01:46 larrylap2 kernel: firmware_loading_store: unexpected value (0)
Mar 22 13:01:46 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:01:46 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:01:46 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:01:46 larrylap2 kernel: bcm43xx: Error: PCM "bcm43xx_pcm5.fw" not available or load failed.
Mar 22 13:01:46 larrylap2 kernel: bcm43xx: core_up for active 802.11 core failed (-2)
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:02:10 larrylap2 kernel: general protection fault: 0000 [2] SMP
Mar 22 13:02:10 larrylap2 kernel: last sysfs file: class/firmware/0000:01:00.0/loading
Mar 22 13:02:10 larrylap2 kernel: CPU 0
Mar 22 13:02:10 larrylap2 kernel: Modules linked in: snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device cpufreq_conservative cpufreq_ondemand cpufreq_userspace cpufreq_powersave powernow_k8 freq_table button battery ac nls_utf8 ntfs loop nfsd exportfs nfs lockd nfs_acl sunrpc sr_mod usb_storage libusual snd_hda_intel snd_hda_codec snd_pcm snd_timer snd ohci1394 sdhci ieee1394 soundcore mmc_core snd_page_alloc forcedeth ehci_hcd ide_cd cdrom ohci_hcd usbcore i2c_nforce2 bcm43xx firmware_class ieee80211softmac ieee80211 ieee80211_crypt ext3 mbcache jbd edd sg fan sata_nv libata amd74xx thermal processor sd_mod scsi_mod ide_disk ide_core
Mar 22 13:02:10 larrylap2 kernel: Pid: 4178, comm: cat Not tainted 2.6.21-rc4-mm1-mm1 #14
Mar 22 13:02:10 larrylap2 kernel: RIP: 0010:[<ffffffff80251f99>] [<ffffffff80251f99>] module_put+0x1f/0x35
Mar 22 13:02:10 larrylap2 kernel: RSP: 0018:ffff810009161ea8 EFLAGS: 00010256
Mar 22 13:02:10 larrylap2 kernel: RAX: 6b6b6b6b6b6b6dab RBX: ffff810005e0c420 RCX: ffffffff803f6183
Mar 22 13:02:10 larrylap2 kernel: RDX: 0000000000000001 RSI: ffffffff802ec409 RDI: 6b6b6b6b6b6b6b6b
Mar 22 13:02:10 larrylap2 kernel: RBP: ffff810009161ea8 R08: 0000000000000000 R09: ffff81000879a888
Mar 22 13:02:10 larrylap2 kernel: R10: 0000000000000000 R11: ffffffff8811b020 R12: ffff810007c7d000
Mar 22 13:02:10 larrylap2 kernel: R13: ffff8100089c86f8 R14: ffff8100082ba048 R15: ffff8100034017c0
Mar 22 13:02:10 larrylap2 kernel: FS: 00002adf2d6ee6f0(0000) GS:ffffffff8051e000(0000) knlGS:00000000f70776d0
Mar 22 13:02:10 larrylap2 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
Mar 22 13:02:10 larrylap2 kernel: CR2: 00002adf2d4133c0 CR3: 0000000009146000 CR4: 00000000000006e0
Mar 22 13:02:10 larrylap2 kernel: Process cat (pid: 4178, threadinfo ffff810009160000, task ffff810004d94080)
Mar 22 13:02:10 larrylap2 kernel: Stack: ffff810009161ec8 ffffffff802cf4ba 0000000000000008 ffff8100082ba048
Mar 22 13:02:10 larrylap2 kernel: ffff810009161f08 ffffffff80290a44 ffff8100086c2390 ffff8100089c86f8
Mar 22 13:02:10 larrylap2 kernel: ffff810003590368 0000000000000000 ffff8100089c86f8 00007fff7d91b2d0
Mar 22 13:02:10 larrylap2 kernel: Call Trace:
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff802cf4ba>] release+0x3a/0x49
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff80290a44>] __fput+0xca/0x180
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff80290b0e>] fput+0x14/0x16
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff8028e1aa>] filp_close+0x66/0x71
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff8028f3c2>] sys_close+0x98/0xdd
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff80209f6e>] system_call+0x7e/0x83
Mar 22 13:02:10 larrylap2 kernel:
Mar 22 13:02:10 larrylap2 kernel:
Mar 22 13:02:10 larrylap2 kernel: Code: 48 ff 08 83 3f 02 75 0c 48 8b bf 50 22 00 00 e8 08 9d fd ff
Mar 22 13:02:10 larrylap2 kernel: RIP [<ffffffff80251f99>] module_put+0x1f/0x35
Mar 22 13:02:10 larrylap2 kernel: RSP <ffff810009161ea8>
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: firmware_loading_store: unexpected value (0)
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: registering. parent: firmware, set: devices
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: 0000:01:00.0
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/class/firmware/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: fill_kobj_path: path = '/devices/pci0000:00/0000:00:02.0/0000:01:00.0'
Mar 22 13:02:10 larrylap2 kernel: kobject_uevent_env: returning 0
Mar 22 13:02:10 larrylap2 kernel: bcm43xx: Error: InitVals "bcm43xx_initval06.fw" not available or load failed.
Mar 22 13:02:10 larrylap2 kernel: bcm43xx: core_up for active 802.11 core failed (-2)
Mar 22 13:02:10 larrylap2 kernel: kobject 0000:01:00.0: cleaning up
Mar 22 13:02:10 larrylap2 kernel: general protection fault: 0000 [3] SMP
Mar 22 13:02:10 larrylap2 kernel: last sysfs file: class/firmware/0000:01:00.0/loading
Mar 22 13:02:10 larrylap2 kernel: CPU 0
Mar 22 13:02:10 larrylap2 kernel: Modules linked in: snd_pcm_oss snd_mixer_oss snd_seq snd_seq_device cpufreq_conservative cpufreq_ondemand cpufreq_userspace cpufreq_powersave powernow_k8 freq_table button battery ac nls_utf8 ntfs loop nfsd exportfs nfs lockd nfs_acl sunrpc sr_mod usb_storage libusual snd_hda_intel snd_hda_codec snd_pcm snd_timer snd ohci1394 sdhci ieee1394 soundcore mmc_core snd_page_alloc forcedeth ehci_hcd ide_cd cdrom ohci_hcd usbcore i2c_nforce2 bcm43xx firmware_class ieee80211softmac ieee80211 ieee80211_crypt ext3 mbcache jbd edd sg fan sata_nv libata amd74xx thermal processor sd_mod scsi_mod ide_disk ide_core
Mar 22 13:02:10 larrylap2 kernel: Pid: 4199, comm: cat Not tainted 2.6.21-rc4-mm1-mm1 #14
Mar 22 13:02:10 larrylap2 kernel: RIP: 0010:[<ffffffff80251f99>] [<ffffffff80251f99>] module_put+0x1f/0x35
Mar 22 13:02:10 larrylap2 kernel: RSP: 0018:ffff810008bfdea8 EFLAGS: 00010256
Mar 22 13:02:10 larrylap2 kernel: RAX: 6b6b6b6b6b6b6dab RBX: ffff8100070eb308 RCX: ffffffff803f6183
Mar 22 13:02:10 larrylap2 kernel: RDX: 0000000000000001 RSI: ffffffff802ec409 RDI: 6b6b6b6b6b6b6b6b
Mar 22 13:02:10 larrylap2 kernel: RBP: ffff810008bfdea8 R08: 0000000000000000 R09: ffff810008745338
Mar 22 13:02:10 larrylap2 kernel: R10: 0000000000000000 R11: ffffffff8811b020 R12: ffff81000999c000
Mar 22 13:02:10 larrylap2 kernel: R13: ffff8100089945a8 R14: ffff81000883e828 R15: ffff8100034017c0
Mar 22 13:02:10 larrylap2 kernel: FS: 00002b325c21c6f0(0000) GS:ffffffff8051e000(0000) knlGS:00000000f70776d0
Mar 22 13:02:10 larrylap2 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
Mar 22 13:02:10 larrylap2 kernel: CR2: 00002b325bf413c0 CR3: 0000000005a02000 CR4: 00000000000006e0
Mar 22 13:02:10 larrylap2 kernel: Process cat (pid: 4199, threadinfo ffff810008bfc000, task ffff810006b78140)
Mar 22 13:02:10 larrylap2 kernel: Stack: ffff810008bfdec8 ffffffff802cf4ba 0000000000000008 ffff81000883e828
Mar 22 13:02:10 larrylap2 kernel: ffff810008bfdf08 ffffffff80290a44 ffff8100086c26a8 ffff8100089945a8
Mar 22 13:02:10 larrylap2 kernel: ffff810009010050 0000000000000000 ffff8100089945a8 00007fff4edeb7a0
Mar 22 13:02:10 larrylap2 kernel: Call Trace:
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff802cf4ba>] release+0x3a/0x49
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff80290a44>] __fput+0xca/0x180
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff80290b0e>] fput+0x14/0x16
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff8028e1aa>] filp_close+0x66/0x71
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff8028f3c2>] sys_close+0x98/0xdd
Mar 22 13:02:10 larrylap2 kernel: [<ffffffff80209f6e>] system_call+0x7e/0x83
Mar 22 13:02:10 larrylap2 kernel:
Mar 22 13:02:10 larrylap2 kernel:
Mar 22 13:02:10 larrylap2 kernel: Code: 48 ff 08 83 3f 02 75 0c 48 8b bf 50 22 00 00 e8 08 9d fd ff
Mar 22 13:02:10 larrylap2 kernel: RIP [<ffffffff80251f99>] module_put+0x1f/0x35
Mar 22 13:02:10 larrylap2 kernel: RSP <ffff810008bfdea8>
On Tue, 27 Mar 2007 11:25:57 +0200,
"Kay Sievers" <[email protected]> wrote:
> I don't see any point in deregistering a kernel device, if the event
> to userspace goes wrong, or a subsytem returns a non-zero value in the
> filter.
But if we filter the event, we just return 0?
> Checking the uevent return value, will not prevent any malfunction,
> usually this kind of "error handling" just prevents bringing up a
> whole subsystem, or booting-up a box, because the needed device does
> not exist at all.
OK, if we consider uevents to be non-vital to a functioning device.
OTOH, I think using something like uevent_suppress (maybe via
dev_uevent_filter?) is a saner way to suppress a uevent than to return
an error code in the uevent function.
On Mon, Mar 26, 2007 at 01:22:32AM -0800, Andrew Morton wrote:
> On Mon, 26 Mar 2007 11:09:49 +0200 Cornelia Huck <[email protected]> wrote:
> > > If so, do you think I should labour on with
> > > uevent-improve-error-checking-and-handling.patch plus your fix, or should I
> > > drop the lot? (I'm inclined toward the latter, but I'm still not
> > > sure which patch(es) need to be dropped).
> >
> > This depends on what semantics uevent returning an error code should
> > have. The firmware code was using it to suppress uevents, but
> > uevent_suppress is a better idea now. So if we want uevent returning !=
> > 0 to imply "something really bad happened", all uevent functions have
> > to be audited and those that work like firmware_uevent have to be
> > converted to uevent_suppress. This would be cleaner, but I'm not sure
> > it's worth the work.
>
> We're generally struggling to stay alive amongst all the bugs at present -
> I'll drop all those patches.
My mistake, I wrote the guilty patch
uevent-improve-error-checking-and-handling.patch assuming it was safe to
treat the return value as an error code, since several uevent functions
returns things like -ENOMEM.
Should I rework the patch as Cornelia suggests and resubmit later, when
things have settled down a little?
Thanks.