linux-kernel - Re: [PATCH v2] kconfig: autogenerated config_is

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20110518052747.GC15292@game.jcrosoft.org>
Date:	Wed, 18 May 2011 07:27:47 +0200
From:	Jean-Christophe PLAGNIOL-VILLARD <plagnioj@...osoft.com>
To:	Valdis.Kletnieks@...edu
Cc:	Arnaud Lacombe <lacombar@...il.com>, Michal Marek <mmarek@...e.cz>,
	linux-kernel@...r.kernel.org, linux-kbuild@...r.kernel.org,
	x86@...nel.org, Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

On 15:13 Tue 17 May     , Valdis.Kletnieks@...edu wrote:
> 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@...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.
yeah because the code is not design today to be able to compile if not def

But it's not the case everywhere in the kernel
as example on ARM it's common to if machine_is_xxx in the code and this is
handle the same way

And this is a good practice

I'll not said that to be able to use this config_is in most of the place will
be trivial but this will allow to simplify the maintenance a lot. This will
considerably reduce the number of config to test to be sure we brake nothing

> 
> 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?
> 
you can do it in one grep also
grep -r -E "CONFIG_|config_is" *

Best Regards,
J.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/