Date: Wed, 18 May 2011 07:27:47 +0200
From: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
To: Valdis.Kletnieks@vt.edu
Cc: Arnaud Lacombe <lacombar@gmail.com>, Michal Marek <mmarek@suse.cz>,
        linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
        x86@kernel.org, Ingo Molnar <mingo@redhat.com>
Subject: Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro
Message-ID: <20110518052747.GC15292@game.jcrosoft.org>
References: <1304658229-30820-1-git-send-email-plagnioj@jcrosoft.com>
 <BANLkTi=s8w01NHnxB7cfCvy4ZS5GnUABZg@mail.gmail.com>
 <20110507015041.GA21017@game.jcrosoft.org>
 <4DC7AB57.9050002@suse.cz>
 <20110513080909.GO18952@game.jcrosoft.org>
 <32557.1305572616@localhost>
 <BANLkTinBkaaDOkx_oJZor5DXQyXh2=wWpg@mail.gmail.com>
 <20110517010329.GB13466@game.jcrosoft.org>
 <14777.1305659606@localhost>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <14777.1305659606@localhost>
User-Agent: Mutt/1.5.20 (2009-06-14)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3102
Lines: 76

On 15:13 Tue 17 May     , Valdis.Kletnieks@vt.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@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.
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/