diff mbox

strange behaviour from "make localmodconfig" throws out ath9k stuff

Message ID 1367258419.30667.20.camel@gandalf.local.home (mailing list archive)
State New, archived
Headers show

Commit Message

Steven Rostedt April 29, 2013, 6 p.m. UTC
Can you add this patch and see if it fixes your issue. If so, can I add
your "tested-by" to this patch?

Thanks,

-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Robert P. J. Day April 29, 2013, 6:26 p.m. UTC | #1
On Mon, 29 Apr 2013, Steven Rostedt wrote:

> Can you add this patch and see if it fixes your issue. If so, can I
> add your "tested-by" to this patch?

  i'll get to this shortly and let you know.

rday
Robert P. J. Day April 29, 2013, 6:41 p.m. UTC | #2
On Mon, 29 Apr 2013, Steven Rostedt wrote:

> Can you add this patch and see if it fixes your issue. If so, can I
> add your "tested-by" to this patch?

   here's what i can verify.  if i start with stock linux 3.9 and copy
in a known good .config with ATH-related stuff, here's the situation
immediately after the copy:

$ grep CONFIG_ATH .config
CONFIG_ATH_COMMON=m
CONFIG_ATH_CARDS=m
# CONFIG_ATH_DEBUG is not set
# CONFIG_ATH5K is not set
# CONFIG_ATH5K_PCI is not set
CONFIG_ATH9K_HW=m
CONFIG_ATH9K_COMMON=m
# CONFIG_ATH9K_BTCOEX_SUPPORT is not set
CONFIG_ATH9K=m
CONFIG_ATH9K_PCI=y
# CONFIG_ATH9K_AHB is not set
# CONFIG_ATH9K_DEBUGFS is not set
CONFIG_ATH9K_RATE_CONTROL=y
# CONFIG_ATH9K_HTC is not set
# CONFIG_ATH6KL is not set
$

which is fine and represents a valid collection of settings for 3.9.
but here's where things go wrong in the original case:

$ make localmodconfig
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  SHIPPED scripts/kconfig/zconf.tab.c
  SHIPPED scripts/kconfig/zconf.lex.c
  SHIPPED scripts/kconfig/zconf.hash.c
  HOSTCC  scripts/kconfig/zconf.tab.o
  HOSTLD  scripts/kconfig/conf
using config: '.config'
vboxnetadp config not found!!
vboxnetflt config not found!!
vboxdrv config not found!!
vboxpci config not found!!
#
# configuration written to .config
#
$ grep CONFIG_ATH .config
# CONFIG_ATH_CARDS is not set   <-- obviously wrong, all my ATH stuff is gone
$

  if i apply your patch and do this all over again, then i retain all
the ATH-related stuff after the "make localmodconfig":

$ grep CONFIG_ATH .config
CONFIG_ATH_COMMON=m
CONFIG_ATH_CARDS=m
# CONFIG_ATH_DEBUG is not set
# CONFIG_ATH5K is not set
# CONFIG_ATH5K_PCI is not set
CONFIG_ATH9K_HW=m
CONFIG_ATH9K_COMMON=m
# CONFIG_ATH9K_BTCOEX_SUPPORT is not set
CONFIG_ATH9K=m
CONFIG_ATH9K_PCI=y
# CONFIG_ATH9K_AHB is not set
# CONFIG_ATH9K_DEBUGFS is not set
CONFIG_ATH9K_RATE_CONTROL=y
# CONFIG_ATH9K_HTC is not set
# CONFIG_ATH6KL is not set
$

  so the patch appears to resolve at least that specific issue.

rday
Robert P. J. Day April 29, 2013, 7:15 p.m. UTC | #3
On Mon, 29 Apr 2013, Steven Rostedt wrote:

> Can you add this patch and see if it fixes your issue. If so, can I
> add your "tested-by" to this patch?

  just to be clear, that patch didn't solve the issue involving a
config file from an earlier kernel but i don't think it was supposed
to, was it?

rday
Robert P. J. Day April 29, 2013, 8:39 p.m. UTC | #4
On Mon, 29 Apr 2013, Steven Rostedt wrote:

> Can you add this patch and see if it fixes your issue. If so, can I add
> your "tested-by" to this patch?
>
> Thanks,
>
> -- Steve
>
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 3368939..4606cdf 100644
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -156,7 +156,6 @@ sub read_kconfig {
>
>      my $state = "NONE";
>      my $config;
> -    my @kconfigs;
>
>      my $cont = 0;
>      my $line;
> @@ -190,7 +189,13 @@ sub read_kconfig {
>
>  	# collect any Kconfig sources
>  	if (/^source\s*"(.*)"/) {
> -	    $kconfigs[$#kconfigs+1] = $1;
> +	    my $kconfig = $1;
> +	    # prevent reading twice.
> +	    if (!defined($read_kconfigs{$kconfig})) {
> +		$read_kconfigs{$kconfig} = 1;
> +		read_kconfig($kconfig);
> +	    }
> +	    next;
>  	}
>
>  	# configs found
> @@ -250,14 +255,6 @@ sub read_kconfig {
>  	}
>      }
>      close($kinfile);
> -
> -    # read in any configs that were found.
> -    foreach my $kconfig (@kconfigs) {
> -	if (!defined($read_kconfigs{$kconfig})) {
> -	    $read_kconfigs{$kconfig} = 1;
> -	    read_kconfig($kconfig);
> -	}
> -    }
>  }
>
>  if ($kconfig) {
>

  i'm interested -- should that Kconfig file have been written using a
different structure to avoid this issue? surely there are other
Kconfig files that would have had the same problem, no? i can't
believe this could be restricted to a single example involving
atheros stuff.

rday

p.s.  i'm going to submit at least one patch to reorg the atheros
Kconfig file since it's a bit awkward at the moment -- when you select
some options, other options suddenly appear in arbitrary other
locations. i really hate that.
Steven Rostedt April 29, 2013, 8:43 p.m. UTC | #5
On Mon, 2013-04-29 at 15:15 -0400, Robert P. J. Day wrote:
> On Mon, 29 Apr 2013, Steven Rostedt wrote:
> 
> > Can you add this patch and see if it fixes your issue. If so, can I
> > add your "tested-by" to this patch?
> 
>   just to be clear, that patch didn't solve the issue involving a
> config file from an earlier kernel but i don't think it was supposed
> to, was it?

No, and honestly, I don't see that as an issue ;-) Old configs with new
kernels will always have issues if a new dependency is created. In other
words, that's a WONTFIX bug.

make localmodconfig is to disable only the configs not needed for the
loaded modules. If a required config was not in the original .config,
then that's out of scope for localmodconfig. But the bug you reported
was, localmodconfig was disabling a required config that was in the
original .config.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Robert P. J. Day April 29, 2013, 8:50 p.m. UTC | #6
On Mon, 29 Apr 2013, Steven Rostedt wrote:

> On Mon, 2013-04-29 at 15:15 -0400, Robert P. J. Day wrote:
> > On Mon, 29 Apr 2013, Steven Rostedt wrote:
> >
> > > Can you add this patch and see if it fixes your issue. If so, can I
> > > add your "tested-by" to this patch?
> >
> >   just to be clear, that patch didn't solve the issue involving a
> > config file from an earlier kernel but i don't think it was supposed
> > to, was it?
>
> No, and honestly, I don't see that as an issue ;-) Old configs with
> new kernels will always have issues if a new dependency is created.
> In other words, that's a WONTFIX bug.

  no, that's fine, that's what i assumed.

rday
Steven Rostedt April 29, 2013, 11:40 p.m. UTC | #7
On Mon, 2013-04-29 at 16:39 -0400, Robert P. J. Day wrote:

>   i'm interested -- should that Kconfig file have been written using a
> different structure to avoid this issue? surely there are other
> Kconfig files that would have had the same problem, no? i can't
> believe this could be restricted to a single example involving
> atheros stuff.

I did a quick search and there's lots of places that source is within if
statements. But the config used isn't usually a tristate. That is, it is
either enabled or not, but can't be a module.

Here the if condition was a module and is something that localmodconfig
can disabled.

Hmm, I did find another case like this (and there may be more).

drivers/ipack/Kconfig does it too. But nobody noticed.

Anyway, it's fixed now, which is good.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 3368939..4606cdf 100644
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -156,7 +156,6 @@  sub read_kconfig {
 
     my $state = "NONE";
     my $config;
-    my @kconfigs;
 
     my $cont = 0;
     my $line;
@@ -190,7 +189,13 @@  sub read_kconfig {
 
 	# collect any Kconfig sources
 	if (/^source\s*"(.*)"/) {
-	    $kconfigs[$#kconfigs+1] = $1;
+	    my $kconfig = $1;
+	    # prevent reading twice.
+	    if (!defined($read_kconfigs{$kconfig})) {
+		$read_kconfigs{$kconfig} = 1;
+		read_kconfig($kconfig);
+	    }
+	    next;
 	}
 
 	# configs found
@@ -250,14 +255,6 @@  sub read_kconfig {
 	}
     }
     close($kinfile);
-
-    # read in any configs that were found.
-    foreach my $kconfig (@kconfigs) {
-	if (!defined($read_kconfigs{$kconfig})) {
-	    $read_kconfigs{$kconfig} = 1;
-	    read_kconfig($kconfig);
-	}
-    }
 }
 
 if ($kconfig) {