[PATCH v2] kconfig: autogenerated config_is_xxx macro

[PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

this will allow to use to use

	if(config_is_xxx())
	if(config_is_xxx_module())

in the code instead of

	#ifdef CONFIG_xxx
	#ifdef CONFIG_xxx_MODULE

and now let the compiler remove the non usefull code and not the
pre-processor

as done in the mach-types for arm as exmaple

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
v2:

	use config_is

	add support of config_is_xxxx_module

Best Regards,
J.
 scripts/kconfig/confdata.c |   29 +++++++++++++++++++++++++++++
 1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 61c35bf..11b8b31 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -778,6 +778,29 @@ out:
 	return res;
 }
 
+static void conf_write_function_autoconf(FILE *out, char* conf, char* name,
+					 int val)
+{
+	char c;
+	char *tmp, *d;
+
+	d = strdup(conf);
+	tmp = d;
+	while ((c = *conf++))
+		*d++ = tolower(c);
+
+	fprintf(out, "#define %sis_", tmp);
+	free(tmp);
+
+	d = strdup(name);
+	tmp = d;
+	while ((c = *name++))
+		*d++ = tolower(c);
+	fprintf(out, "%s%s() %d\n", tmp, (val > 1) ? "_module" : "",
+		      val ? 1 : 0);
+	free(tmp);
+}
+
 int conf_write_autoconf(void)
 {
 	struct symbol *sym;
@@ -786,6 +809,7 @@ int conf_write_autoconf(void)
 	FILE *out, *tristate, *out_h;
 	time_t now;
 	int i;
+	int fct_val;
 
 	sym_clear_all_valid();
 
@@ -829,6 +853,7 @@ int conf_write_autoconf(void)
 		       rootmenu.prompt->text, ctime(&now));
 
 	for_all_symbols(i, sym) {
+		fct_val = 1;
 		sym_calc_value(sym);
 		if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
 			continue;
@@ -842,12 +867,14 @@ int conf_write_autoconf(void)
 		case S_TRISTATE:
 			switch (sym_get_tristate_value(sym)) {
 			case no:
+				fct_val = 0;
 				break;
 			case mod:
 				fprintf(tristate, "%s%s=M\n",
 				    CONFIG_, sym->name);
 				fprintf(out_h, "#define %s%s_MODULE 1\n",
 				    CONFIG_, sym->name);
+				fct_val = 2;
 				break;
 			case yes:
 				if (sym->type == S_TRISTATE)
@@ -874,8 +901,10 @@ int conf_write_autoconf(void)
 			    CONFIG_, sym->name, str);
 			break;
 		default:
+			fct_val = 0;
 			break;
 		}
+		conf_write_function_autoconf(out_h, CONFIG_, sym->name, fct_val);
 	}
 	fclose(out);
 	fclose(tristate);
-- 
1.7.4.1

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Arnaud Lacombe]

Hi,

On Fri, May 6, 2011 at 1:03 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> this will allow to use to use
>
>        if(config_is_xxx())
>        if(config_is_xxx_module())
>
> in the code instead of
>
>        #ifdef CONFIG_xxx
>        #ifdef CONFIG_xxx_MODULE
>
> and now let the compiler remove the non usefull code and not the
> pre-processor
>
Why would it be a good thing ?

Most configuration-dependent code inside functions tends to be moved
to a static inline already, which get conditionally defined based on
the CONFIG_<foo>. If it is not, then the code is badly architectured
(-> bad). Using that if(xxx) notation would also lead to yet more
heavily indented function (-> bad). Moreover, this introduces
yet-another way to check for an information (-> bad), and you will end
up with mixing the config_is_<xxx> notation inside a function
declaration, and CONFIG_<xxx> when not inside a function (-> bad)

Actually, this is even worse than that as you'll not be able to hide
structure (or structure members) inside CONFIG_<xxx> and use that
structure (or structure members) in config_is_<xxx> protected block
without causing compile-time failure.

Thanks,
 - Arnaud

> as done in the mach-types for arm as exmaple
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> v2:
>
>        use config_is
>
>        add support of config_is_xxxx_module
>
> Best Regards,
> J.
>  scripts/kconfig/confdata.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 61c35bf..11b8b31 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -778,6 +778,29 @@ out:
>        return res;
>  }
>
> +static void conf_write_function_autoconf(FILE *out, char* conf, char* name,
> +                                        int val)
> +{
> +       char c;
> +       char *tmp, *d;
> +
> +       d = strdup(conf);
> +       tmp = d;
> +       while ((c = *conf++))
> +               *d++ = tolower(c);
> +
> +       fprintf(out, "#define %sis_", tmp);
> +       free(tmp);
> +
> +       d = strdup(name);
> +       tmp = d;
> +       while ((c = *name++))
> +               *d++ = tolower(c);
> +       fprintf(out, "%s%s() %d\n", tmp, (val > 1) ? "_module" : "",
> +                     val ? 1 : 0);
> +       free(tmp);
> +}
> +
>  int conf_write_autoconf(void)
>  {
>        struct symbol *sym;
> @@ -786,6 +809,7 @@ int conf_write_autoconf(void)
>        FILE *out, *tristate, *out_h;
>        time_t now;
>        int i;
> +       int fct_val;
>
>        sym_clear_all_valid();
>
> @@ -829,6 +853,7 @@ int conf_write_autoconf(void)
>                       rootmenu.prompt->text, ctime(&now));
>
>        for_all_symbols(i, sym) {
> +               fct_val = 1;
>                sym_calc_value(sym);
>                if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
>                        continue;
> @@ -842,12 +867,14 @@ int conf_write_autoconf(void)
>                case S_TRISTATE:
>                        switch (sym_get_tristate_value(sym)) {
>                        case no:
> +                               fct_val = 0;
>                                break;
>                        case mod:
>                                fprintf(tristate, "%s%s=M\n",
>                                    CONFIG_, sym->name);
>                                fprintf(out_h, "#define %s%s_MODULE 1\n",
>                                    CONFIG_, sym->name);
> +                               fct_val = 2;
>                                break;
>                        case yes:
>                                if (sym->type == S_TRISTATE)
> @@ -874,8 +901,10 @@ int conf_write_autoconf(void)
>                            CONFIG_, sym->name, str);
>                        break;
>                default:
> +                       fct_val = 0;
>                        break;
>                }
> +               conf_write_function_autoconf(out_h, CONFIG_, sym->name, fct_val);
>        }
>        fclose(out);
>        fclose(tristate);
> --
> 1.7.4.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
>

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

On 12:19 Fri 06 May     , Arnaud Lacombe wrote:
> Hi,
> 
> On Fri, May 6, 2011 at 1:03 AM, Jean-Christophe PLAGNIOL-VILLARD
> <plagnioj@jcrosoft.com> wrote:
> > this will allow to use to use
> >
> >        if(config_is_xxx())
> >        if(config_is_xxx_module())
> >
> > in the code instead of
> >
> >        #ifdef CONFIG_xxx
> >        #ifdef CONFIG_xxx_MODULE
> >
> > and now let the compiler remove the non usefull code and not the
> > pre-processor
> >
> Why would it be a good thing ?
> 
> Most configuration-dependent code inside functions tends to be moved
> to a static inline already, which get conditionally defined based on
> the CONFIG_<foo>. If it is not, then the code is badly architectured
> (-> bad). Using that if(xxx) notation would also lead to yet more
> heavily indented function (-> bad). Moreover, this introduces
> yet-another way to check for an information (-> bad), and you will end
> up with mixing the config_is_<xxx> notation inside a function
> declaration, and CONFIG_<xxx> when not inside a function (-> bad)
> 
> Actually, this is even worse than that as you'll not be able to hide
> structure (or structure members) inside CONFIG_<xxx> and use that
> structure (or structure members) in config_is_<xxx> protected block
> without causing compile-time failure.
sorry but conditionnal structure members is bad practice
you save nearly no space nut for the test of the code in multiple
configuration. Use union for this.

the compile-time failure is good here. it's means your code is not generic.

specially when you want to keep code running on multiple soc/arch keep compiling
no matter the configuration

#ifdef in the code is a really bad habit

Best Regards,
J.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Michal Marek]

On 7.5.2011 03:50, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 12:19 Fri 06 May     , Arnaud Lacombe wrote:
>> Why would it be a good thing ?
>>
>> Most configuration-dependent code inside functions tends to be moved
>> to a static inline already, which get conditionally defined based on
>> the CONFIG_<foo>. If it is not, then the code is badly architectured
>> (->  bad). Using that if(xxx) notation would also lead to yet more
>> heavily indented function (->  bad). Moreover, this introduces
>> yet-another way to check for an information (->  bad), and you will end
>> up with mixing the config_is_<xxx>  notation inside a function
>> declaration, and CONFIG_<xxx>  when not inside a function (->  bad)
>>
>> Actually, this is even worse than that as you'll not be able to hide
>> structure (or structure members) inside CONFIG_<xxx>  and use that
>> structure (or structure members) in config_is_<xxx>  protected block
>> without causing compile-time failure.
> sorry but conditionnal structure members is bad practice
> you save nearly no space nut for the test of the code in multiple
> configuration. Use union for this.
>
> the compile-time failure is good here. it's means your code is not generic.
>
> specially when you want to keep code running on multiple soc/arch keep compiling
> no matter the configuration
>
> #ifdef in the code is a really bad habit

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.

Michal

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]

On 10:52 Mon 09 May     , Michal Marek wrote:
> On 7.5.2011 03:50, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >On 12:19 Fri 06 May     , Arnaud Lacombe wrote:
> >>Why would it be a good thing ?
> >>
> >>Most configuration-dependent code inside functions tends to be moved
> >>to a static inline already, which get conditionally defined based on
> >>the CONFIG_<foo>. If it is not, then the code is badly architectured
> >>(->  bad). Using that if(xxx) notation would also lead to yet more
> >>heavily indented function (->  bad). Moreover, this introduces
> >>yet-another way to check for an information (->  bad), and you will end
> >>up with mixing the config_is_<xxx>  notation inside a function
> >>declaration, and CONFIG_<xxx>  when not inside a function (->  bad)
> >>
> >>Actually, this is even worse than that as you'll not be able to hide
> >>structure (or structure members) inside CONFIG_<xxx>  and use that
> >>structure (or structure members) in config_is_<xxx>  protected block
> >>without causing compile-time failure.
> >sorry but conditionnal structure members is bad practice
> >you save nearly no space nut for the test of the code in multiple
> >configuration. Use union for this.
> >
> >the compile-time failure is good here. it's means your code is not generic.
> >
> >specially when you want to keep code running on multiple soc/arch keep compiling
> >no matter the configuration
> >
> >#ifdef in the code is a really bad habit
> 
> 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

Best Regards,
J.

[-- Attachment #2: p.patch --]
[-- Type: text/x-diff, Size: 2418 bytes --]

diff --git a/arch/x86/pci/irq.c b/arch/x86/pci/irq.c
index 8201165..ce4bd8a 100644
--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -527,14 +527,14 @@ static int pirq_pico_set(struct pci_dev *router, struct pci_dev *dev, int pirq,
 }
 
 #ifdef CONFIG_PCI_BIOS
-
 static int pirq_bios_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq)
 {
 	struct pci_dev *bridge;
 	int pin = pci_get_interrupt_pin(dev, &bridge);
 	return pcibios_set_irq_routing(bridge, pin - 1, irq);
 }
-
+#else
+static int pirq_bios_set(struct pci_dev *router, struct pci_dev *dev, int pirq, int irq) {}
 #endif
 
 static __init int intel_router_probe(struct irq_router *r, struct pci_dev *router, u16 device)
@@ -821,14 +821,12 @@ static void __init pirq_find_router(struct irq_router *r)
 	struct irq_routing_table *rt = pirq_table;
 	struct irq_router_handler *h;
 
-#ifdef CONFIG_PCI_BIOS
-	if (!rt->signature) {
+	if (config_is_pci_bios() && !rt->signature) {
 		printk(KERN_INFO "PCI: Using BIOS for IRQ routing\n");
 		r->set = pirq_bios_set;
 		r->name = "BIOS";
 		return;
 	}
-#endif
 
 	/* Default unless a driver reloads it */
 	r->name = "default";
@@ -1126,10 +1124,10 @@ void __init pcibios_irq_init(void)
 
 	pirq_table = pirq_find_routing_table();
 
-#ifdef CONFIG_PCI_BIOS
-	if (!pirq_table && (pci_probe & PCI_BIOS_IRQ_SCAN))
+	if (config_is_pci_bios() &&
+	    (!pirq_table && (pci_probe & PCI_BIOS_IRQ_SCAN)))
 		pirq_table = pcibios_get_irq_routing_table();
-#endif
+
 	if (pirq_table) {
 		pirq_peer_trick();
 		pirq_find_router(&pirq_router);
@@ -1178,11 +1176,9 @@ static void pirq_penalize_isa_irq(int irq, int active)
 
 void pcibios_penalize_isa_irq(int irq, int active)
 {
-#ifdef CONFIG_ACPI
-	if (!acpi_noirq)
+	if (config_is_pci_bios() && !acpi_noirq)
 		acpi_penalize_isa_irq(irq, active);
 	else
-#endif
 		pirq_penalize_isa_irq(irq, active);
 }
 
@@ -1197,8 +1193,7 @@ static int pirq_enable_irq(struct pci_dev *dev)
 		if (!io_apic_assign_pci_irqs && dev->irq)
 			return 0;
 
-		if (io_apic_assign_pci_irqs) {
-#ifdef CONFIG_X86_IO_APIC
+		if (config_is_x86_io_apic() && io_apic_assign_pci_irqs) {
 			struct pci_dev *temp_dev;
 			int irq;
 			struct io_apic_irq_attr irq_attr;
@@ -1237,7 +1232,6 @@ static int pirq_enable_irq(struct pci_dev *dev)
 				return 0;
 			} else
 				msg = "; probably buggy MP table";
-#endif
 		} else if (pci_probe & PCI_BIOS_IRQ_SCAN)
 			msg = "";
 		else

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Ingo Molnar]

* Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:

> -#ifdef CONFIG_PCI_BIOS
> -	if (!rt->signature) {
> +	if (config_is_pci_bios() && !rt->signature) {

Makes sense - but please name it in a more obvious way, such as:

	pci_bios_enabled()

Thanks,

	Ingo

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

On 10:30 Fri 13 May     , Ingo Molnar wrote:
> 
> * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> 
> > -#ifdef CONFIG_PCI_BIOS
> > -	if (!rt->signature) {
> > +	if (config_is_pci_bios() && !rt->signature) {
> 
> Makes sense - but please name it in a more obvious way, such as:
> 
> 	pci_bios_enabled()
the idea to generate the macro via Kconfig

so I propose config_is_pci_bios() and if it's a module
config_is_pci_bios_module()

if you have a better convention naming I'm fine to change it

maybe

config_pci_bios_enabled() / config_pci_bios_module_enabled

or 

pci_bios_enabled() / pci_bios_module_enabled()

Best Regards,
J.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Ingo Molnar]

* Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:

> On 10:30 Fri 13 May     , Ingo Molnar wrote:
> > 
> > * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> > 
> > > -#ifdef CONFIG_PCI_BIOS
> > > -	if (!rt->signature) {
> > > +	if (config_is_pci_bios() && !rt->signature) {
> > 
> > Makes sense - but please name it in a more obvious way, such as:
> > 
> > 	pci_bios_enabled()
> the idea to generate the macro via Kconfig

Okay, and there we are stuck with whatever the Kconfig name is. (we could 
rename that but not needed really)

Why not the canonical config_pci_bios() variant? It's the shortest one to 
write. The '_is' looks pretty superfluous to me.

Hm, i guess it could be mixed up with a function that configures the pci_bios.

I guess since i don't have any better idea config_is_pci_bios() sounds like a 
good choice after all.

Thanks,

	Ingo

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Alexey Dobriyan]

On Fri, May 13, 2011 at 1:01 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
>
>> On 10:30 Fri 13 May     , Ingo Molnar wrote:
>> >
>> > * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
>> >
>> > > -#ifdef CONFIG_PCI_BIOS
>> > > - if (!rt->signature) {
>> > > + if (config_is_pci_bios() && !rt->signature) {
>> >
>> > Makes sense - but please name it in a more obvious way, such as:
>> >
>> >     pci_bios_enabled()
>> the idea to generate the macro via Kconfig
>
> Okay, and there we are stuck with whatever the Kconfig name is. (we could
> rename that but not needed really)
>
> Why not the canonical config_pci_bios() variant? It's the shortest one to
> write. The '_is' looks pretty superfluous to me.
>
> Hm, i guess it could be mixed up with a function that configures the pci_bios.
>
> I guess since i don't have any better idea config_is_pci_bios() sounds like a
> good choice after all.

But we don't name config options like CONFIG_IS_PCI_BIOS, do we?
One should lowercase config option to minimize confusion, nothing more
if lowercased variant is OK.

Why it looks like a function call?

In fact one can even do

    if (CONFIG_PCI_BIOS && !rt->signature) {

for boolean options.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Michal Marek]

On 13.5.2011 12:21, Alexey Dobriyan wrote:
> Why it looks like a function call?
>
> In fact one can even do
>
>      if (CONFIG_PCI_BIOS&&  !rt->signature) {
>
> for boolean options.

That unfortunately doesn't work, CONFIG_* macros are not defined if not 
enabled.

Michal

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Ingo Molnar]

* Alexey Dobriyan <adobriyan@gmail.com> wrote:

> On Fri, May 13, 2011 at 1:01 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> >
> >> On 10:30 Fri 13 May     , Ingo Molnar wrote:
> >> >
> >> > * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> >> >
> >> > > -#ifdef CONFIG_PCI_BIOS
> >> > > - if (!rt->signature) {
> >> > > + if (config_is_pci_bios() && !rt->signature) {
> >> >
> >> > Makes sense - but please name it in a more obvious way, such as:
> >> >
> >> >     pci_bios_enabled()
> >> the idea to generate the macro via Kconfig
> >
> > Okay, and there we are stuck with whatever the Kconfig name is. (we could
> > rename that but not needed really)
> >
> > Why not the canonical config_pci_bios() variant? It's the shortest one to
> > write. The '_is' looks pretty superfluous to me.
> >
> > Hm, i guess it could be mixed up with a function that configures the pci_bios.
> >
> > I guess since i don't have any better idea config_is_pci_bios() sounds like a
> > good choice after all.
> 
> But we don't name config options like CONFIG_IS_PCI_BIOS, do we?

The problem is that 'config' can be misunderstood as a verb - it's often used 
for function names to say 'to configure'.

By naming it 'config_is_()' it unambiguously becomes a noun.

Thanks,

	Ingo

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Arnaud Lacombe]

Hi,

On Fri, May 13, 2011 at 4:09 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> On 10:52 Mon 09 May     , Michal Marek wrote:
>> On 7.5.2011 03:50, Jean-Christophe PLAGNIOL-VILLARD wrote:
>> >On 12:19 Fri 06 May     , Arnaud Lacombe wrote:
>> >>Why would it be a good thing ?
>> >>
>> >>Most configuration-dependent code inside functions tends to be moved
>> >>to a static inline already, which get conditionally defined based on
>> >>the CONFIG_<foo>. If it is not, then the code is badly architectured
>> >>(->  bad). Using that if(xxx) notation would also lead to yet more
>> >>heavily indented function (->  bad). Moreover, this introduces
>> >>yet-another way to check for an information (->  bad), and you will end
>> >>up with mixing the config_is_<xxx>  notation inside a function
>> >>declaration, and CONFIG_<xxx>  when not inside a function (->  bad)
>> >>
>> >>Actually, this is even worse than that as you'll not be able to hide
>> >>structure (or structure members) inside CONFIG_<xxx>  and use that
>> >>structure (or structure members) in config_is_<xxx>  protected block
>> >>without causing compile-time failure.
>> >sorry but conditionnal structure members is bad practice
>> >you save nearly no space nut for the test of the code in multiple
>> >configuration. Use union for this.
>> >
>> >the compile-time failure is good here. it's means your code is not generic.
>> >
>> >specially when you want to keep code running on multiple soc/arch keep compiling
>> >no matter the configuration
>> >
>> >#ifdef in the code is a really bad habit
>>
>> 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
>
An you get a nice build error, at least from:

 void pcibios_penalize_isa_irq(int irq, int active)
 {
-#ifdef CONFIG_ACPI
-	if (!acpi_noirq)
+	if (config_is_pci_bios() && !acpi_noirq)
 		acpi_penalize_isa_irq(irq, active);
 	else
-#endif
 		pirq_penalize_isa_irq(irq, active);
 }

as acpi_penalize_isa_irq() is only declared if CONFIG_ACPI is. So be
prepared to fix a lot of code.

I don't really care about the good or the bad, of each solution. These
are just tools, they are not intrinsically good or bad, only their
(over/under)usage might eventually get criticized. To further extend,
I am not sure you can keep x86-64 and x86-32 merged in the same
`arch/x86' tree without using a single #ifdef in struct definition and
function declaration.

 - Arnaud

> Best Regards,
> J.
>

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Andi Kleen]

Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> writes:

> this will allow to use to use
>
> 	if(config_is_xxx())
> 	if(config_is_xxx_module())
>
> in the code instead of
>
> 	#ifdef CONFIG_xxx
> 	#ifdef CONFIG_xxx_MODULE

Very nice idea.

Acked-by: Andi Kleen <ak@linux.intel.com>

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Arnaud Lacombe]

Hi,

On Fri, May 6, 2011 at 1:03 AM, Jean-Christophe PLAGNIOL-VILLARD
<plagnioj@jcrosoft.com> wrote:
> this will allow to use to use
>
>        if(config_is_xxx())
>        if(config_is_xxx_module())
>
> in the code instead of
>
>        #ifdef CONFIG_xxx
>        #ifdef CONFIG_xxx_MODULE
>
> and now let the compiler remove the non usefull code and not the
> pre-processor
>
For the record, there is at least one report of dead code elimination
regression in GCC 4.4.x:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=42494

 - Arnaud

> as done in the mach-types for arm as exmaple
>
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
> v2:
>
>        use config_is
>
>        add support of config_is_xxxx_module
>
> Best Regards,
> J.
>  scripts/kconfig/confdata.c |   29 +++++++++++++++++++++++++++++
>  1 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 61c35bf..11b8b31 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -778,6 +778,29 @@ out:
>        return res;
>  }
>
> +static void conf_write_function_autoconf(FILE *out, char* conf, char* name,
> +                                        int val)
> +{
> +       char c;
> +       char *tmp, *d;
> +
> +       d = strdup(conf);
> +       tmp = d;
> +       while ((c = *conf++))
> +               *d++ = tolower(c);
> +
> +       fprintf(out, "#define %sis_", tmp);
> +       free(tmp);
> +
> +       d = strdup(name);
> +       tmp = d;
> +       while ((c = *name++))
> +               *d++ = tolower(c);
> +       fprintf(out, "%s%s() %d\n", tmp, (val > 1) ? "_module" : "",
> +                     val ? 1 : 0);
> +       free(tmp);
> +}
> +
>  int conf_write_autoconf(void)
>  {
>        struct symbol *sym;
> @@ -786,6 +809,7 @@ int conf_write_autoconf(void)
>        FILE *out, *tristate, *out_h;
>        time_t now;
>        int i;
> +       int fct_val;
>
>        sym_clear_all_valid();
>
> @@ -829,6 +853,7 @@ int conf_write_autoconf(void)
>                       rootmenu.prompt->text, ctime(&now));
>
>        for_all_symbols(i, sym) {
> +               fct_val = 1;
>                sym_calc_value(sym);
>                if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
>                        continue;
> @@ -842,12 +867,14 @@ int conf_write_autoconf(void)
>                case S_TRISTATE:
>                        switch (sym_get_tristate_value(sym)) {
>                        case no:
> +                               fct_val = 0;
>                                break;
>                        case mod:
>                                fprintf(tristate, "%s%s=M\n",
>                                    CONFIG_, sym->name);
>                                fprintf(out_h, "#define %s%s_MODULE 1\n",
>                                    CONFIG_, sym->name);
> +                               fct_val = 2;
>                                break;
>                        case yes:
>                                if (sym->type == S_TRISTATE)
> @@ -874,8 +901,10 @@ int conf_write_autoconf(void)
>                            CONFIG_, sym->name, str);
>                        break;
>                default:
> +                       fct_val = 0;
>                        break;
>                }
> +               conf_write_function_autoconf(out_h, CONFIG_, sym->name, fct_val);
>        }
>        fclose(out);
>        fclose(tristate);
> --
> 1.7.4.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
>

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

On 14:21 Fri 13 May     , Ingo Molnar wrote:
> 
> * Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > On Fri, May 13, 2011 at 1:01 PM, Ingo Molnar <mingo@elte.hu> wrote:
> > >
> > > * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> > >
> > >> On 10:30 Fri 13 May     , Ingo Molnar wrote:
> > >> >
> > >> > * Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> wrote:
> > >> >
> > >> > > -#ifdef CONFIG_PCI_BIOS
> > >> > > - if (!rt->signature) {
> > >> > > + if (config_is_pci_bios() && !rt->signature) {
> > >> >
> > >> > Makes sense - but please name it in a more obvious way, such as:
> > >> >
> > >> >     pci_bios_enabled()
> > >> the idea to generate the macro via Kconfig
> > >
> > > Okay, and there we are stuck with whatever the Kconfig name is. (we could
> > > rename that but not needed really)
> > >
> > > Why not the canonical config_pci_bios() variant? It's the shortest one to
> > > write. The '_is' looks pretty superfluous to me.
> > >
> > > Hm, i guess it could be mixed up with a function that configures the pci_bios.
> > >
> > > I guess since i don't have any better idea config_is_pci_bios() sounds like a
> > > good choice after all.
> > 
> > But we don't name config options like CONFIG_IS_PCI_BIOS, do we?
> 
> The problem is that 'config' can be misunderstood as a verb - it's often used 
> for function names to say 'to configure'.
> 
> By naming it 'config_is_()' it unambiguously becomes a noun.
with Andi and Ingo ack

can I assume this patch go the -next and the .40

if yes I rebase some of my patch against it for at91

I'll regulary send patch to switch to it

Best Regards,
J.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1264 bytes --]

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?


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Arnaud Lacombe]

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

 - Arnaud

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Michal Marek]

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

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Arnaud Lacombe]

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

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Michal Marek]

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

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

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.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

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.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Michal Marek]

On Fri, May 06, 2011 at 07:03:49AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> this will allow to use to use
> 
> 	if(config_is_xxx())
> 	if(config_is_xxx_module())
> 
> in the code instead of
> 
> 	#ifdef CONFIG_xxx
> 	#ifdef CONFIG_xxx_MODULE
> 
> and now let the compiler remove the non usefull code and not the
> pre-processor
> 
> as done in the mach-types for arm as exmaple
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

I pushed this to kbuild-2.6.git#kconfig, but I found an issue that needs
to be fixed before we start using this feature: scripts/basic/fixdep
scans source files for occurences of CONFIG_xxx and adds dependencies on
include/config/xxx. It needs to be taught to match  config_is_xxx as
well.

Michal

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:45 Tue 17 May     , Michal Marek wrote:
> On Fri, May 06, 2011 at 07:03:49AM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > this will allow to use to use
> > 
> > 	if(config_is_xxx())
> > 	if(config_is_xxx_module())
> > 
> > in the code instead of
> > 
> > 	#ifdef CONFIG_xxx
> > 	#ifdef CONFIG_xxx_MODULE
> > 
> > and now let the compiler remove the non usefull code and not the
> > pre-processor
> > 
> > as done in the mach-types for arm as exmaple
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> 
> I pushed this to kbuild-2.6.git#kconfig, but I found an issue that needs
> to be fixed before we start using this feature: scripts/basic/fixdep
> scans source files for occurences of CONFIG_xxx and adds dependencies on
> include/config/xxx. It needs to be taught to match  config_is_xxx as
> well.
ok I take a look as I'm some patch that use it

Best Regards,
J.

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2185 bytes --]

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?


[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

Re: [PATCH v2] kconfig: autogenerated config_is_xxx macro

[Jean-Christophe PLAGNIOL-VILLARD]

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.