Re: [PATCH] fix dependency generation
On Sat, Mar 31, 2007 at 06:11:36PM +0200, Roman Zippel wrote: > Hi, > > On Sat, 31 Mar 2007, Sam Ravnborg wrote: > > > The problem is that tristate symbol represent three values. > > =n => CONFIG_SYMBOL is undefined > > =y => CONFIG_SYMBOL is defined > > =m => COMFIG_SYMBOL_MODULE is defined > > > > The function split_config does not take into account the > > different values and 'fixing' this in fixdep is wrong. > > Because fixdep does not know if the variable is a tristate symbol or not > > so it can either blindly remove _MODULE (your patch) > > or each time it encounters _MODULE check for a symbol with and > > without _MODULE. > > What really matters is that CONFIG_SYMBOL changed, one could optimize for > the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially > ... > > > The better fix is to teach the split_config function that > > for tristate symbols two files shall be created in the include/config > > hirachy. So for apm this gets: > > include/config/apm.h > > include/config/apm/module.h > > if it requires thousands of new inodes for a feature which should be > rarely used. > > > This will make kconfig behave correct the day that someone add a config > > symbol with a _MODULE suffix. > > I'd rather reserve that namespace, if it allows for the simpler version to > just map all symbols to the basic config symbol name. OK, I have forwarded Jan's patch for inclusion (the bug-fix part). Sam > > bye, Roman - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Sat, Mar 31, 2007 at 06:11:36PM +0200, Roman Zippel wrote: Hi, On Sat, 31 Mar 2007, Sam Ravnborg wrote: The problem is that tristate symbol represent three values. =n = CONFIG_SYMBOL is undefined =y = CONFIG_SYMBOL is defined =m = COMFIG_SYMBOL_MODULE is defined The function split_config does not take into account the different values and 'fixing' this in fixdep is wrong. Because fixdep does not know if the variable is a tristate symbol or not so it can either blindly remove _MODULE (your patch) or each time it encounters _MODULE check for a symbol with and without _MODULE. What really matters is that CONFIG_SYMBOL changed, one could optimize for the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially ... The better fix is to teach the split_config function that for tristate symbols two files shall be created in the include/config hirachy. So for apm this gets: include/config/apm.h include/config/apm/module.h if it requires thousands of new inodes for a feature which should be rarely used. This will make kconfig behave correct the day that someone add a config symbol with a _MODULE suffix. I'd rather reserve that namespace, if it allows for the simpler version to just map all symbols to the basic config symbol name. OK, I have forwarded Jan's patch for inclusion (the bug-fix part). Sam bye, Roman - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Hi, On Sat, 31 Mar 2007, Sam Ravnborg wrote: > The problem is that tristate symbol represent three values. > =n => CONFIG_SYMBOL is undefined > =y => CONFIG_SYMBOL is defined > =m => COMFIG_SYMBOL_MODULE is defined > > The function split_config does not take into account the > different values and 'fixing' this in fixdep is wrong. > Because fixdep does not know if the variable is a tristate symbol or not > so it can either blindly remove _MODULE (your patch) > or each time it encounters _MODULE check for a symbol with and > without _MODULE. What really matters is that CONFIG_SYMBOL changed, one could optimize for the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially ... > The better fix is to teach the split_config function that > for tristate symbols two files shall be created in the include/config > hirachy. So for apm this gets: > include/config/apm.h > include/config/apm/module.h if it requires thousands of new inodes for a feature which should be rarely used. > This will make kconfig behave correct the day that someone add a config > symbol with a _MODULE suffix. I'd rather reserve that namespace, if it allows for the simpler version to just map all symbols to the basic config symbol name. bye, Roman - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Hi, On Sat, 31 Mar 2007, Sam Ravnborg wrote: The problem is that tristate symbol represent three values. =n = CONFIG_SYMBOL is undefined =y = CONFIG_SYMBOL is defined =m = COMFIG_SYMBOL_MODULE is defined The function split_config does not take into account the different values and 'fixing' this in fixdep is wrong. Because fixdep does not know if the variable is a tristate symbol or not so it can either blindly remove _MODULE (your patch) or each time it encounters _MODULE check for a symbol with and without _MODULE. What really matters is that CONFIG_SYMBOL changed, one could optimize for the COMFIG_SYMBOL_MODULE case, but I don't think it's worth it, especially ... The better fix is to teach the split_config function that for tristate symbols two files shall be created in the include/config hirachy. So for apm this gets: include/config/apm.h include/config/apm/module.h if it requires thousands of new inodes for a feature which should be rarely used. This will make kconfig behave correct the day that someone add a config symbol with a _MODULE suffix. I'd rather reserve that namespace, if it allows for the simpler version to just map all symbols to the basic config symbol name. bye, Roman - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
> the split config tree is built, but failed to also adjust fixdep
> accordingly - if changing a config option from or to m, files
> referencing the respective CONFIG_..._MODULE (but not the
> corresponding CONFIG_...) didn't get rebuilt.
The problem is that tristate symbol represent three values.
=n => CONFIG_SYMBOL is undefined
=y => CONFIG_SYMBOL is defined
=m => COMFIG_SYMBOL_MODULE is defined
The function split_config does not take into account the
different values and 'fixing' this in fixdep is wrong.
Because fixdep does not know if the variable is a tristate symbol or not
so it can either blindly remove _MODULE (your patch)
or each time it encounters _MODULE check for a symbol with and
without _MODULE.
The better fix is to teach the split_config function that
for tristate symbols two files shall be created in the include/config
hirachy. So for apm this gets:
include/config/apm.h
include/config/apm/module.h
This will make kconfig behave correct the day that someone add a config
symbol with a _MODULE suffix.
I will follow-up with two patches that implement the changes to split_config.
The first is a pure code refactoring preparing for the second patch.
Roman - please ack/nack these this since they touches kconfig backend.
Sam
>
> Once at it, also eliminate false dependencies due to use of
> ...CONFIG_... identifiers.
>
> Signed-off-by: Jan Beulich <[EMAIL PROTECTED]>
>
> --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0
> +0100
> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29
> 11:11:10.0 +0200
> @@ -29,8 +29,7 @@
> * option which is mentioned in any of the listed prequisites.
> *
> * To be exact, split-include populates a tree in include/config/,
> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> - * for the CONFIG_HIS_DRIVER option.
> + * e.g. include/config/his/driver.h, consiting of empty files.
> *
> * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
> * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> @@ -223,7 +222,7 @@ void use_config(char *m, int slen)
> void parse_config_file(char *map, size_t len)
> {
> int *end = (int *) (map + len);
> - /* start at +1, so that p can never be < map */
> + /* start at +1, so that p can never be <= map */
> int *m = (int *) map + 1;
> char *p, *q;
>
> @@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
> continue;
> conf:
> if (p > map + len - 7)
> + break;
> + if (isalnum(p[-1]) || p[-1] == '_')
> continue;
> if (memcmp(p, "CONFIG_", 7))
> continue;
> @@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
> continue;
>
> found:
> + if (!memcmp(q - 7, "_MODULE", 7))
> + q -= 7;
> use_config(p+7, q-p-7);
> }
> }
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Fri, Mar 30, 2007 at 04:43:17PM +0100, Jan Beulich wrote: > >But that will break UM - no?? > >See following note from fixdep: > > * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend > > onto > > * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do > > not > > * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as > > * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h, > > * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that > > * those files will have correct dependencies. > > Hmm, didn't see this note. Then this might warrant special casing UML, but > penalizing all code due to this seems at least odd to me. If I understand this, I would think that special-casing UML_CONFIG_* instead of *_CONFIG_* would be the way to go. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
>>> Sam Ravnborg <[EMAIL PROTECTED]> 30.03.07 17:08 >>> >On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote: >> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way >> the split config tree is built, but failed to also adjust fixdep >> accordingly - if changing a config option from or to m, files >> referencing the respective CONFIG_..._MODULE (but not the >> corresponding CONFIG_...) didn't get rebuilt. >Do you have a test case for this? >I want to play a little with this before I submit it. On i386, set CONFIG_APM=y, build, then change it to m. >> >> Once at it, also eliminate false dependencies due to use of >> ...CONFIG_... identifiers. >But that will break UM - no?? >See following note from fixdep: > * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto > * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not > * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as > * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h, > * through arch/um/include/uml-config.h; this fixdep "bug" makes sure that > * those files will have correct dependencies. Hmm, didn't see this note. Then this might warrant special casing UML, but penalizing all code due to this seems at least odd to me. Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Jan Beulich wrote: Randy Dunlap <[EMAIL PROTECTED]> 29.03.07 18:38 >>> On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote: Randy Dunlap <[EMAIL PROTECTED]> 29.03.07 17:39 >>> --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0 +0100 +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c2007-03-29 11:11:10.0 +0200 @@ -29,8 +29,7 @@ * option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, - * e.g. include/config/his/driver.h, which contains the #define/#undef - * for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above. Because it is no longer true. These files are empty as of 2.6.18. We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the CONFIG_symbol name, which is what I was trying to get at. So how about a comment like this: * To be exact, split-include populates a tree in include/config/, * e.g., include/config/sysctl/syscall.h, * for the CONFIG_SYSCTL_SYSCALL option, when that option * is enabled (=y or =m). Shouldn't that then be '..., when that option is or ever was enabled (=y or =m) since last cleaning the tree'? Uh, I have no idea, but I guess that's OK with me. -- ~Randy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
> the split config tree is built, but failed to also adjust fixdep
> accordingly - if changing a config option from or to m, files
> referencing the respective CONFIG_..._MODULE (but not the
> corresponding CONFIG_...) didn't get rebuilt.
Do you have a test case for this?
I want to play a little with this before I submit it.
>
> Once at it, also eliminate false dependencies due to use of
> ...CONFIG_... identifiers.
But that will break UM - no??
See following note from fixdep:
* Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto
* CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
* fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
* UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
* through arch/um/include/uml-config.h; this fixdep "bug" makes sure that
* those files will have correct dependencies.
Sam
>
> Signed-off-by: Jan Beulich <[EMAIL PROTECTED]>
>
> --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0
> +0100
> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29
> 11:11:10.0 +0200
> @@ -29,8 +29,7 @@
> * option which is mentioned in any of the listed prequisites.
> *
> * To be exact, split-include populates a tree in include/config/,
> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> - * for the CONFIG_HIS_DRIVER option.
> + * e.g. include/config/his/driver.h, consiting of empty files.
> *
> * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
> * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> @@ -223,7 +222,7 @@ void use_config(char *m, int slen)
> void parse_config_file(char *map, size_t len)
> {
> int *end = (int *) (map + len);
> - /* start at +1, so that p can never be < map */
> + /* start at +1, so that p can never be <= map */
> int *m = (int *) map + 1;
> char *p, *q;
>
> @@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
> continue;
> conf:
> if (p > map + len - 7)
> + break;
> + if (isalnum(p[-1]) || p[-1] == '_')
> continue;
> if (memcmp(p, "CONFIG_", 7))
> continue;
> @@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
> continue;
>
> found:
> + if (!memcmp(q - 7, "_MODULE", 7))
> + q -= 7;
> use_config(p+7, q-p-7);
> }
> }
>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
>>> Randy Dunlap <[EMAIL PROTECTED]> 29.03.07 18:38 >>> >On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote: > >> >>> Randy Dunlap <[EMAIL PROTECTED]> 29.03.07 17:39 >>> >> >> --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 >> >> 19:44:54.0 +0100 >> >> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29 >> >> 11:11:10.0 +0200 >> >> @@ -29,8 +29,7 @@ >> >> * option which is mentioned in any of the listed prequisites. >> >> * >> >> * To be exact, split-include populates a tree in include/config/, >> >> - * e.g. include/config/his/driver.h, which contains the #define/#undef >> >> - * for the CONFIG_HIS_DRIVER option. >> > >> >I don't see why you deleted the line above. >> >> Because it is no longer true. These files are empty as of 2.6.18. > >We seem to be talking about different lines above. Yes, the files >are empty, but they are named based on the CONFIG_symbol name, which >is what I was trying to get at. So how about a comment like this: > > * To be exact, split-include populates a tree in include/config/, > * e.g., include/config/sysctl/syscall.h, > * for the CONFIG_SYSCTL_SYSCALL option, when that option > * is enabled (=y or =m). Shouldn't that then be '..., when that option is or ever was enabled (=y or =m) since last cleaning the tree'? Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Randy Dunlap [EMAIL PROTECTED] 29.03.07 18:38 On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote: Randy Dunlap [EMAIL PROTECTED] 29.03.07 17:39 --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0 +0100 +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29 11:11:10.0 +0200 @@ -29,8 +29,7 @@ * option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, - * e.g. include/config/his/driver.h, which contains the #define/#undef - * for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above. Because it is no longer true. These files are empty as of 2.6.18. We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the CONFIG_symbol name, which is what I was trying to get at. So how about a comment like this: * To be exact, split-include populates a tree in include/config/, * e.g., include/config/sysctl/syscall.h, * for the CONFIG_SYSCTL_SYSCALL option, when that option * is enabled (=y or =m). Shouldn't that then be '..., when that option is or ever was enabled (=y or =m) since last cleaning the tree'? Jan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
the split config tree is built, but failed to also adjust fixdep
accordingly - if changing a config option from or to m, files
referencing the respective CONFIG_..._MODULE (but not the
corresponding CONFIG_...) didn't get rebuilt.
Do you have a test case for this?
I want to play a little with this before I submit it.
Once at it, also eliminate false dependencies due to use of
...CONFIG_... identifiers.
But that will break UM - no??
See following note from fixdep:
* Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto
* CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not
* fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as
* UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h,
* through arch/um/include/uml-config.h; this fixdep bug makes sure that
* those files will have correct dependencies.
Sam
Signed-off-by: Jan Beulich [EMAIL PROTECTED]
--- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0
+0100
+++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29
11:11:10.0 +0200
@@ -29,8 +29,7 @@
* option which is mentioned in any of the listed prequisites.
*
* To be exact, split-include populates a tree in include/config/,
- * e.g. include/config/his/driver.h, which contains the #define/#undef
- * for the CONFIG_HIS_DRIVER option.
+ * e.g. include/config/his/driver.h, consiting of empty files.
*
* So if the user changes his CONFIG_HIS_DRIVER option, only the objects
* which depend on include/linux/config/his/driver.h will be rebuilt,
@@ -223,7 +222,7 @@ void use_config(char *m, int slen)
void parse_config_file(char *map, size_t len)
{
int *end = (int *) (map + len);
- /* start at +1, so that p can never be map */
+ /* start at +1, so that p can never be = map */
int *m = (int *) map + 1;
char *p, *q;
@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
continue;
conf:
if (p map + len - 7)
+ break;
+ if (isalnum(p[-1]) || p[-1] == '_')
continue;
if (memcmp(p, CONFIG_, 7))
continue;
@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
continue;
found:
+ if (!memcmp(q - 7, _MODULE, 7))
+ q -= 7;
use_config(p+7, q-p-7);
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Jan Beulich wrote: Randy Dunlap [EMAIL PROTECTED] 29.03.07 18:38 On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote: Randy Dunlap [EMAIL PROTECTED] 29.03.07 17:39 --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0 +0100 +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c2007-03-29 11:11:10.0 +0200 @@ -29,8 +29,7 @@ * option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, - * e.g. include/config/his/driver.h, which contains the #define/#undef - * for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above. Because it is no longer true. These files are empty as of 2.6.18. We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the CONFIG_symbol name, which is what I was trying to get at. So how about a comment like this: * To be exact, split-include populates a tree in include/config/, * e.g., include/config/sysctl/syscall.h, * for the CONFIG_SYSCTL_SYSCALL option, when that option * is enabled (=y or =m). Shouldn't that then be '..., when that option is or ever was enabled (=y or =m) since last cleaning the tree'? Uh, I have no idea, but I guess that's OK with me. -- ~Randy - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Sam Ravnborg [EMAIL PROTECTED] 30.03.07 17:08 On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote: Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way the split config tree is built, but failed to also adjust fixdep accordingly - if changing a config option from or to m, files referencing the respective CONFIG_..._MODULE (but not the corresponding CONFIG_...) didn't get rebuilt. Do you have a test case for this? I want to play a little with this before I submit it. On i386, set CONFIG_APM=y, build, then change it to m. Once at it, also eliminate false dependencies due to use of ...CONFIG_... identifiers. But that will break UM - no?? See following note from fixdep: * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h, * through arch/um/include/uml-config.h; this fixdep bug makes sure that * those files will have correct dependencies. Hmm, didn't see this note. Then this might warrant special casing UML, but penalizing all code due to this seems at least odd to me. Jan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Fri, Mar 30, 2007 at 04:43:17PM +0100, Jan Beulich wrote: But that will break UM - no?? See following note from fixdep: * Note 2: if somebody writes HELLO_CONFIG_BOOM in a file, it will depend onto * CONFIG_BOOM. This could seem a bug (not too hard to fix), but please do not * fix it! Some UserModeLinux files (look at arch/um/) call CONFIG_BOOM as * UML_CONFIG_BOOM, to avoid conflicts with /usr/include/linux/autoconf.h, * through arch/um/include/uml-config.h; this fixdep bug makes sure that * those files will have correct dependencies. Hmm, didn't see this note. Then this might warrant special casing UML, but penalizing all code due to this seems at least odd to me. If I understand this, I would think that special-casing UML_CONFIG_* instead of *_CONFIG_* would be the way to go. Jeff -- Work email - jdike at linux dot intel dot com - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, Mar 29, 2007 at 10:27:14AM +0100, Jan Beulich wrote:
Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
the split config tree is built, but failed to also adjust fixdep
accordingly - if changing a config option from or to m, files
referencing the respective CONFIG_..._MODULE (but not the
corresponding CONFIG_...) didn't get rebuilt.
The problem is that tristate symbol represent three values.
=n = CONFIG_SYMBOL is undefined
=y = CONFIG_SYMBOL is defined
=m = COMFIG_SYMBOL_MODULE is defined
The function split_config does not take into account the
different values and 'fixing' this in fixdep is wrong.
Because fixdep does not know if the variable is a tristate symbol or not
so it can either blindly remove _MODULE (your patch)
or each time it encounters _MODULE check for a symbol with and
without _MODULE.
The better fix is to teach the split_config function that
for tristate symbols two files shall be created in the include/config
hirachy. So for apm this gets:
include/config/apm.h
include/config/apm/module.h
This will make kconfig behave correct the day that someone add a config
symbol with a _MODULE suffix.
I will follow-up with two patches that implement the changes to split_config.
The first is a pure code refactoring preparing for the second patch.
Roman - please ack/nack these this since they touches kconfig backend.
Sam
Once at it, also eliminate false dependencies due to use of
...CONFIG_... identifiers.
Signed-off-by: Jan Beulich [EMAIL PROTECTED]
--- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0
+0100
+++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29
11:11:10.0 +0200
@@ -29,8 +29,7 @@
* option which is mentioned in any of the listed prequisites.
*
* To be exact, split-include populates a tree in include/config/,
- * e.g. include/config/his/driver.h, which contains the #define/#undef
- * for the CONFIG_HIS_DRIVER option.
+ * e.g. include/config/his/driver.h, consiting of empty files.
*
* So if the user changes his CONFIG_HIS_DRIVER option, only the objects
* which depend on include/linux/config/his/driver.h will be rebuilt,
@@ -223,7 +222,7 @@ void use_config(char *m, int slen)
void parse_config_file(char *map, size_t len)
{
int *end = (int *) (map + len);
- /* start at +1, so that p can never be map */
+ /* start at +1, so that p can never be = map */
int *m = (int *) map + 1;
char *p, *q;
@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
continue;
conf:
if (p map + len - 7)
+ break;
+ if (isalnum(p[-1]) || p[-1] == '_')
continue;
if (memcmp(p, CONFIG_, 7))
continue;
@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
continue;
found:
+ if (!memcmp(q - 7, _MODULE, 7))
+ q -= 7;
use_config(p+7, q-p-7);
}
}
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote: > >>> Randy Dunlap <[EMAIL PROTECTED]> 29.03.07 17:39 >>> > >> --- linux-2.6.21-rc5/scripts/basic/fixdep.c2007-02-04 > >> 19:44:54.0 +0100 > >> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29 > >> 11:11:10.0 +0200 > >> @@ -29,8 +29,7 @@ > >> * option which is mentioned in any of the listed prequisites. > >> * > >> * To be exact, split-include populates a tree in include/config/, > >> - * e.g. include/config/his/driver.h, which contains the #define/#undef > >> - * for the CONFIG_HIS_DRIVER option. > > > >I don't see why you deleted the line above. > > Because it is no longer true. These files are empty as of 2.6.18. We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the CONFIG_symbol name, which is what I was trying to get at. So how about a comment like this: * To be exact, split-include populates a tree in include/config/, * e.g., include/config/sysctl/syscall.h, * for the CONFIG_SYSCTL_SYSCALL option, when that option * is enabled (=y or =m). --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
>>> Randy Dunlap <[EMAIL PROTECTED]> 29.03.07 17:39 >>> >> --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0 >> +0100 >> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29 >> 11:11:10.0 +0200 >> @@ -29,8 +29,7 @@ >> * option which is mentioned in any of the listed prequisites. >> * >> * To be exact, split-include populates a tree in include/config/, >> - * e.g. include/config/his/driver.h, which contains the #define/#undef >> - * for the CONFIG_HIS_DRIVER option. > >I don't see why you deleted the line above. Because it is no longer true. These files are empty as of 2.6.18. Jan - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, 29 Mar 2007 10:27:14 +0100 Jan Beulich wrote:
> Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
> the split config tree is built, but failed to also adjust fixdep
> accordingly - if changing a config option from or to m, files
> referencing the respective CONFIG_..._MODULE (but not the
> corresponding CONFIG_...) didn't get rebuilt.
>
> Once at it, also eliminate false dependencies due to use of
> ...CONFIG_... identifiers.
>
> Signed-off-by: Jan Beulich <[EMAIL PROTECTED]>
>
> --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0
> +0100
> +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29
> 11:11:10.0 +0200
> @@ -29,8 +29,7 @@
> * option which is mentioned in any of the listed prequisites.
> *
> * To be exact, split-include populates a tree in include/config/,
> - * e.g. include/config/his/driver.h, which contains the #define/#undef
> - * for the CONFIG_HIS_DRIVER option.
I don't see why you deleted the line above.
> + * e.g. include/config/his/driver.h, consiting of empty files.
consisting
> *
> * So if the user changes his CONFIG_HIS_DRIVER option, only the objects
> * which depend on "include/linux/config/his/driver.h" will be rebuilt,
> @@ -223,7 +222,7 @@ void use_config(char *m, int slen)
> void parse_config_file(char *map, size_t len)
> {
> int *end = (int *) (map + len);
> - /* start at +1, so that p can never be < map */
> + /* start at +1, so that p can never be <= map */
> int *m = (int *) map + 1;
> char *p, *q;
>
> @@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
> continue;
> conf:
> if (p > map + len - 7)
> + break;
> + if (isalnum(p[-1]) || p[-1] == '_')
> continue;
> if (memcmp(p, "CONFIG_", 7))
> continue;
> @@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
> continue;
>
> found:
> + if (!memcmp(q - 7, "_MODULE", 7))
> + q -= 7;
> use_config(p+7, q-p-7);
> }
> }
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, 29 Mar 2007 10:27:14 +0100 Jan Beulich wrote:
Commit 2e3646e51b2d6415549b310655df63e7e0d7a080 changed the way
the split config tree is built, but failed to also adjust fixdep
accordingly - if changing a config option from or to m, files
referencing the respective CONFIG_..._MODULE (but not the
corresponding CONFIG_...) didn't get rebuilt.
Once at it, also eliminate false dependencies due to use of
...CONFIG_... identifiers.
Signed-off-by: Jan Beulich [EMAIL PROTECTED]
--- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0
+0100
+++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29
11:11:10.0 +0200
@@ -29,8 +29,7 @@
* option which is mentioned in any of the listed prequisites.
*
* To be exact, split-include populates a tree in include/config/,
- * e.g. include/config/his/driver.h, which contains the #define/#undef
- * for the CONFIG_HIS_DRIVER option.
I don't see why you deleted the line above.
+ * e.g. include/config/his/driver.h, consiting of empty files.
consisting
*
* So if the user changes his CONFIG_HIS_DRIVER option, only the objects
* which depend on include/linux/config/his/driver.h will be rebuilt,
@@ -223,7 +222,7 @@ void use_config(char *m, int slen)
void parse_config_file(char *map, size_t len)
{
int *end = (int *) (map + len);
- /* start at +1, so that p can never be map */
+ /* start at +1, so that p can never be = map */
int *m = (int *) map + 1;
char *p, *q;
@@ -235,6 +234,8 @@ void parse_config_file(char *map, size_t
continue;
conf:
if (p map + len - 7)
+ break;
+ if (isalnum(p[-1]) || p[-1] == '_')
continue;
if (memcmp(p, CONFIG_, 7))
continue;
@@ -245,6 +246,8 @@ void parse_config_file(char *map, size_t
continue;
found:
+ if (!memcmp(q - 7, _MODULE, 7))
+ q -= 7;
use_config(p+7, q-p-7);
}
}
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
Randy Dunlap [EMAIL PROTECTED] 29.03.07 17:39 --- linux-2.6.21-rc5/scripts/basic/fixdep.c 2007-02-04 19:44:54.0 +0100 +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29 11:11:10.0 +0200 @@ -29,8 +29,7 @@ * option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, - * e.g. include/config/his/driver.h, which contains the #define/#undef - * for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above. Because it is no longer true. These files are empty as of 2.6.18. Jan - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fix dependency generation
On Thu, 29 Mar 2007 17:06:24 +0100 Jan Beulich wrote: Randy Dunlap [EMAIL PROTECTED] 29.03.07 17:39 --- linux-2.6.21-rc5/scripts/basic/fixdep.c2007-02-04 19:44:54.0 +0100 +++ 2.6.21-rc5-fixdep-mod/scripts/basic/fixdep.c 2007-03-29 11:11:10.0 +0200 @@ -29,8 +29,7 @@ * option which is mentioned in any of the listed prequisites. * * To be exact, split-include populates a tree in include/config/, - * e.g. include/config/his/driver.h, which contains the #define/#undef - * for the CONFIG_HIS_DRIVER option. I don't see why you deleted the line above. Because it is no longer true. These files are empty as of 2.6.18. We seem to be talking about different lines above. Yes, the files are empty, but they are named based on the CONFIG_symbol name, which is what I was trying to get at. So how about a comment like this: * To be exact, split-include populates a tree in include/config/, * e.g., include/config/sysctl/syscall.h, * for the CONFIG_SYSCTL_SYSCALL option, when that option * is enabled (=y or =m). --- ~Randy *** Remember to use Documentation/SubmitChecklist when testing your code *** - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
