| Message ID | 20180228191805.20094-3-linux@rasmusvillemoes.dk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > kconfig.h was excluded from consideration by fixdep by > 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false > positive hits > > (1) include/config/.h > (2) include/config/h.h > (3) include/config/foo.h > > (1) occurred because kconfig.h contains the string CONFIG_ in a > comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we > have a check that the part after CONFIG_ is non-empty, so this does not > happen anymore (and CONFIG_ appears by itself elsewhere, so that check > is worthwhile). > > (2) comes from the include guard, __LINUX_KCONFIG_H. But with the > previous patch, we no longer match that either. > > That leaves (3), which amounts to one [1] false dependency (aka stat() call > done by make), which I think we can live with: > > We've already had one case [2] where the lack of include/linux/kconfig.h in > the .o.cmd file caused a missing rebuild, and while I originally thought > we should just put kconfig.h in the dependency list without parsing it > for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols > mentioned in it, and one can imagine some translation unit that just > does '#ifdef __BIG_ENDIAN' but doesn't through some other header > actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target > endianness could end up rebuilding the world, minus that small > TU. Quoting Linus, > > ... when missing dependencies cause a missed re-compile, the resulting > bugs can be _really_ subtle. > > [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change > that to FOO if we care > > [2] https://lkml.org/lkml/2018/2/22/838 > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> Sorry, I missed to include this series in the Kbuild fixes PR I sent two days ago. I was not tracking the randomize-struct thread. I read [2] and I noticed the background of this series just now. Hopefully, I will have another opportunity of PR if this series is necessary for v4.16 (seems so) Regardless of the randomize-struct issue, this series is great! > --- > scripts/basic/fixdep.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c > index 1b21870d6e7f..449b68c4c90c 100644 > --- a/scripts/basic/fixdep.c > +++ b/scripts/basic/fixdep.c > @@ -283,7 +283,6 @@ static int is_ignored_file(const char *s, int len) > { > return str_ends_with(s, len, "include/generated/autoconf.h") || > str_ends_with(s, len, "include/generated/autoksyms.h") || > - str_ends_with(s, len, "include/linux/kconfig.h") || > str_ends_with(s, len, ".ver"); > } > > -- > 2.15.1 > > -- > 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
On 5 March 2018 at 05:52, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > 2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: >> kconfig.h was excluded from consideration by fixdep by >> 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false >> positive hits [...] >> We've already had one case [2] where the lack of include/linux/kconfig.h in >> the .o.cmd file caused a missing rebuild, [...] >> [2] https://lkml.org/lkml/2018/2/22/838 > > > Sorry, I missed to include this series in the Kbuild fixes PR > I sent two days ago. > > I was not tracking the randomize-struct thread. > I read [2] and I noticed the background of this series just now. > > Hopefully, I will have another opportunity of PR > if this series is necessary for v4.16 (seems so) I should probably have put 3/3 first, if that is 4.16 material, the other two can obviously wait for 4.17. They don't really depend on each other apart from overlapping context, and the above commit message would need slight rewording if put before the others. I can do that reordering and resend if you wish. Rasmus -- 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
2018-03-05 17:15 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > On 5 March 2018 at 05:52, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: >> 2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: >>> kconfig.h was excluded from consideration by fixdep by >>> 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false >>> positive hits > [...] >>> We've already had one case [2] where the lack of include/linux/kconfig.h in >>> the .o.cmd file caused a missing rebuild, > [...] >>> [2] https://lkml.org/lkml/2018/2/22/838 >> >> >> Sorry, I missed to include this series in the Kbuild fixes PR >> I sent two days ago. >> >> I was not tracking the randomize-struct thread. >> I read [2] and I noticed the background of this series just now. >> >> Hopefully, I will have another opportunity of PR >> if this series is necessary for v4.16 (seems so) > > I should probably have put 3/3 first, if that is 4.16 material, the > other two can obviously wait for 4.17. They don't really depend on > each other apart from overlapping context, and the above commit > message would need slight rewording if put before the others. I can do > that reordering and resend if you wish. > > Rasmus 1/3 is obviously safe, 2/3 too. I think the current series is OK for v4.16. I will queue it up to the fixes branch shortly. Thanks!
2018-03-01 4:17 GMT+09:00 Rasmus Villemoes <linux@rasmusvillemoes.dk>: > kconfig.h was excluded from consideration by fixdep by > 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false > positive hits > > (1) include/config/.h > (2) include/config/h.h > (3) include/config/foo.h > > (1) occurred because kconfig.h contains the string CONFIG_ in a > comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we > have a check that the part after CONFIG_ is non-empty, so this does not > happen anymore (and CONFIG_ appears by itself elsewhere, so that check > is worthwhile). > > (2) comes from the include guard, __LINUX_KCONFIG_H. But with the > previous patch, we no longer match that either. > > That leaves (3), which amounts to one [1] false dependency (aka stat() call > done by make), which I think we can live with: > > We've already had one case [2] where the lack of include/linux/kconfig.h in > the .o.cmd file caused a missing rebuild, and while I originally thought > we should just put kconfig.h in the dependency list without parsing it > for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols > mentioned in it, and one can imagine some translation unit that just > does '#ifdef __BIG_ENDIAN' but doesn't through some other header > actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target > endianness could end up rebuilding the world, minus that small > TU. Quoting Linus, > > ... when missing dependencies cause a missed re-compile, the resulting > bugs can be _really_ subtle. > > [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change > that to FOO if we care > > [2] https://lkml.org/lkml/2018/2/22/838 > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> > --- > scripts/basic/fixdep.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c > index 1b21870d6e7f..449b68c4c90c 100644 > --- a/scripts/basic/fixdep.c > +++ b/scripts/basic/fixdep.c > @@ -283,7 +283,6 @@ static int is_ignored_file(const char *s, int len) > { > return str_ends_with(s, len, "include/generated/autoconf.h") || > str_ends_with(s, len, "include/generated/autoksyms.h") || > - str_ends_with(s, len, "include/linux/kconfig.h") || > str_ends_with(s, len, ".ver"); > } > > -- > 2.15.1 > > -- > 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 The series, applied to linux-kbuild/fixes. Thanks!
diff --git a/scripts/basic/fixdep.c b/scripts/basic/fixdep.c index 1b21870d6e7f..449b68c4c90c 100644 --- a/scripts/basic/fixdep.c +++ b/scripts/basic/fixdep.c @@ -283,7 +283,6 @@ static int is_ignored_file(const char *s, int len) { return str_ends_with(s, len, "include/generated/autoconf.h") || str_ends_with(s, len, "include/generated/autoksyms.h") || - str_ends_with(s, len, "include/linux/kconfig.h") || str_ends_with(s, len, ".ver"); }
kconfig.h was excluded from consideration by fixdep by 6a5be57f0f00 (fixdep: fix extraneous dependencies) to avoid some false positive hits (1) include/config/.h (2) include/config/h.h (3) include/config/foo.h (1) occurred because kconfig.h contains the string CONFIG_ in a comment. However, since dee81e988674 (fixdep: faster CONFIG_ search), we have a check that the part after CONFIG_ is non-empty, so this does not happen anymore (and CONFIG_ appears by itself elsewhere, so that check is worthwhile). (2) comes from the include guard, __LINUX_KCONFIG_H. But with the previous patch, we no longer match that either. That leaves (3), which amounts to one [1] false dependency (aka stat() call done by make), which I think we can live with: We've already had one case [2] where the lack of include/linux/kconfig.h in the .o.cmd file caused a missing rebuild, and while I originally thought we should just put kconfig.h in the dependency list without parsing it for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols mentioned in it, and one can imagine some translation unit that just does '#ifdef __BIG_ENDIAN' but doesn't through some other header actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target endianness could end up rebuilding the world, minus that small TU. Quoting Linus, ... when missing dependencies cause a missed re-compile, the resulting bugs can be _really_ subtle. [1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change that to FOO if we care [2] https://lkml.org/lkml/2018/2/22/838 Cc: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk> --- scripts/basic/fixdep.c | 1 - 1 file changed, 1 deletion(-)