Re: [broken-out-2007-07-20-00-22] kernel bug at kernel/params:570

[Satyam Sharma]

On 7/21/07, Satyam Sharma <satyam.sharma@xxxxxxxxx> wrote:
> Hopefully this bug should be 100% reproducible at boot time anyway.
> Don't care much for XFS and unionfs, but hoping deselecting ATA from
> the config doesn't change the variables much in this equation. ]


Gargh! My system obviously cannot boot without libata. Guess it's
time to go through git log and see how to fix that build breakage
myself ...

Michal, how did you even manage to build / boot this kernel!


> On 7/21/07, Greg KH <gregkh@xxxxxxx> wrote:
> > On Fri, Jul 20, 2007 at 06:37:33PM -0700, Andrew Morton wrote:
> > > On Fri, 20 Jul 2007 18:02:57 -0700 Greg KH <gregkh@xxxxxxx> wrote:
> > >
> > > > --- a/kernel/params.c
> > > > +++ b/kernel/params.c
> > > > @@ -567,7 +567,11 @@ static void __init kernel_param_sysfs_se
> > > >     kobject_set_name(&mk->kobj, name);
> > > >     kobject_init(&mk->kobj);
> > > >     ret = kobject_add(&mk->kobj);
> > > > -   BUG_ON(ret < 0);
> > > > +   if (ret) {
> > > > +           printk(KERN_ERR "module '%s' failed to be added to sysfs, "
> > > > +                   "the system will be unstable now.\n", name);
> > > > +           return;
> > > > +   }
> > >
> > > It would be nice to print the value of `ret' too.
>
>
> What I'm surprised about is that %eax doesn't seem to contain the
> return value `ret' of kobject_add(). It's 1, which is funny, given:
>
> ret = kobject_add(&mk->kobj);
> BUG_ON(ret < 0);
>
> One wouldn't expect BUG() -- or the corresponding exception handler --
> to clobber registers, that would be a sad day.


But I cracked this one alright. His .config has CONFIG_PROFILE_LIKELY=y
which replaces unlikely() / likely() with do_check_likely() and forces
gcc to clobber %eax with the condition itself, which in our case was
(ret < 0) == TRUE, and thus, the "1" value we saw in %eax in the
register dumps.

We should probably document somewhere that CONFIG_PROFILE_LIKELY
is not good for debugging.

Hmmm ... thinking out aloud here, but probably I don't need to fix that
libata breakage at all. I'll just put the BUG_ON(ret < 0) back in the
code, deselect PROFILE_LIKELY, and this time we _will_ have the
return of kobject_add() in %eax ...


That'll at least clear up the EEXIST vs EINVAL mystery, that'll be a
good data point, yes.

Anyway, I guess I must stop my running commentary -- will only post
after this is cleared up now :-)

Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/