Hi,
The first patch in this series adds a warning to the kbuild system when
options that are selected have unmet dependencies.
The subsequent patches show two simple examples that were spotted by the
first patch.
In the view of the ARM defconfig story, such patch would help with
ensuring that a platform Kconfig selects the necessary options.
Thanks.
Catalin Marinas (3):
kbuild: Warn on selecting symbols with unmet direct dependencies
ARM: Mark CPU_32v6K as depended on CPU_V7
Change DEBUG_PAGEALLOC dependency on ARCH_SUPPORTS_DEBUG_PAGEALLOC
arch/arm/mm/Kconfig | 2 +-
mm/Kconfig.debug | 4 ++--
scripts/kconfig/expr.h | 2 ++
scripts/kconfig/menu.c | 5 +++++
scripts/kconfig/symbol.c | 18 ++++++++++++++++++
5 files changed, 28 insertions(+), 3 deletions(-)
--
Catalin
The "select" statement in Kconfig files allows the enabling of options
even if they have unmet direct dependencies (i.e. "depends on" expands
to "no"). Currently, the "depends on" clauses are used in calculating
the visibility but they do not affect the reverse dependencies in any
way.
The patch introduces additional tracking of the "depends on" statements
and prints a warning on selecting an option if its direct dependencies
are not met.
Signed-off-by: Catalin Marinas <[email protected]>
Cc: Sam Ravnborg <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
scripts/kconfig/expr.h | 2 ++
scripts/kconfig/menu.c | 5 +++++
scripts/kconfig/symbol.c | 18 ++++++++++++++++++
3 files changed, 25 insertions(+), 0 deletions(-)
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index 6408fef..c8be420 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -83,6 +83,7 @@ struct symbol {
tristate visible;
int flags;
struct property *prop;
+ struct expr_value dir_dep;
struct expr_value rev_dep;
};
@@ -164,6 +165,7 @@ struct menu {
struct symbol *sym;
struct property *prompt;
struct expr *dep;
+ struct expr *dir_dep;
unsigned int flags;
char *help;
struct file *file;
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 059a246..8519f21 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -105,6 +105,7 @@ static struct expr *menu_check_dep(struct expr *e)
void menu_add_dep(struct expr *dep)
{
current_entry->dep = expr_alloc_and(current_entry->dep, menu_check_dep(dep));
+ current_entry->dir_dep = current_entry->dep;
}
void menu_set_type(int type)
@@ -288,6 +289,10 @@ void menu_finalize(struct menu *parent)
for (menu = parent->list; menu; menu = menu->next)
menu_finalize(menu);
} else if (sym) {
+ /* ignore inherited dependencies for dir_dep */
+ sym->dir_dep.expr = expr_transform(expr_copy(parent->dir_dep));
+ sym->dir_dep.expr = expr_eliminate_dups(sym->dir_dep.expr);
+
basedep = parent->prompt ? parent->prompt->visible.expr : NULL;
basedep = expr_trans_compare(basedep, E_UNEQUAL, &symbol_no);
basedep = expr_eliminate_dups(expr_transform(basedep));
diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c
index 6c8fbbb..7445cc0 100644
--- a/scripts/kconfig/symbol.c
+++ b/scripts/kconfig/symbol.c
@@ -205,6 +205,16 @@ static void sym_calc_visibility(struct symbol *sym)
}
if (sym_is_choice_value(sym))
return;
+ /* defaulting to "yes" if no explicit "depends on" are given */
+ tri = yes;
+ if (sym->dir_dep.expr)
+ tri = expr_calc_value(sym->dir_dep.expr);
+ if (tri == mod)
+ tri = yes;
+ if (sym->dir_dep.tri != tri) {
+ sym->dir_dep.tri = tri;
+ sym_set_changed(sym);
+ }
tri = no;
if (sym->rev_dep.expr)
tri = expr_calc_value(sym->rev_dep.expr);
@@ -321,6 +331,14 @@ void sym_calc_value(struct symbol *sym)
}
}
calc_newval:
+ if (sym->dir_dep.tri == no && sym->rev_dep.tri != no) {
+ fprintf(stderr, "warning: (");
+ expr_fprint(sym->rev_dep.expr, stderr);
+ fprintf(stderr, ") selects %s which has unmet direct dependencies (",
+ sym->name);
+ expr_fprint(sym->dir_dep.expr, stderr);
+ fprintf(stderr, ")\n");
+ }
newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri);
}
if (newval.tri == mod && sym_get_type(sym) == S_BOOLEAN)
CPU_32v6K is selected by CPU_V7 but it only depends on CPU_V6.
Signed-off-by: Catalin Marinas <[email protected]>
Cc: Russell King <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
arch/arm/mm/Kconfig | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index 9f10a9b..a4909db 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -398,7 +398,7 @@ config CPU_V6
# ARMv6k
config CPU_32v6K
bool "Support ARM V6K processor extensions" if !SMP
- depends on CPU_V6
+ depends on CPU_V6 || CPU_V7
default y if SMP && !(ARCH_MX3 || ARCH_OMAP2)
help
Say Y here if your ARMv6 processor supports the 'K' extension.
DEBUG_PAGEALLOC currently depends on ARCH_SUPPORTS_DEBUG_PAGEALLOC.
However, it is selected by PAGE_POISONING which depends on
!ARCH_SUPPORTS_DEBUG_PAGEALLOC.
This patch changes the DEBUG_PAGEALLOC entry so that the dependency is
moved to the definition line rather than having an explicit "depends on"
line.
Signed-off-by: Catalin Marinas <[email protected]>
Cc: Akinobu Mita <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
mm/Kconfig.debug | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/Kconfig.debug b/mm/Kconfig.debug
index af7cfb4..983375d 100644
--- a/mm/Kconfig.debug
+++ b/mm/Kconfig.debug
@@ -1,6 +1,6 @@
config DEBUG_PAGEALLOC
- bool "Debug page memory allocations"
- depends on DEBUG_KERNEL && ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ bool "Debug page memory allocations" if ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ depends on DEBUG_KERNEL
depends on !HIBERNATION || !PPC && !SPARC
depends on !KMEMCHECK
---help---
On Tue, 08 Jun 2010 17:25:57 +0100 Catalin Marinas wrote:
> The "select" statement in Kconfig files allows the enabling of options
> even if they have unmet direct dependencies (i.e. "depends on" expands
> to "no"). Currently, the "depends on" clauses are used in calculating
> the visibility but they do not affect the reverse dependencies in any
> way.
>
> The patch introduces additional tracking of the "depends on" statements
> and prints a warning on selecting an option if its direct dependencies
> are not met.
Hi Catalin,
Can these messages (on linux-next-20100611) be modified to include the
kconfig symbol that is causing them?
warning: (IP_VS_PROTO_ESP && NET && NETFILTER && IP_VS || IP_VS_PROTO_AH && NET && NETFILTER && IP_VS) selects IP_VS_PROTO_AH_ESP which has unmet direct dependencies (UNDEFINED)
warning: (SCx200_GPIO && SCx200 || PC8736x_GPIO && X86) selects NSC_GPIO which has unmet direct dependencies (X86_32)
Ah! It's the first symbol listed in each "phrase":
SCx200_GPIO and PC8736x_GPIO both have this problem.
(I haven't looked at IP_VS yet).
Jordan, is GEODE always 32-bit, so that some of these dependencies could be
cleaned up by using X86_32?
thanks,
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
On Fri, 2010-06-11 at 21:31 +0100, Randy Dunlap wrote:
> On Tue, 08 Jun 2010 17:25:57 +0100 Catalin Marinas wrote:
>
> > The "select" statement in Kconfig files allows the enabling of options
> > even if they have unmet direct dependencies (i.e. "depends on" expands
> > to "no"). Currently, the "depends on" clauses are used in calculating
> > the visibility but they do not affect the reverse dependencies in any
> > way.
> >
> > The patch introduces additional tracking of the "depends on" statements
> > and prints a warning on selecting an option if its direct dependencies
> > are not met.
>
> Can these messages (on linux-next-20100611) be modified to include the
> kconfig symbol that is causing them?
>
> warning: (IP_VS_PROTO_ESP && NET && NETFILTER && IP_VS ||
> IP_VS_PROTO_AH && NET && NETFILTER && IP_VS) selects
> IP_VS_PROTO_AH_ESP which has unmet direct dependencies (UNDEFINED)
> warning: (SCx200_GPIO && SCx200 || PC8736x_GPIO && X86) selects
> NSC_GPIO which has unmet direct dependencies (X86_32)
>
> Ah! It's the first symbol listed in each "phrase":
(sorry for the delay)
It is usually the first symbol though kbuild does some expression
reduction when calculating the dependencies and I can't guarantee that
it is always the case. It's probably for someone who understands kbuild
better then me to make the output easier to read.
--
Catalin
On Fri, 11 Jun 2010 13:31:19 -0700
Randy Dunlap <[email protected]> wrote:
> On Tue, 08 Jun 2010 17:25:57 +0100 Catalin Marinas wrote:
>
> > The "select" statement in Kconfig files allows the enabling of options
> > even if they have unmet direct dependencies (i.e. "depends on" expands
> > to "no"). Currently, the "depends on" clauses are used in calculating
> > the visibility but they do not affect the reverse dependencies in any
> > way.
> >
> > The patch introduces additional tracking of the "depends on" statements
> > and prints a warning on selecting an option if its direct dependencies
> > are not met.
>
>
> Hi Catalin,
>
> Can these messages (on linux-next-20100611) be modified to include the
> kconfig symbol that is causing them?
>
> warning: (IP_VS_PROTO_ESP && NET && NETFILTER && IP_VS || IP_VS_PROTO_AH && NET && NETFILTER && IP_VS) selects IP_VS_PROTO_AH_ESP which has unmet direct dependencies (UNDEFINED)
> warning: (SCx200_GPIO && SCx200 || PC8736x_GPIO && X86) selects NSC_GPIO which has unmet direct dependencies (X86_32)
>
> Ah! It's the first symbol listed in each "phrase":
>
> SCx200_GPIO and PC8736x_GPIO both have this problem.
> (I haven't looked at IP_VS yet).
It's unclear why NSC_GPIO depends upon X86_32 (davej sent the patch,
699352c30da8525a). While geode stuff will probably only ever exist on x86,
there's nothing in the driver that's X86_32-specific.
>
> Jordan, is GEODE always 32-bit, so that some of these dependencies could be
> cleaned up by using X86_32?
Sadly, I don't think Jordan tracks this list any more.