| Message ID | BANLkTinBkaaDOkx_oJZor5DXQyXh2=wWpg@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16.5.2011 21:38, Arnaud Lacombe wrote: > On Mon, May 16, 2011 at 3:03 PM, <Valdis.Kletnieks@vt.edu> wrote: >> #if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) >> if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) >> cpu = get_nohz_timer_target(); >> #endif >> new_base = per_cpu(tvec_bases, cpu); >> >> If you convert this to an if statement, will it still compile? Which will >> happen first, dead code elimination, or the warning that get_nohz_timer_target() >> is an implicit declaration because the definition in the .h file is also >> guarded by #ifdef CONFIG_NO_HZ? >> > I already exposed this case, but let's prove it: > [proven] Yes, probably a majority #ifdef CONFIG_FOO construct cannot be converted to C if statements. And architecture specific code can only be built on that architecture. But there are places where it is possible to let the compiler eliminate the if(0) and at least Ingo likes it for x86, so I'll merge it. The more build coverage the better. I figure that this feature is not wanted outside of the kernel build, though. So what about an option to 'conf' that controls whether these macros will be generated? Michal -- 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
Hi, On Mon, May 16, 2011 at 4:05 PM, Michal Marek <mmarek@suse.cz> wrote: > I figure that this feature is not wanted outside of the kernel build, > though. So what about an option to 'conf' that controls whether these > macros will be generated? > kconfig internal behaviors are mostly controlled by environment variable, which has the advantage to be front-end agnostic, that might be better. - Arnaud -- 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 16.5.2011 22:24, Arnaud Lacombe wrote: > Hi, > > On Mon, May 16, 2011 at 4:05 PM, Michal Marek <mmarek@suse.cz> wrote: >> I figure that this feature is not wanted outside of the kernel build, >> though. So what about an option to 'conf' that controls whether these >> macros will be generated? >> > kconfig internal behaviors are mostly controlled by environment > variable, which has the advantage to be front-end agnostic, that might > be better. Note that the header file is written by scripts/kconfig/conf --silentoldconfig, so different front-ends are not an issue. But an environment variable works fine as well. Michal -- 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 15:38 Mon 16 May , Arnaud Lacombe wrote: > Hi, > > On Mon, May 16, 2011 at 3:03 PM, <Valdis.Kletnieks@vt.edu> wrote: > > On Fri, 13 May 2011 10:09:09 +0200, Jean-Christophe PLAGNIOL-VILLARD said: > >> On 10:52 Mon 09 May , Michal Marek wrote: > >> > Do you have proof of concept patches that make use of the > >> > config_is_xxx macros? Acked by the respective subsystem maintainers? > >> > It would be a good idea to send them along to show that this feature > >> > is going to be actually used. > >> I've seen thousands of place in the kernel we can use > >> so I'll just take one example on x86 > >> > >> the patch attached is just an example > > > > Out of curiosity, will this Do The Right Thing for cases where things simply won't > > build for some configs? For example, consider this code snippet from kernel/timer.c, > > in __mod_timer() (near line 682): > > > > debug_activate(timer, expires); > > > > cpu = smp_processor_id(); > > > > #if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) > > if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) > > cpu = get_nohz_timer_target(); > > #endif > > new_base = per_cpu(tvec_bases, cpu); > > > > If you convert this to an if statement, will it still compile? Which will > > happen first, dead code elimination, or the warning that get_nohz_timer_target() > > is an implicit declaration because the definition in the .h file is also > > guarded by #ifdef CONFIG_NO_HZ? > > > I already exposed this case, but let's prove it: > > % grep CONFIG_SMP .config > # CONFIG_SMP is not set > > % git diff > diff --git a/kernel/timer.c b/kernel/timer.c > index fd61986..ea4a5ba 100644 > --- a/kernel/timer.c > +++ b/kernel/timer.c > @@ -681,10 +681,8 @@ __mod_timer(struct timer_list *timer, unsigned > long expires, > > cpu = smp_processor_id(); > > -#if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) > - if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) > + if (0 && 0 && !pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) > cpu = get_nohz_timer_target(); > -#endif > new_base = per_cpu(tvec_bases, cpu); > > % gmake kernel/timer.o > CHK include/linux/version.h > CHK include/generated/utsrelease.h > CALL scripts/checksyscalls.sh > CC kernel/timer.o > kernel/timer.c: In function '__mod_timer': > kernel/timer.c:685:3: error: implicit declaration of function > 'get_nohz_timer_target' > gmake[1]: *** [kernel/timer.o] Error 1 > gmake: *** [kernel/timer.o] Error 2 because we do not define the inline function if the CONFIG_ is not define as we are supposed to do if we want to compile without ifdef everywhere Best Regards, J. -- 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 22:33 Mon 16 May , Michal Marek wrote: > On 16.5.2011 22:24, Arnaud Lacombe wrote: > > Hi, > > > > On Mon, May 16, 2011 at 4:05 PM, Michal Marek <mmarek@suse.cz> wrote: > >> I figure that this feature is not wanted outside of the kernel build, > >> though. So what about an option to 'conf' that controls whether these > >> macros will be generated? > >> > > kconfig internal behaviors are mostly controlled by environment > > variable, which has the advantage to be front-end agnostic, that might > > be better. > > Note that the header file is written by scripts/kconfig/conf > --silentoldconfig, so different front-ends are not an issue. But an > environment variable works fine as well. I want it for ARM, AVR32 and SH too at least so make it optionnal is wished As they do not affect any code # git grep config_is_ 0 result Best Regards, J. -- 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 Tue, 17 May 2011 03:03:29 +0200, Jean-Christophe PLAGNIOL-VILLARD said: > On 15:38 Mon 16 May , Arnaud Lacombe wrote: > > On Mon, May 16, 2011 at 3:03 PM, <Valdis.Kletnieks@vt.edu> wrote: > > > #if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) > > > if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) > > > cpu = get_nohz_timer_target(); > > > #endif > > > new_base = per_cpu(tvec_bases, cpu); > > I already exposed this case, but let's prove it: > > > > % grep CONFIG_SMP .config > > # CONFIG_SMP is not set > > > > % git diff > > diff --git a/kernel/timer.c b/kernel/timer.c > > index fd61986..ea4a5ba 100644 > > --- a/kernel/timer.c > > +++ b/kernel/timer.c > > @@ -681,10 +681,8 @@ __mod_timer(struct timer_list *timer, unsigned > > long expires, > > > > cpu = smp_processor_id(); > > > > -#if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) > > - if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) > > + if (0 && 0 && !pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) > > cpu = get_nohz_timer_target(); > > -#endif > > new_base = per_cpu(tvec_bases, cpu); > > > > % gmake kernel/timer.o > > CHK include/linux/version.h > > CHK include/generated/utsrelease.h > > CALL scripts/checksyscalls.sh > > CC kernel/timer.o > > kernel/timer.c: In function '__mod_timer': > > kernel/timer.c:685:3: error: implicit declaration of function > > 'get_nohz_timer_target' > > gmake[1]: *** [kernel/timer.o] Error 1 > > gmake: *** [kernel/timer.o] Error 2 > because we do not define the inline function if the CONFIG_ is not define > as we are supposed to do if we want to compile without ifdef everywhere Right - the point is that since many/most cases of #ifdef CONFIG_FOO in open code won't compile cleanly if converted to config_is_foo(), it limits the usefulness of the feature. Which raises another question - does this create a maintenance headache, where people who used to just 'grep -r CONFIG_FOO' to find code they needed to fix up now have to remember to do a *second* grep to find all callsites?
diff --git a/kernel/timer.c b/kernel/timer.c index fd61986..ea4a5ba 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -681,10 +681,8 @@ __mod_timer(struct timer_list *timer, unsigned long expires, cpu = smp_processor_id(); -#if defined(CONFIG_NO_HZ) && defined(CONFIG_SMP) - if (!pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) + if (0 && 0 && !pinned && get_sysctl_timer_migration() && idle_cpu(cpu)) cpu = get_nohz_timer_target(); -#endif new_base = per_cpu(tvec_bases, cpu);