Prev: staging/rar_register: depends on PCI
Next: Your Webmail Quota Has Exceeded The Set Quota Please Upgrade
From: James Bottomley on 6 May 2010 11:00
On Thu, 2010-05-06 at 16:24 +0200, Michal Marek wrote:
> On 5.5.2010 23:49, James Bottomley wrote:
> > [Sam: I know you don't maintain kbuild anymore, but since you have the
> > most experience, if you could find time to comment, I'd be grateful]
> >
> > 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.
> >
> > There seems to be a fairly easy way to do this in kbuild. Right at the
> > moment, select is handled as additional symbol values as the last point
> > in the symbol tree evaluation process. Instead, what I propose doing is
> > for every select symbol, we add an extra unconditional default for the
> > selected symbol of the selecting symbol's current value (this breaks a
> > possible dependency cycle) and add to the dependencies of the selecting
> > symbol, the symbol it's currently selecting.
>
> Nice trick :-).
>
>
> > There's one wrinkle to all of this in that the current parser for
> > default values stops when it finds the first valid (i.e. whose if clause
> > evaluates to true) default. To make the above scheme work, I need to
> > modify the default parser so it takes the highest tristate of all the
> > valid defaults (and bumps m to y for bool).
>
> We should check if some Kconfig file doesn't rely on this "first hit"
> behavior and fix it to explicitly list the condition for a given
> default.

I actually asked kconfig to generate the list of symbols (in my config)
with multiple defaults. It's pretty small and the default y seems to be
the thing with multiple if clauses, so they act like or statements.

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

> Another option would be to add
> default SYM1 || SYM2
> to a symbol selected by SYM1 and SYM2.

Well, that's effectively what the proposal does (it or's the states).

> > Does this look acceptable to people? I think it should give the desired
> > result and has the added benefit that we can then strip the extra select
> > overlay out of the kbuild system (making the parser slightly simpler).
> >
> > If this looks like a good idea to people, I think I can code up a quick
> > patch.
>
> Other than the above, right now I don't see any issues with such approach.
>
> On a related note, I see Vegard's GSoC project to use a sat solver for
> kconfig got accepted [1]. Vegard, how is the project progressing?
>
> [1]
> http://socghop.appspot.com/gsoc/student_project/show/google/gsoc2010/psu_home/t127230762803
>
> Michal

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: Valdis.Kletnieks on 6 May 2010 12:50
On Thu, 06 May 2010 09:17:24 EDT, James Bottomley said:
> On Thu, 2010-05-06 at 08:47 +0200, Geert Uytterhoeven wrote:
> > On Wed, May 5, 2010 at 23:49, James Bottomley
> > <James.Bottomley(a)hansenpartnership.com> wrote:
> > > [Sam: I know you don't maintain kbuild anymore, but since you have the
> > > most experience, if you could find time to comment, I'd be grateful]
> > >
> > > 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.
> >
> > What if there's a good reason the selected symbol has this dependency?
> > E.g. it depends on a critical feature not available? Like CONFIG_HAS_IOMEM?
>
> I don't quite understand the question. If a selected symbol has a
> critical dependency which is config'd to N then the build usually
> breaks ... that's what I'm calling the select problem. I thought
> CONFIG_HAS_IOMEM was usually selected by the architecture, though. In
> the new proposal, we wouldn't be able to generate the invalid
> configuration in the first place.

I think Geert is asking "If the arch says CONFIG_HAS_IOMEM=n, but some driver
does a 'select CONFIG_FOO' which then (under your proposal) forces the
value CONFIG_BAR=y, which eventually ends up with CONFIG_HAS_IOMEM=y, what
should the behavior be?"

(I suspect the right answer here is "one of the symbols is buggy and its
'select' should be a 'depends' instead", but somebody else better double-check
that conclusion - I'm hardly a Kconfig expert).
From: Vegard Nossum on 6 May 2010 13:00
On 6 May 2010 16:24, Michal Marek <mmarek(a)suse.cz> wrote:
> On a related note, I see Vegard's GSoC project to use a sat solver for
> kconfig got accepted [1]. Vegard, how is the project progressing?
>
> [1]
> http://socghop.appspot.com/gsoc/student_project/show/google/gsoc2010/psu_home/t127230762803

Hi,

Yes, that is true :-) I was planning to send an announcement to LKML
this weekend. Actual coding doesn't start until the last week of May.

So hopefully we can soon get rid of select altogether :-)


Vegard
--
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: Geert Uytterhoeven on 6 May 2010 13:30
On Thu, May 6, 2010 at 18:47, <Valdis.Kletnieks(a)vt.edu> wrote:
> On Thu, 06 May 2010 09:17:24 EDT, James Bottomley said:
>> On Thu, 2010-05-06 at 08:47 +0200, Geert Uytterhoeven wrote:
>> > On Wed, May 5, 2010 at 23:49, James Bottomley
>> > <James.Bottomley(a)hansenpartnership.com> wrote:
>> > > [Sam: I know you don't maintain kbuild anymore, but since you have the
>> > > most experience, if you could find time to comment, I'd be grateful]
>> > >
>> > > 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.
>> >
>> > What if there's a good reason the selected symbol has this dependency?
>> > E.g. it depends on a critical feature not available? Like CONFIG_HAS_IOMEM?
>>
>> I don't quite understand the question.  If a selected symbol has a
>> critical dependency which is config'd to N then the build usually
>> breaks ... that's what I'm calling the select problem.  I thought
>> CONFIG_HAS_IOMEM was usually selected by the architecture, though.  In
>> the new proposal, we wouldn't be able to generate the invalid
>> configuration in the first place.
>
> I think Geert is asking "If the arch says CONFIG_HAS_IOMEM=n, but some driver
> does a 'select CONFIG_FOO' which then (under your proposal) forces the
> value CONFIG_BAR=y, which eventually ends up with CONFIG_HAS_IOMEM=y, what
> should the behavior be?"

Yep, that was my (bogus) question...

> (I suspect the right answer here is "one of the symbols is buggy and its
> 'select' should be a 'depends' instead", but somebody else better double-check
> that conclusion - I'm hardly a Kconfig expert).

No, the CONFIG_FOO will inherit the dependency on CONFIG_HAS_IOMEM.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
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: James Bottomley on 6 May 2010 16:50
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.

James

---

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)


--
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/
First  |  Prev  |  Next  |  Last
Pages: 1 2 3
Prev: staging/rar_register: depends on PCI
Next: Your Webmail Quota Has Exceeded The Set Quota Please Upgrade