From: James Bottomley on
On Thu, 2010-05-06 at 13:59 -0700, Randy Dunlap wrote:
> On Thu, 06 May 2010 15:48:35 -0500 James Bottomley wrote:
>
> > On Thu, 2010-05-06 at 09:52 -0500, James Bottomley wrote:
> > > The list is
> > >
> > > USB_ARCH_HAS_HCD has 4 defaults
> > > DEFCONFIG_LIST has 5 defaults
> > > MAC80211_RC_DEFAULT has 2 defaults
> > > X86_L1_CACHE_SHIFT has 2 defaults
> > > SPLIT_PTLOCK_CPUS has 2 defaults
> > > X86_MINIMUM_CPU_FAMILY has 3 defaults
> > > DEFAULT_TCP_CONG has 2 defaults
> > > DEFCONFIG_LIST has 5 defaults
> > > USB_ARCH_HAS_HCD has 4 defaults
> > > X86_L1_CACHE_SHIFT has 2 defaults
> > > X86_MINIMUM_CPU_FAMILY has 3 defaults
> > > SPLIT_PTLOCK_CPUS has 2 defaults
> > > DEFAULT_TCP_CONG has 2 defaults
> > > MAC80211_RC_DEFAULT has 2 defaults
> >
> > Here's a patch that alters the default processing to the needed form and
> > checks the old vs new values. I've been running randconfigs but I can't
> > get the warning to trip ... have at it.
> >
>
> I don't see the warning, but I took a problem config from linux-next 2010-0506
> (today) [attached]. In it, CONFIG_TCG_BIOS_LOG=y and that selects ACPI,
> so now ACPI is enabled, but ACPI depends on PM and PCI, but they are still not
> selected...
>
> or is this only step 1 of a multi-step implementation?

Actually, it's not any step. It's just validation of the contention
that I can alter the default parser from taking the first valid default
to calculating the or of the defaults without changing the current
kbuild behaviour.

The patch to fix kbuild goes on top of this change minus the warnings
(when I actually code it up).

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Randy Dunlap on
On Thu, 06 May 2010 15:48:35 -0500 James Bottomley wrote:

> On Thu, 2010-05-06 at 09:52 -0500, James Bottomley wrote:
> > The list is
> >
> > USB_ARCH_HAS_HCD has 4 defaults
> > DEFCONFIG_LIST has 5 defaults
> > MAC80211_RC_DEFAULT has 2 defaults
> > X86_L1_CACHE_SHIFT has 2 defaults
> > SPLIT_PTLOCK_CPUS has 2 defaults
> > X86_MINIMUM_CPU_FAMILY has 3 defaults
> > DEFAULT_TCP_CONG has 2 defaults
> > DEFCONFIG_LIST has 5 defaults
> > USB_ARCH_HAS_HCD has 4 defaults
> > X86_L1_CACHE_SHIFT has 2 defaults
> > X86_MINIMUM_CPU_FAMILY has 3 defaults
> > SPLIT_PTLOCK_CPUS has 2 defaults
> > DEFAULT_TCP_CONG has 2 defaults
> > MAC80211_RC_DEFAULT has 2 defaults
>
> Here's a patch that alters the default processing to the needed form and
> checks the old vs new values. I've been running randconfigs but I can't
> get the warning to trip ... have at it.
>

I don't see the warning, but I took a problem config from linux-next 2010-0506
(today) [attached]. In it, CONFIG_TCG_BIOS_LOG=y and that selects ACPI,
so now ACPI is enabled, but ACPI depends on PM and PCI, but they are still not
selected...

or is this only step 1 of a multi-step implementation?

>
> ---
>
> diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
> index 6c8fbbb..722bc4e 100644
> --- a/scripts/kconfig/symbol.c
> +++ b/scripts/kconfig/symbol.c
> @@ -115,14 +115,31 @@ struct property *sym_get_env_prop(struct symbol *sym)
>
> struct property *sym_get_default_prop(struct symbol *sym)
> {
> - struct property *prop;
> + struct property *prop, *ret = NULL;
> + tristate old_val = no, val = no;
>
> for_all_defaults(sym, prop) {
> prop->visible.tri = expr_calc_value(prop->visible.expr);
> - if (prop->visible.tri != no)
> - return prop;
> + if (prop->visible.tri != no) {
> + tristate v = expr_calc_value(prop->expr);
> + if (!ret)
> + old_val = v;
> + if (v >= val) {
> + val = v;
> + ret = prop;
> + }
> + }
> }
> - return NULL;
> + /*
> + * Previously, we took the first valid default we found (this
> + * is now old_val). In the new scheme, the value is the or of
> + * all the defaults.
> + */
> + if (old_val != val)
> + menu_warn(ret->menu, "ERROR: new parser has inconsistent "
> + "values for %s (%d != %d)\n", sym->name,
> + old_val, val);
> + return ret;
> }
>
> static struct property *sym_get_range_prop(struct symbol *sym)

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
From: Catalin Marinas on
James Bottomley <James.Bottomley(a)HansenPartnership.com> wrote:
> The select problem is that the kbuild select directive will turn a
> symbol on without reference to its dependencies. This, in turn, means
> that either selected symbols must select their dependencies, or that
> people using select have to be aware of the selected symbol's dependency
> and build those dependencies into their symbol (leading to duplication
> and the possibility of getting the dependencies out of sync). We use
> select for the scsi transport classes, so we run into this problem in
> SCSI quite a lot.
>
> I think the correct fix is to make a symbol that selects another symbol
> automatically inherit all of the selected symbol's dependencies.

You may want to have a look at a patch I proposed some time ago, which
at just warns of selecting options with unmet dependencies. I think it's
a better approach that forcing the selection of the dependencies:

http://lkml.org/lkml/2009/9/16/413

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