Re: [PATCH -next] thermal: depends on NET

Re: [PATCH -next] thermal: depends on NET

[Randy Dunlap]

On Mon, 13 Dec 2010 11:20:33 -0800 Randy Dunlap wrote:

This is still needed in linux-next 20110110.

Adding linux-acpi.

Ping.  Is anybody there?  It has only been 4 weeks now with no reply...



> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> ERROR: "genl_register_family" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "genl_unregister_family" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "init_net" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "__alloc_skb" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "netlink_broadcast" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "kfree_skb" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "genl_register_mc_group" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "nla_reserve" [drivers/thermal/thermal_sys.ko] undefined!
> ERROR: "skb_put" [drivers/thermal/thermal_sys.ko] undefined!
> 
> Reported-by: Zimny Lech <napohybelskurwysynom2010@gmail.com>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> ---
>  drivers/thermal/Kconfig |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- linux-next-20101213.orig/drivers/thermal/Kconfig
> +++ linux-next-20101213/drivers/thermal/Kconfig
> @@ -4,6 +4,7 @@
>  
>  menuconfig THERMAL
>  	tristate "Generic Thermal sysfs driver"
> +	depends on NET
>  	help
>  	  Generic Thermal Sysfs driver offers a generic mechanism for
>  	  thermal management. Usually it's made up of one or more thermal
> --


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH -next] thermal: depends on NET

[Len Brown]

> > --- linux-next-20101213.orig/drivers/thermal/Kconfig
> > +++ linux-next-20101213/drivers/thermal/Kconfig
> > @@ -4,6 +4,7 @@
> >  
> >  menuconfig THERMAL
> >  	tristate "Generic Thermal sysfs driver"
> > +	depends on NET

I've added this line to the offending patch.

While I agree that randconfig build testing
is theoretically useful, in recent memory
its results do not seem particularly relevant
to useful configs.

Perhaps it would be a good idea to spend some time
making non-useful configs impossible, and thus focus
the testing where it will be of more benefit?

thanks,
Len Brown, Intel Open Source Technology Center

on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Randy Dunlap]

On 01/11/11 21:18, Len Brown wrote:
>>> --- linux-next-20101213.orig/drivers/thermal/Kconfig
>>> +++ linux-next-20101213/drivers/thermal/Kconfig
>>> @@ -4,6 +4,7 @@
>>>  
>>>  menuconfig THERMAL
>>>  	tristate "Generic Thermal sysfs driver"
>>> +	depends on NET
> 
> I've added this line to the offending patch.

Thank you.

> While I agree that randconfig build testing
> is theoretically useful, in recent memory
> its results do not seem particularly relevant
> to useful configs.

Who defines useful?
"what is good for the goose is not good for the gander"

> Perhaps it would be a good idea to spend some time
> making non-useful configs impossible, and thus focus
> the testing where it will be of more benefit?

We have a plethora of kernel configs, so yes,
I'd be glad to see your efforts in that area.


Here's my take on kernel builds:

Ideally (randconfig) build testing wouldn't be needed
and developers would:

- know what kernel facilities their code uses and #include
  header files for all of them

- know what kernel configs their code uses and make their code
  depend on or select the needed config symbols

- actually read & review build output to look for errors and
  warnings in their code and not ignore them but actually fix them

- use sparse to check for other warnings

The current attitude of "if it builds, then it must be OK"
is not good.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Len Brown]

> > While I agree that randconfig build testing
> > is theoretically useful, in recent memory
> > its results do not seem particularly relevant
> > to useful configs.
> 
> Who defines useful?

Simple.

Configs that will be used are useful.

Phantom configs that will NEVER be used are NOT useful.

Phantom configs are HARMFUL, as they squander
finite testing and maintainer resources
that should be applied to code that is actually used.

Rather than celebrating our theoretical flexibility
with every new config option, we should recoil at the
fact that each one may up to double the number of
configs that need to be tested and supported.

When I'm answering your nagging e-mail about a build failure
in a phantom config that nobody would even conceive of using,
I'm not using that time to fix somebody's real problem
on a real machine.

I'd rather see you spend your time making select work
to delete an entire category of Kconfig failures,
or simply adding dependencies making phantom configs impossible.

eg. Look in drivers/acpi/Kconfig:

menuconfig ACPI
        bool "ACPI (Advanced Configuration and Power Interface) Support"
        depends on !IA64_HP_SIM
        depends on IA64 || X86
        depends on PCI
        depends on PM
        select PNP

Does all of ACPI technically depend on PCI?
Does all of ACPI technically depend on PM support?
Does all of ACPI technically depend on configuration and PNP?

Theoretically, no.

Do I care about the phantom configs that would be possible
if these false dependencies were not in place.  No,
not until somebody invents such a system,
and may be not even then.

Is there a user out there on LKML who can dream up
a use for one of these phantom configs and claim that
his life will end if he'd prevented from building it?
Sure.  Does he suffer from a total lack of perspective?
Yes.

-Len Brown, Intel Open Source Technology Center

Re: on builds/randconfigs

[Randy Dunlap]

On 01/12/11 10:35, Len Brown wrote:
>>> While I agree that randconfig build testing
>>> is theoretically useful, in recent memory
>>> its results do not seem particularly relevant
>>> to useful configs.
>>
>> Who defines useful?
> 
> Simple.
> 
> Configs that will be used are useful.

Then I suggest that you start deleting non-useful kernel configs.
You can begin in ACPI.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: on builds/randconfigs

[Len Brown]

> >> Who defines useful?

> > Configs that will be used are useful.

> Then I suggest that you start deleting non-useful kernel configs.
> You can begin in ACPI.

Maybe I wasn't clear in my previous note...

Originally, ACPI did not depend on PCI, PNP, or PM.
Now it does.  That is an 8x reduction in phantom configs.

I'm all ears for ideas on further simplification.

-Len

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Andrew Morton]

On Wed, 12 Jan 2011 13:35:10 -0500 (EST)
Len Brown <lenb@kernel.org> wrote:

> Do I care about the phantom configs that would be possible
> if these false dependencies were not in place.  No,
> not until somebody invents such a system,
> and may be not even then.
> 
> Is there a user out there on LKML who can dream up
> a use for one of these phantom configs and claim that
> his life will end if he'd prevented from building it?
> Sure.  Does he suffer from a total lack of perspective?
> Yes.

These unusable config combinations should be prevented via Kconfig. 
That prevents users from selecting them, which otherwise adds to
our workload and to theirs.  It also prevents false-positives
during our useful randconfig testing.

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Rafael J. Wysocki]

On Wednesday, January 12, 2011, Andrew Morton wrote:
> On Wed, 12 Jan 2011 13:35:10 -0500 (EST)
> Len Brown <lenb@kernel.org> wrote:
> 
> > Do I care about the phantom configs that would be possible
> > if these false dependencies were not in place.  No,
> > not until somebody invents such a system,
> > and may be not even then.
> > 
> > Is there a user out there on LKML who can dream up
> > a use for one of these phantom configs and claim that
> > his life will end if he'd prevented from building it?
> > Sure.  Does he suffer from a total lack of perspective?
> > Yes.
> 
> These unusable config combinations should be prevented via Kconfig. 
> That prevents users from selecting them, which otherwise adds to
> our workload and to theirs.  It also prevents false-positives
> during our useful randconfig testing.

But it is kind of difficult to achieve IMhO.  For example, there are options
that are only SELECTed if something else is set, but randconfig doesn't seem
to care.

Thanks,
Rafael

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Len Brown]

> > These unusable config combinations should be prevented via Kconfig. 
> > That prevents users from selecting them, which otherwise adds to
> > our workload and to theirs.  It also prevents false-positives
> > during our useful randconfig testing.
> 
> But it is kind of difficult to achieve IMhO.  For example, there are options
> that are only SELECTed if something else is set, but randconfig doesn't seem
> to care.

Kconfig select needs to be fixed so that it is not possible to
select something if that something's dependencies are not met.

This long standing issue is independent of randconfig,
but Randy can probably attest that a measurable portion
of the randconfig failures are due to this bug.

-Len Brown, Intel Open Source Technology Center

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Valdis.Kletnieks]

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

On Wed, 12 Jan 2011 16:58:26 EST, Len Brown said:

> Kconfig select needs to be fixed so that it is not possible to
> select something if that something's dependencies are not met.

Amen to that. Got bit by that in the CONFIG_XT_MATCH_REALM thread, it was
trying to do a select on NET_CLS_ROUTE, which depended on NET_SCHED which was
unset...


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

Re: on builds/randconfigs

[Randy Dunlap]

On 01/12/11 13:58, Len Brown wrote:
>>> These unusable config combinations should be prevented via Kconfig. 
>>> That prevents users from selecting them, which otherwise adds to
>>> our workload and to theirs.  It also prevents false-positives
>>> during our useful randconfig testing.
>>
>> But it is kind of difficult to achieve IMhO.  For example, there are options
>> that are only SELECTed if something else is set, but randconfig doesn't seem
>> to care.

s/randconfig/kconfig/


> Kconfig select needs to be fixed so that it is not possible to
> select something if that something's dependencies are not met.
> 
> This long standing issue is independent of randconfig,
> but Randy can probably attest that a measurable portion
> of the randconfig failures are due to this bug.

Certainly.  At least now we have a kconfig warning on this class of
problem.

-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Stephen Rothwell]

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

Hi all,

On Wed, 12 Jan 2011 16:58:26 -0500 (EST) Len Brown <lenb@kernel.org> wrote:
>
> Kconfig select needs to be fixed so that it is not possible to
> select something if that something's dependencies are not met.

So, should the config step fail if such a thing is attempted, or just not
honour the "select"?

BTW, Linus asked me to investigate the alternative of having "select"
(recursively) automatically set the dependencies if they are not already
set ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Len Brown]

On Thu, 13 Jan 2011, Stephen Rothwell wrote:

> Hi all,
> 
> On Wed, 12 Jan 2011 16:58:26 -0500 (EST) Len Brown <lenb@kernel.org> wrote:
> >
> > Kconfig select needs to be fixed so that it is not possible to
> > select something if that something's dependencies are not met.
> 
> So, should the config step fail if such a thing is attempted, or just not
> honour the "select"?
> 
> BTW, Linus asked me to investigate the alternative of having "select"
> (recursively) automatically set the dependencies if they are not already
> set ...

I expect that users want to interact only with select.

See feature X, enable feature X, and feature X
selects what is necessary to support what the user requested...

user interact with "depends-on", by comparison, is torture.
Users are not offered options unless the options
already have all of their dependencies met.
If they manually edit the config file and enable such
options, make oldconfig magically throws away their choices.
That drives people crazy.

cheers,
Len Brown, Intel Open Source Technology Center

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Steven Rostedt]

On Thu, 2011-01-13 at 11:44 +1100, Stephen Rothwell wrote:
> Hi all,
> 
> On Wed, 12 Jan 2011 16:58:26 -0500 (EST) Len Brown <lenb@kernel.org> wrote:
> >
> > Kconfig select needs to be fixed so that it is not possible to
> > select something if that something's dependencies are not met.
> 
> So, should the config step fail if such a thing is attempted, or just not
> honour the "select"?
> 
> BTW, Linus asked me to investigate the alternative of having "select"
> (recursively) automatically set the dependencies if they are not already
> set ...
> 

What happens if you have foo depends on bar || fib ?

Do you automatically enable bar and fib?

-- Steve

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Stephen Rothwell]

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

Hi Steve,

On Wed, 12 Jan 2011 22:45:40 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
>
> What happens if you have foo depends on bar || fib ?
> 
> Do you automatically enable bar and fib?

The plan was to error out and let the user decide ... or behave as we do
and just warn?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

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

Re: on builds/randconfigs (was: [PATCH -next] thermal: depends on NET)

[Linus Torvalds]

On Wed, Jan 12, 2011 at 7:48 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Steve,
>
> On Wed, 12 Jan 2011 22:45:40 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>> What happens if you have foo depends on bar || fib ?
>>
>> Do you automatically enable bar and fib?
>
> The plan was to error out and let the user decide ... or behave as we do
> and just warn?

One thing to notice is that a lot of the "a || b" conditionals tend to
be cases where one of the expressions is a known constant 'y'.

In other words, the most common case of "a || b" tends to be things
like "X86 || MIPS || POWER.." or possibly "PCI || ISA", where one of
the options will have been hard-coded by the initial architecture
config file - and would generally not be something that the user sets
at all.

Now that may not be _universally_ true, but a quick grep certainly
supports the notion that that is a very common case.

So I think we want the rule to be:

 - check all entries in the "||" chain - and if one of them is already
set to 'y', then just ignore the dependency as trivially fulfilled

 - if all are 'n' or unknown, error out (or warn)

and I think there won't be all that many cases where people get the
error in practice.

                      Linus

Re: on builds/randconfigs

[Michal Marek]

On 12.1.2011 22:58, Len Brown wrote:
>>> These unusable config combinations should be prevented via Kconfig. 
>>> That prevents users from selecting them, which otherwise adds to
>>> our workload and to theirs.  It also prevents false-positives
>>> during our useful randconfig testing.
>>
>> But it is kind of difficult to achieve IMhO.  For example, there are options
>> that are only SELECTed if something else is set, but randconfig doesn't seem
>> to care.
> 
> Kconfig select needs to be fixed so that it is not possible to
> select something if that something's dependencies are not met.

Right now, it issues a warning in such case. I think changing it to a
fatal error would be too premature, not long ago there were a couple of
annoying false positives.

But from the rest of the thread, I conclude that you actually meant "not
possible to select something if that something's dependencies CANNOT be
met", i.e. automatically select dependencies if that is possible. That
was actually one of the goals of Vegard Nossum's GSoC poject last year,
but I haven't heard of any outcome yet. Vegard, is there something we
could use, be it code or mistakes we could learn from?

Thanks,
Michal

Re: on builds/randconfigs

[Vegard Nossum]

On 13 January 2011 13:23, Michal Marek <mmarek@suse.cz> wrote:
> On 12.1.2011 22:58, Len Brown wrote:
>>>> These unusable config combinations should be prevented via Kconfig.
>>>> That prevents users from selecting them, which otherwise adds to
>>>> our workload and to theirs.  It also prevents false-positives
>>>> during our useful randconfig testing.
>>>
>>> But it is kind of difficult to achieve IMhO.  For example, there are options
>>> that are only SELECTed if something else is set, but randconfig doesn't seem
>>> to care.
>>
>> Kconfig select needs to be fixed so that it is not possible to
>> select something if that something's dependencies are not met.
>
> Right now, it issues a warning in such case. I think changing it to a
> fatal error would be too premature, not long ago there were a couple of
> annoying false positives.
>
> But from the rest of the thread, I conclude that you actually meant "not
> possible to select something if that something's dependencies CANNOT be
> met", i.e. automatically select dependencies if that is possible. That
> was actually one of the goals of Vegard Nossum's GSoC poject last year,
> but I haven't heard of any outcome yet. Vegard, is there something we
> could use, be it code or mistakes we could learn from?

Maybe I am wrong, but doesn't conf_write() actually take care of this
when it does that sym_calc_value() for all the symbols? Or maybe
that's the problem (i.e. that it doesn't).

Current satconfig code can be changed to produce random configs (that
nevertheless respect the user's choices) with a 1-line patch. But
there are some other issues to be worked out, mainly that conf_write()
doesn't respect all the choices that the SAT solver made. There could
be several reasons for that: 1. satconfig doesn't deal with
hex/int/string values and relies on conf_write() to fill in those
(this is a plain old bug), 2. the translation into boolean logic is
subtly wrong, perhaps a corner case or something like that, and/or 3.
conf_write() does something wrong.

I'll be happy to give a tour of the satconfig code if you or anybody
else would like to start hacking on it.


Vegard

Re: on builds/randconfigs

[Michal Marek]

Dne 15.1.2011 13:24, Vegard Nossum napsal(a):
> I'll be happy to give a tour of the satconfig code if you or anybody
> else would like to start hacking on it.

Sure, I'm interested!

Michal