[RFC PATCH 0/3] kbuild: Unmet dependency warning

[RFC PATCH 0/3] kbuild: Unmet dependency warning

[Catalin Marinas]

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

[RFC PATCH 1/3] kbuild: Warn on selecting symbols with unmet direct dependencies

[Catalin Marinas]

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 <catalin.marinas@arm.com>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 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)

Re: [RFC PATCH 1/3] kbuild: Warn on selecting symbols with unmet direct dependencies

[Randy Dunlap]

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 ***

Re: [RFC PATCH 1/3] kbuild: Warn on selecting symbols with unmet direct dependencies

[Catalin Marinas]

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

Re: [RFC PATCH 1/3] kbuild: Warn on selecting symbols with unmet direct dependencies

[Andres Salomon]

On Fri, 11 Jun 2010 13:31:19 -0700
Randy Dunlap <randy.dunlap@oracle.com> 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.

[RFC PATCH 2/3] ARM: Mark CPU_32v6K as depended on CPU_V7

[Catalin Marinas]

CPU_32v6K is selected by CPU_V7 but it only depends on CPU_V6.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Cc: Russell King <rmk@arm.linux.org.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 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.

[RFC PATCH 3/3] Change DEBUG_PAGEALLOC dependency on ARCH_SUPPORTS_DEBUG_PAGEALLOC

[Catalin Marinas]

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 <catalin.marinas@arm.com>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 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---