[PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Paul Gortmaker]

Some linux-next builds were showing thousands of repeated warnings like:

 linux/netdevice.h:964:1: warning: "__enabled_CONFIG_FCOE" is not defined [-Wundef]

An example is at:  http://kisskb.ellerman.id.au/kisskb/buildresult/6091752/

TonyB proposed a fix, and DaveM indicated IS_ENABLED should be fixed
instead.  [ https://lkml.org/lkml/2012/3/15/612 ]

This got me looking at IS_ENABLED, and I realized that to support using
this thing in C code, it was bloating our autoconf.h header file by
about 15,000 lines.  Ouch.  And it still doesn't have all the possible
lines in it, otherwise we'd not see warnings like the above.

If we limit its use to just CPP #if ... directives, we can unwind the
bloat and get down to less than ~1k of lines.

Fortunately IS_ENABLED is fairly new, and of the ~300 instances, only
a handful are actually in C code, so it is easy to unwind those.

The other downside of C users, I thought while unwinding the above users,
is that the use of IS_ENABLED on Kconfig bools in C if (...) conditionals
can give the misleading impression that the two code paths will exist in
the binary, and that the path is chosen at runtime (which it is not).

Commits 1 --> 4 are just largely unwind, so if you are going to just
look at one, please skip to commit #5 and read the commit log there,
as it has more details on how this bloat all came about.

I want to test this more, but I also wanted to get some initial
feedback in case I'm completely barking up the wrong tree here.

Commit ce1744f4ed20ca8 seems to endorse C users of IS_ENABLED, and
it went through Linus and Andrew, so I'm not sure scrapping C users
as a solution will be universally agreeable...

Thanks,
Paul
---

Paul Gortmaker (5):
  Revert "mm: replace PAGE_MIGRATION with IS_ENABLED(CONFIG_MIGRATION)"
  ARM: remove C users of IS_ENABLED on THUMB2_KERNEL
  drivers/net: remove IS_ENABLED usage from wiznet drivers
  Revert "kconfig: fix __enabled_ macros definition for invisible and
    un-selected symbols"
  kconfig: limit IS_ENABLED & similar to CPP usage

 arch/arm/include/asm/opcodes.h      |    6 ++++
 arch/arm/kernel/ftrace.c            |    9 +-----
 arch/arm/kernel/insn.c              |    9 +++---
 arch/arm/kernel/kprobes.c           |   34 ++++++++++++------------
 arch/arm/kernel/patch.c             |   28 ++++++++++---------
 drivers/net/ethernet/wiznet/w5100.c |    9 ++++--
 drivers/net/ethernet/wiznet/w5300.c |   18 ++++++++----
 include/linux/kconfig.h             |    8 +++---
 include/linux/migrate.h             |    2 +
 mm/mprotect.c                       |    2 +-
 mm/rmap.c                           |    7 ++---
 scripts/kconfig/confdata.c          |   49 +++++++++-------------------------
 12 files changed, 86 insertions(+), 95 deletions(-)

-- 
1.7.9.1

[PATCH 1/5] Revert "mm: replace PAGE_MIGRATION with IS_ENABLED(CONFIG_MIGRATION)"

[Paul Gortmaker]

This reverts commit ce1744f4ed20ca873360e54502f8a71564ef7cc6.

Using IS_ENABLED() within C (vs. within CPP #if statements) requires
us to actually define every possible bool/tristate Kconfig option
twice (__enabled_* and __enabled_*_MODULE variants).

That means we end up with about 16k lines of __enabled_ in autoconf.h,
instead of only less than 1k, and we'll be processing that extra 15k
lines for each and every C file. [x86_64 defconfig]

Kill off the C users of IS_ENABLED so we can shrink autoconf.h

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 include/linux/migrate.h |    2 ++
 mm/mprotect.c           |    2 +-
 mm/rmap.c               |    7 +++----
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index 855c337..05ed282 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -8,6 +8,7 @@
 typedef struct page *new_page_t(struct page *, unsigned long private, int **);
 
 #ifdef CONFIG_MIGRATION
+#define PAGE_MIGRATION 1
 
 extern void putback_lru_pages(struct list_head *l);
 extern int migrate_page(struct address_space *,
@@ -31,6 +32,7 @@ extern void migrate_page_copy(struct page *newpage, struct page *page);
 extern int migrate_huge_page_move_mapping(struct address_space *mapping,
 				  struct page *newpage, struct page *page);
 #else
+#define PAGE_MIGRATION 0
 
 static inline void putback_lru_pages(struct list_head *l) {}
 static inline int migrate_pages(struct list_head *l, new_page_t x,
diff --git a/mm/mprotect.c b/mm/mprotect.c
index a409926..142ef4a 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -60,7 +60,7 @@ static void change_pte_range(struct mm_struct *mm, pmd_t *pmd,
 				ptent = pte_mkwrite(ptent);
 
 			ptep_modify_prot_commit(mm, addr, pte, ptent);
-		} else if (IS_ENABLED(CONFIG_MIGRATION) && !pte_file(oldpte)) {
+		} else if (PAGE_MIGRATION && !pte_file(oldpte)) {
 			swp_entry_t entry = pte_to_swp_entry(oldpte);
 
 			if (is_write_migration_entry(entry)) {
diff --git a/mm/rmap.c b/mm/rmap.c
index 36d01a2..4720d3d 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -1298,7 +1298,7 @@ int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 			}
 			dec_mm_counter(mm, MM_ANONPAGES);
 			inc_mm_counter(mm, MM_SWAPENTS);
-		} else if (IS_ENABLED(CONFIG_MIGRATION)) {
+		} else if (PAGE_MIGRATION) {
 			/*
 			 * Store the pfn of the page in a special migration
 			 * pte. do_swap_page() will wait until the migration
@@ -1309,8 +1309,7 @@ int try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 		}
 		set_pte_at(mm, address, pte, swp_entry_to_pte(entry));
 		BUG_ON(pte_file(*pte));
-	} else if (IS_ENABLED(CONFIG_MIGRATION) &&
-		   (TTU_ACTION(flags) == TTU_MIGRATION)) {
+	} else if (PAGE_MIGRATION && (TTU_ACTION(flags) == TTU_MIGRATION)) {
 		/* Establish migration entry for a file page */
 		swp_entry_t entry;
 		entry = make_migration_entry(page, pte_write(pteval));
@@ -1516,7 +1515,7 @@ static int try_to_unmap_anon(struct page *page, enum ttu_flags flags)
 		 * locking requirements of exec(), migration skips
 		 * temporary VMAs until after exec() completes.
 		 */
-		if (IS_ENABLED(CONFIG_MIGRATION) && (flags & TTU_MIGRATION) &&
+		if (PAGE_MIGRATION && (flags & TTU_MIGRATION) &&
 				is_vma_temporary_stack(vma))
 			continue;
 
-- 
1.7.9.1

Re: [PATCH 1/5] Revert "mm: replace PAGE_MIGRATION with IS_ENABLED(CONFIG_MIGRATION)"

[Andrew Morton]

On Wed, 11 Apr 2012 19:50:53 -0400
Paul Gortmaker <paul.gortmaker@windriver.com> wrote:

> This reverts commit ce1744f4ed20ca873360e54502f8a71564ef7cc6.

Acked-by: Andrew Morton <akpm@linux-foundation.org>

Please include it in linux-next and merge it up thyself.

[PATCH 2/5] ARM: remove C users of IS_ENABLED on THUMB2_KERNEL

[Paul Gortmaker]

We need to limit our use of IS_ENABLED to CPP #if directives if
we want to have a small autoconf.h file.  The use of it in C
code also has the negative aspect of implicitly making it look
like THUMB2_KERNEL is some kind of dynamic entity that is evaluated
at runtime, when it really is a Kconfig bool and hence an
either/or thing selected at config time.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 arch/arm/include/asm/opcodes.h |    6 ++++++
 arch/arm/kernel/ftrace.c       |    9 ++-------
 arch/arm/kernel/insn.c         |    9 +++++----
 arch/arm/kernel/kprobes.c      |   34 +++++++++++++++++-----------------
 arch/arm/kernel/patch.c        |   28 +++++++++++++++-------------
 5 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/arch/arm/include/asm/opcodes.h b/arch/arm/include/asm/opcodes.h
index 19c48de..7e372d7 100644
--- a/arch/arm/include/asm/opcodes.h
+++ b/arch/arm/include/asm/opcodes.h
@@ -58,6 +58,12 @@ extern asmlinkage unsigned int arm_check_condition(u32 opcode, u32 psr);
 #define __opcode_to_mem_thumb32(x) swahw32(x)
 #endif
 
+#ifdef CONFIG_THUMB2_KERNEL
+#define __opcode_to_mem	__opcode_to_mem_thumb32
+#else
+#define __opcode_to_mem	__opcode_to_mem_arm
+#endif
+
 #define __mem_to_opcode_arm(x) __opcode_to_mem_arm(x)
 #define __mem_to_opcode_thumb16(x) __opcode_to_mem_thumb16(x)
 #define __mem_to_opcode_thumb32(x) __opcode_to_mem_thumb32(x)
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index df0bf0c..0e41ff8 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -73,13 +73,8 @@ static int ftrace_modify_code(unsigned long pc, unsigned long old,
 {
 	unsigned long replaced;
 
-	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
-		old = __opcode_to_mem_thumb32(old);
-		new = __opcode_to_mem_thumb32(new);
-	} else {
-		old = __opcode_to_mem_arm(old);
-		new = __opcode_to_mem_arm(new);
-	}
+	old = __opcode_to_mem(old);
+	new = __opcode_to_mem(new);
 
 	if (validate) {
 		if (probe_kernel_read(&replaced, (void *)pc, MCOUNT_INSN_SIZE))
diff --git a/arch/arm/kernel/insn.c b/arch/arm/kernel/insn.c
index b760340..12f9fbd 100644
--- a/arch/arm/kernel/insn.c
+++ b/arch/arm/kernel/insn.c
@@ -55,8 +55,9 @@ __arm_gen_branch_arm(unsigned long pc, unsigned long addr, bool link)
 unsigned long
 __arm_gen_branch(unsigned long pc, unsigned long addr, bool link)
 {
-	if (IS_ENABLED(CONFIG_THUMB2_KERNEL))
-		return __arm_gen_branch_thumb2(pc, addr, link);
-	else
-		return __arm_gen_branch_arm(pc, addr, link);
+#ifdef CONFIG_THUMB2_KERNEL
+	return __arm_gen_branch_thumb2(pc, addr, link);
+#else
+	return __arm_gen_branch_arm(pc, addr, link);
+#endif
 }
diff --git a/arch/arm/kernel/kprobes.c b/arch/arm/kernel/kprobes.c
index 4dd41fc..859c7d2 100644
--- a/arch/arm/kernel/kprobes.c
+++ b/arch/arm/kernel/kprobes.c
@@ -109,25 +109,25 @@ void __kprobes arch_arm_kprobe(struct kprobe *p)
 	unsigned int brkp;
 	void *addr;
 
-	if (IS_ENABLED(CONFIG_THUMB2_KERNEL)) {
-		/* Remove any Thumb flag */
-		addr = (void *)((uintptr_t)p->addr & ~1);
-
-		if (is_wide_instruction(p->opcode))
-			brkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
-		else
-			brkp = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
-	} else {
-		kprobe_opcode_t insn = p->opcode;
+#ifdef CONFIG_THUMB2_KERNEL
+	/* Remove any Thumb flag */
+	addr = (void *)((uintptr_t)p->addr & ~1);
 
-		addr = p->addr;
-		brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
+	if (is_wide_instruction(p->opcode))
+		brkp = KPROBE_THUMB32_BREAKPOINT_INSTRUCTION;
+	else
+		brkp = KPROBE_THUMB16_BREAKPOINT_INSTRUCTION;
+#else
+	kprobe_opcode_t insn = p->opcode;
 
-		if (insn >= 0xe0000000)
-			brkp |= 0xe0000000;  /* Unconditional instruction */
-		else
-			brkp |= insn & 0xf0000000;  /* Copy condition from insn */
-	}
+	addr = p->addr;
+	brkp = KPROBE_ARM_BREAKPOINT_INSTRUCTION;
+
+	if (insn >= 0xe0000000)
+		brkp |= 0xe0000000;  /* Unconditional instruction */
+	else
+		brkp |= insn & 0xf0000000;  /* Copy condition from insn */
+#endif
 
 	patch_text(addr, brkp);
 }
diff --git a/arch/arm/kernel/patch.c b/arch/arm/kernel/patch.c
index 07314af..2d1995c 100644
--- a/arch/arm/kernel/patch.c
+++ b/arch/arm/kernel/patch.c
@@ -15,13 +15,13 @@ struct patch {
 
 void __kprobes __patch_text(void *addr, unsigned int insn)
 {
-	bool thumb2 = IS_ENABLED(CONFIG_THUMB2_KERNEL);
-	int size;
+	int size = 0;
 
-	if (thumb2 && __opcode_is_thumb16(insn)) {
+#ifdef CONFIG_THUMB2_KERNEL
+	if (__opcode_is_thumb16(insn)) {
 		*(u16 *)addr = __opcode_to_mem_thumb16(insn);
 		size = sizeof(u16);
-	} else if (thumb2 && ((uintptr_t)addr & 2)) {
+	} else if (((uintptr_t)addr & 2)) {
 		u16 first = __opcode_thumb32_first(insn);
 		u16 second = __opcode_thumb32_second(insn);
 		u16 *addrh = addr;
@@ -30,12 +30,10 @@ void __kprobes __patch_text(void *addr, unsigned int insn)
 		addrh[1] = __opcode_to_mem_thumb16(second);
 
 		size = sizeof(u32);
-	} else {
-		if (thumb2)
-			insn = __opcode_to_mem_thumb32(insn);
-		else
-			insn = __opcode_to_mem_arm(insn);
-
+	}
+#endif
+	if (!size) {
+		insn = __opcode_to_mem(insn);
 		*(u32 *)addr = insn;
 		size = sizeof(u32);
 	}
@@ -63,10 +61,14 @@ void __kprobes patch_text(void *addr, unsigned int insn)
 	if (cache_ops_need_broadcast()) {
 		stop_machine(patch_text_stop_machine, &patch, cpu_online_mask);
 	} else {
-		bool straddles_word = IS_ENABLED(CONFIG_THUMB2_KERNEL)
-				      && __opcode_is_thumb32(insn)
-				      && ((uintptr_t)addr & 2);
+		bool straddles_word;
 
+#ifdef CONFIG_THUMB2_KERNEL
+		straddles_word = __opcode_is_thumb32(insn)
+				      && ((uintptr_t)addr & 2);
+#else
+		straddles_word = 0;
+#endif
 		if (straddles_word)
 			stop_machine(patch_text_stop_machine, &patch, NULL);
 		else
-- 
1.7.9.1

[PATCH 3/5] drivers/net: remove IS_ENABLED usage from wiznet drivers

[Paul Gortmaker]

The use of IS_ENABLED in C code (vs just in CPP #if directives)
causes us to carry the burden of a huge autoconf.h file.  It is
also misleading in that a casual inspection of the code would
leave one thinking that the if statements were evaluated at
runtime, when TX_FLOW is a Kconfig bool and hence evaluated
at configure time as an either/or.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 drivers/net/ethernet/wiznet/w5100.c |    9 ++++++---
 drivers/net/ethernet/wiznet/w5300.c |   18 ++++++++++++------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index c28e1d5..fdac57d 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -441,8 +441,9 @@ static int w5100_start_tx(struct sk_buff *skb, struct net_device *ndev)
 	struct w5100_priv *priv = netdev_priv(ndev);
 	u16 offset;
 
-	if (IS_ENABLED(CONFIG_WIZNET_TX_FLOW))
-		netif_stop_queue(ndev);
+#ifdef CONFIG_WIZNET_TX_FLOW
+	netif_stop_queue(ndev);
+#endif
 
 	offset = w5100_read16(priv, W5100_S0_TX_WR);
 	w5100_writebuf(priv, offset, skb->data, skb->len);
@@ -517,10 +518,12 @@ static irqreturn_t w5100_interrupt(int irq, void *ndev_instance)
 	w5100_write(priv, W5100_S0_IR, ir);
 	mmiowb();
 
-	if (IS_ENABLED(CONFIG_WIZNET_TX_FLOW) && (ir & S0_IR_SENDOK)) {
+#ifdef CONFIG_WIZNET_TX_FLOW
+	if (ir & S0_IR_SENDOK) {
 		netif_dbg(priv, tx_done, ndev, "tx done\n");
 		netif_wake_queue(ndev);
 	}
+#endif
 
 	if (ir & S0_IR_RECV) {
 		if (napi_schedule_prep(&priv->napi)) {
diff --git a/drivers/net/ethernet/wiznet/w5300.c b/drivers/net/ethernet/wiznet/w5300.c
index 88afde9..294b3fd 100644
--- a/drivers/net/ethernet/wiznet/w5300.c
+++ b/drivers/net/ethernet/wiznet/w5300.c
@@ -73,6 +73,11 @@ MODULE_LICENSE("GPL");
 #define W5300_S0_IR		0x0206	/* S0 Interrupt Register */
 #define   S0_IR_RECV		  0x0004  /* Receive interrupt */
 #define   S0_IR_SENDOK		  0x0010  /* Send OK interrupt */
+#ifdef CONFIG_WIZNET_TX_FLOW
+#define   S0_IR_FLAGS		  (S0_IR_RECV | S0_IR_SENDOK)
+#else
+#define   S0_IR_FLAGS		  S0_IR_RECV
+#endif
 #define W5300_S0_SSR		0x0208	/* S0 Socket Status Register */
 #define W5300_S0_TX_WRSR	0x0220	/* S0 TX Write Size Register */
 #define W5300_S0_TX_FSR		0x0224	/* S0 TX Free Size Register */
@@ -273,9 +278,7 @@ static void w5300_hw_start(struct w5300_priv *priv)
 			  S0_MR_MACRAW : S0_MR_MACRAW_MF);
 	mmiowb();
 	w5300_command(priv, S0_CR_OPEN);
-	w5300_write(priv, W5300_S0_IMR, IS_ENABLED(CONFIG_WIZNET_TX_FLOW) ?
-					S0_IR_RECV | S0_IR_SENDOK :
-					S0_IR_RECV);
+	w5300_write(priv, W5300_S0_IMR, S0_IR_FLAGS);
 	w5300_write(priv, W5300_IMR, IR_S0);
 	mmiowb();
 }
@@ -371,8 +374,9 @@ static int w5300_start_tx(struct sk_buff *skb, struct net_device *ndev)
 {
 	struct w5300_priv *priv = netdev_priv(ndev);
 
-	if (IS_ENABLED(CONFIG_WIZNET_TX_FLOW))
-		netif_stop_queue(ndev);
+#ifdef CONFIG_WIZNET_TX_FLOW
+	netif_stop_queue(ndev);
+#endif
 
 	w5300_write_frame(priv, skb->data, skb->len);
 	mmiowb();
@@ -439,10 +443,12 @@ static irqreturn_t w5300_interrupt(int irq, void *ndev_instance)
 	w5300_write(priv, W5300_S0_IR, ir);
 	mmiowb();
 
-	if (IS_ENABLED(CONFIG_WIZNET_TX_FLOW) && (ir & S0_IR_SENDOK)) {
+#ifdef CONFIG_WIZNET_TX_FLOW
+	if (ir & S0_IR_SENDOK) {
 		netif_dbg(priv, tx_done, ndev, "tx done\n");
 		netif_wake_queue(ndev);
 	}
+#endif
 
 	if (ir & S0_IR_RECV) {
 		if (napi_schedule_prep(&priv->napi)) {
-- 
1.7.9.1

Re: [PATCH 3/5] drivers/net: remove IS_ENABLED usage from wiznet drivers

[David Miller]

From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Wed, 11 Apr 2012 19:50:55 -0400

> The use of IS_ENABLED in C code (vs just in CPP #if directives)
> causes us to carry the burden of a huge autoconf.h file.  It is
> also misleading in that a casual inspection of the code would
> leave one thinking that the if statements were evaluated at
> runtime, when TX_FLOW is a Kconfig bool and hence evaluated
> at configure time as an either/or.
> 
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>

Want me to toss this into net-next?

Re: [PATCH 3/5] drivers/net: remove IS_ENABLED usage from wiznet drivers

[Stephen Rothwell]

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

Hi Dave,

On Wed, 11 Apr 2012 19:54:44 -0400 (EDT) David Miller <davem@davemloft.net> wrote:
>
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Wed, 11 Apr 2012 19:50:55 -0400
> 
> > The use of IS_ENABLED in C code (vs just in CPP #if directives)
> > causes us to carry the burden of a huge autoconf.h file.  It is
> > also misleading in that a casual inspection of the code would
> > leave one thinking that the if statements were evaluated at
> > runtime, when TX_FLOW is a Kconfig bool and hence evaluated
> > at configure time as an either/or.
> > 
> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> Want me to toss this into net-next?

Sure - *after* it has been reviewed, and tested :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: [PATCH 3/5] drivers/net: remove IS_ENABLED usage from wiznet drivers

[Stephen Rothwell]

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

Hi Dave,

On Thu, 12 Apr 2012 10:06:19 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Sure - *after* it has been reviewed, and tested :-)

Sorry, overreaction with limited context and less limited irritation  :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: [PATCH 3/5] drivers/net: remove IS_ENABLED usage from wiznet drivers

[Paul Gortmaker]

On 12-04-11 07:54 PM, David Miller wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Wed, 11 Apr 2012 19:50:55 -0400
> 
>> The use of IS_ENABLED in C code (vs just in CPP #if directives)
>> causes us to carry the burden of a huge autoconf.h file.  It is
>> also misleading in that a casual inspection of the code would
>> leave one thinking that the if statements were evaluated at
>> runtime, when TX_FLOW is a Kconfig bool and hence evaluated
>> at configure time as an either/or.
>>
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> Want me to toss this into net-next?

Yes, that makes sense since wiznet only exists there currently.

Thanks,
Paul.

Re: [PATCH 3/5] drivers/net: remove IS_ENABLED usage from wiznet drivers

[Paul Gortmaker]

On 12-04-11 07:54 PM, David Miller wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Wed, 11 Apr 2012 19:50:55 -0400
> 
>> The use of IS_ENABLED in C code (vs just in CPP #if directives)
>> causes us to carry the burden of a huge autoconf.h file.  It is
>> also misleading in that a casual inspection of the code would
>> leave one thinking that the if statements were evaluated at
>> runtime, when TX_FLOW is a Kconfig bool and hence evaluated
>> at configure time as an either/or.
>>
>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> 
> Want me to toss this into net-next?

Yes, that makes sense since wiznet only exists there currently.

Thanks,
Paul.

[PATCH 4/5] Revert "kconfig: fix __enabled_ macros definition for invisible and un-selected symbols"

[Paul Gortmaker]

This reverts commit 953742c8fe8ac45be453fee959d7be40cd89f920.

Dumping two lines into autoconf.h for all existing Kconfig
options results in a giant file (~16k lines) we have to process
each time.  Instead, lets work to just ensure things we care
about have definitions and we'll only have about 1k lines.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 scripts/kconfig/confdata.c |   49 +++++++++++--------------------------------
 1 files changed, 13 insertions(+), 36 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 0586085..9d06744 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -489,6 +489,17 @@ header_print_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg)
 			fprintf(fp, "#define %s%s%s 1\n",
 			    CONFIG_, sym->name, suffix);
 		}
+		/*
+		 * Generate the __enabled_CONFIG_* and
+		 * __enabled_CONFIG_*_MODULE macros for use by the
+		 * IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is
+		 * generated even for booleans so that the IS_ENABLED() macro
+		 * works.
+		 */
+		fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n",
+				sym->name, (*value == 'y'));
+		fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n",
+				sym->name, (*value == 'm'));
 		break;
 	}
 	case S_HEX: {
@@ -540,35 +551,6 @@ static struct conf_printer header_printer_cb =
 };
 
 /*
- * Generate the __enabled_CONFIG_* and __enabled_CONFIG_*_MODULE macros for
- * use by the IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is
- * generated even for booleans so that the IS_ENABLED() macro works.
- */
-static void
-header_print__enabled_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg)
-{
-
-	switch (sym->type) {
-	case S_BOOLEAN:
-	case S_TRISTATE: {
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n",
-		    sym->name, (*value == 'y'));
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n",
-		    sym->name, (*value == 'm'));
-		break;
-	}
-	default:
-		break;
-	}
-}
-
-static struct conf_printer header__enabled_printer_cb =
-{
-	.print_symbol = header_print__enabled_symbol,
-	.print_comment = header_print_comment,
-};
-
-/*
  * Tristate printer
  *
  * This printer is used when generating the `include/config/tristate.conf' file.
@@ -949,16 +931,11 @@ int conf_write_autoconf(void)
 	conf_write_heading(out_h, &header_printer_cb, NULL);
 
 	for_all_symbols(i, sym) {
-		if (!sym->name)
-			continue;
-
 		sym_calc_value(sym);
-
-		conf_write_symbol(out_h, sym, &header__enabled_printer_cb, NULL);
-
-		if (!(sym->flags & SYMBOL_WRITE))
+		if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
 			continue;
 
+		/* write symbol to auto.conf, tristate and header files */
 		conf_write_symbol(out, sym, &kconfig_printer_cb, (void *)1);
 
 		conf_write_symbol(tristate, sym, &tristate_printer_cb, (void *)1);
-- 
1.7.9.1

[PATCH 5/5] kconfig: limit IS_ENABLED & similar to CPP usage

[Paul Gortmaker]

Using IS_ENABLED() within C (vs. within CPP #if statements) requires
us to actually define every possible bool/tristate Kconfig option
twice (__enabled_* and __enabled_*_MODULE variants).

Commit 953742c8fe8ac45be453fee959d7be40cd89f920

  "kconfig: fix __enabled_ macros definition for invisible
   and un-selected symbols"

caused the autoconf.h file to grow by a large factor, since it
now defined tens of thousands(!) of __enabled_CONFIG* items.

With the few C users that crept in over the last couple of releases
gone, we can now limit these items to be just for CPP usage.  And
in doing so, just emit lines where the options are really enabled
as modules or built in.

In doing so, the defconfig x86_64 autoconf.h size drops significantly:

	before:	15934 of 16943 total are __enabled_CONFIG* lines
	after:	943 of 1952 total are __enabled_CONFIG* lines

The numbers for x86_64 allnoconfig are even more drastic:

	before:	15934 of 16200 total are __enabled_CONFIG* lines
	after:	224 of 490 total are __enabled_CONFIG* lines

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 include/linux/kconfig.h    |    8 ++++----
 scripts/kconfig/confdata.c |   14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 067eda0..5fa3f1b 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -4,7 +4,7 @@
 #include <generated/autoconf.h>
 
 /*
- * Helper macros to use CONFIG_ options in C expressions. Note that
+ * Helper macros to use CONFIG_ options in CPP #if expressions. Note that
  * these only work with boolean and tristate options.
  */
 
@@ -14,19 +14,19 @@
  *
  */
 #define IS_ENABLED(option) \
-	(__enabled_ ## option || __enabled_ ## option ## _MODULE)
+	(defined(__enabled_ ## option) || defined(__enabled_ ## option ## _MODULE))
 
 /*
  * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
  * otherwise. For boolean options, this is equivalent to
  * IS_ENABLED(CONFIG_FOO).
  */
-#define IS_BUILTIN(option) __enabled_ ## option
+#define IS_BUILTIN(option) defined(__enabled_ ## option)
 
 /*
  * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
  * otherwise.
  */
-#define IS_MODULE(option) __enabled_ ## option ## _MODULE
+#define IS_MODULE(option) defined(__enabled_ ## option ## _MODULE)
 
 #endif /* __LINUX_KCONFIG_H */
diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 9d06744..3651ee7 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -492,14 +492,14 @@ header_print_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg)
 		/*
 		 * Generate the __enabled_CONFIG_* and
 		 * __enabled_CONFIG_*_MODULE macros for use by the
-		 * IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is
-		 * generated even for booleans so that the IS_ENABLED() macro
-		 * works.
+		 * IS_{ENABLED,BUILTIN,MODULE} cpp helpers.
 		 */
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n",
-				sym->name, (*value == 'y'));
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n",
-				sym->name, (*value == 'm'));
+		if (*value == 'y')
+			fprintf(fp, "#define __enabled_" CONFIG_ "%s\n",
+				sym->name);
+		if (*value == 'm')
+			fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE\n",
+				sym->name);
 		break;
 	}
 	case S_HEX: {
-- 
1.7.9.1

Re: [PATCH 5/5] kconfig: limit IS_ENABLED & similar to CPP usage

[Linus Torvalds]

On Wed, Apr 11, 2012 at 4:50 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
> Using IS_ENABLED() within C (vs. within CPP #if statements) requires
> us to actually define every possible bool/tristate Kconfig option
> twice (__enabled_* and __enabled_*_MODULE variants).

Why do you keep the __enabled_*[_MODULE] things alive at all?

Why can't you just check the CONFIG_xyz[_MODULE] #defines directly?

IOW, why isn't IS_ENABLED() just

  #define IS_ENABLED(option) \
    (defined(CONFIG_##option) || defined(CONFIG_##option##_MODULE))

  #define IS_BUILTIN(option) \
    (defined(CONFIG_##option))

and we're done with it all? What's the advantage of the __enabled_
thing again, when you depend on the preprocessor anyway?

                      Linus

Re: [PATCH 5/5] kconfig: limit IS_ENABLED & similar to CPP usage

[Paul Gortmaker]

On 12-04-11 08:04 PM, Linus Torvalds wrote:
> On Wed, Apr 11, 2012 at 4:50 PM, Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
>> Using IS_ENABLED() within C (vs. within CPP #if statements) requires
>> us to actually define every possible bool/tristate Kconfig option
>> twice (__enabled_* and __enabled_*_MODULE variants).
> 
> Why do you keep the __enabled_*[_MODULE] things alive at all?

Good point.  Clearly I wasn't aggressive enough in my shitectomy.
Testing with no __enabled_* stuff now.  That reduces the line
count even more, which is nice.

I've not auto-prefixed the CONFIG_ though, since that seems to break
the grep use case.

P.

> 
> Why can't you just check the CONFIG_xyz[_MODULE] #defines directly?
> 
> IOW, why isn't IS_ENABLED() just
> 
>   #define IS_ENABLED(option) \
>     (defined(CONFIG_##option) || defined(CONFIG_##option##_MODULE))
> 
>   #define IS_BUILTIN(option) \
>     (defined(CONFIG_##option))
> 
> and we're done with it all? What's the advantage of the __enabled_
> thing again, when you depend on the preprocessor anyway?
> 
>                       Linus

Re: [PATCH 5/5] kconfig: limit IS_ENABLED & similar to CPP usage

[Paul Gortmaker]

On 12-04-11 08:04 PM, Linus Torvalds wrote:
> On Wed, Apr 11, 2012 at 4:50 PM, Paul Gortmaker
> <paul.gortmaker@windriver.com> wrote:
>> Using IS_ENABLED() within C (vs. within CPP #if statements) requires
>> us to actually define every possible bool/tristate Kconfig option
>> twice (__enabled_* and __enabled_*_MODULE variants).
> 
> Why do you keep the __enabled_*[_MODULE] things alive at all?

Good point.  Clearly I wasn't aggressive enough in my shitectomy.
Testing with no __enabled_* stuff now.  That reduces the line
count even more, which is nice.

I've not auto-prefixed the CONFIG_ though, since that seems to break
the grep use case.

P.

> 
> Why can't you just check the CONFIG_xyz[_MODULE] #defines directly?
> 
> IOW, why isn't IS_ENABLED() just
> 
>   #define IS_ENABLED(option) \
>     (defined(CONFIG_##option) || defined(CONFIG_##option##_MODULE))
> 
>   #define IS_BUILTIN(option) \
>     (defined(CONFIG_##option))
> 
> and we're done with it all? What's the advantage of the __enabled_
> thing again, when you depend on the preprocessor anyway?
> 
>                       Linus

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Linus Torvalds]

On Wed, Apr 11, 2012 at 4:50 PM, Paul Gortmaker
<paul.gortmaker@windriver.com> wrote:
>
> Commit ce1744f4ed20ca8 seems to endorse C users of IS_ENABLED, and
> it went through Linus and Andrew, so I'm not sure scrapping C users
> as a solution will be universally agreeable...

I think we can easily drop IS_ENABLED() if it has been causing
problems. And it clearly is.

And I had never even looked at that generated file. It's disgusting.
We need to stop doing that. It must be driving compile-times up too -
just parsing that monster is RealWork(tm).

                  Linus

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Linus Torvalds]

On Wed, Apr 11, 2012 at 4:58 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I think we can easily drop IS_ENABLED() if it has been causing
> problems. And it clearly is.
>
> And I had never even looked at that generated file. It's disgusting.
> We need to stop doing that. It must be driving compile-times up too -
> just parsing that monster is RealWork(tm).

Oh, but I have a sick and twisted mind. And I'm incredibly smart and
photogenic too.

Here's an implementation of IS_ENABLED() that works in C. Well,
"gcc-C". It depends on the optimizer DTRT, but gcc really does seem to
do it.

Ready? You *will* go blind - blinded by the pure beauty and intellect
in this thing:

    #define IS_DEFINED(x) (__stringify(CONFIG_##x)[0]=='1')

That really is a piece of art. I'm expecting that the Guggenheim will
contact me any moment now about it.

Of course, the above does *not* work for a pre-processor, but hey, you
can't have everything. It literally is *only* valid in C code. You
can't have everything. In the pre-processor, you can just use

   #if CONFIG_OPTION

like you always could, or use another helper macro for the "defined(x)" magic.

                      Linus

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Andrew Morton]

On Wed, 11 Apr 2012 18:19:58 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:

>    #define IS_DEFINED(x) (__stringify(CONFIG_##x)[0]=='1')
> 
> That really is a piece of art. I'm expecting that the Guggenheim will
> contact me any moment now about it.

Several weeks ago I asked the assemblage if anyone could think of a way
of doing this.  It seems that I failed to Cc sufficiently perverted
parties.

> Of course, the above does *not* work for a pre-processor, but hey, you
> can't have everything.  It literally is *only* valid in C code. You
> can't have everything. 

So we shouldn't call it IS_DEFINED() - that would cause confusion with
the old version.

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Linus Torvalds]

On Wed, Apr 11, 2012 at 6:45 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> So we shouldn't call it IS_DEFINED() - that would cause confusion with
> the old version.

Yeah. Maybe we should call it "config_enabled()" - the lower-case
hopefully makes it clear that it acts as a C function, not a
preprocessor one.

Code like "if (config_enabled(IPV6))" looks reasonably readable to me.

I asked G+ if they can come up with a better trick. They did well on
the whole bitmask calculations, maybe somebody can figure out how to
make the same macro work for both preprocessor and C code...

                       Linus

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Stephen Rothwell]

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

Hi Linus,

On Wed, 11 Apr 2012 18:55:10 -0700 Linus Torvalds <torvalds@linux-foundation.org> wrote:
>
> On Wed, Apr 11, 2012 at 6:45 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > So we shouldn't call it IS_DEFINED() - that would cause confusion with
> > the old version.
> 
> Yeah. Maybe we should call it "config_enabled()" - the lower-case
> hopefully makes it clear that it acts as a C function, not a
> preprocessor one.
> 
> Code like "if (config_enabled(IPV6))" looks reasonably readable to me.
> 
> I asked G+ if they can come up with a better trick. They did well on
> the whole bitmask calculations, maybe somebody can figure out how to
> make the same macro work for both preprocessor and C code...

Can we leave the CONFIG_ part in the caller, so they are still grepable, please?

So:

	#define	config_enabled(x)	(__stringify(x)[0] == '1')

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

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

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Linus Torvalds]

On Wed, Apr 11, 2012 at 8:10 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> Can we leave the CONFIG_ part in the caller, so they are still grepable, please?
>
> So:
>
>        #define config_enabled(x)       (__stringify(x)[0] == '1')

Sure. Except G+ came through, and we now have an even better hack for
this, that works entirely in the preprocessor.

That also means that you can use it for static initializers etc, so you can do

   int some_value = config_enabled(CONFIG_ENABLE_XYZ_BY_DEFAULT);

Paul, do you want to write the patch to change IS_ENABLED() to use this trick?

Trick courtesy of "comex ." on G+ (please do give him credit even if
he doesn't seem to use his real name):

  #define is_set(macro) is_set_(macro)
  #define macrotest_1 ,
  #define is_set_(value) is_set__(macrotest_##value)
  #define is_set__(comma) is_set___(comma 1, 0)
  #define is_set___(_, v, ...) v

and now "is_set(CONFIG_xyzzy)" works, and gives either 1 or 0 directly
in the preprocessor.

Obviously the names have to be changed here (let's not use
"macrotest_1" - make it something more internal), but the concept is
solid, and I would actually suggest having this as a generic "is this
macro set to 1" helper first, and then implement IS_ENABLED() in terms
of that.

My likee.

                       Linus

[PATCH 0/3] RFC v2: strip 15,000 lines from a typical autoconf.h

[Paul Gortmaker]

[Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h] On 12/04/2012 (Thu 12:29) Linus Torvalds wrote:

> On Wed, Apr 11, 2012 at 8:10 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> >
> > Can we leave the CONFIG_ part in the caller, so they are still grepable, please?
> >
> > So:
> >
> >	#define config_enabled(x)	(__stringify(x)[0] == '1')
> 
> Sure. Except G+ came through, and we now have an even better hack for
> this, that works entirely in the preprocessor.
> 
> That also means that you can use it for static initializers etc, so you can do
> 
>    int some_value = config_enabled(CONFIG_ENABLE_XYZ_BY_DEFAULT);
> 
> Paul, do you want to write the patch to change IS_ENABLED() to use this trick?

Sure -- I've done this in patch #1 of the (now) three patches.

I've dropped all the patches relating to phasing out the users
of IS_ENABLED within C code, since now they should just work.

> 
> Trick courtesy of "comex ." on G+ (please do give him credit even if
> he doesn't seem to use his real name):
> 
>   #define is_set(macro) is_set_(macro)
>   #define macrotest_1 ,
>   #define is_set_(value) is_set__(macrotest_##value)
>   #define is_set__(comma) is_set___(comma 1, 0)
>   #define is_set___(_, v, ...) v
> 
> and now "is_set(CONFIG_xyzzy)" works, and gives either 1 or 0 directly
> in the preprocessor.
> 
> Obviously the names have to be changed here (let's not use
> "macrotest_1" - make it something more internal), but the concept is
> solid, and I would actually suggest having this as a generic "is this
> macro set to 1" helper first, and then implement IS_ENABLED() in terms
> of that.

Hopefully the names I've chosen are OK.  Please have a look.

Thanks,
Paul.

> 
> My likee.
> 
>                        Linus

Paul Gortmaker (3):
  kconfig: fix IS_ENABLED to not require all options to be defined
  Revert "kconfig: fix __enabled_ macros definition for invisible and
    un-selected symbols"
  kconfig: delete last traces of __enabled_ from autoconf.h

 include/linux/kconfig.h    |   22 ++++++++++++++++++----
 scripts/kconfig/confdata.c |   38 ++------------------------------------
 2 files changed, 20 insertions(+), 40 deletions(-)

-- 
1.7.9.1

[PATCH 1/3] kconfig: fix IS_ENABLED to not require all options to be defined

[Paul Gortmaker]

Using IS_ENABLED() within C (vs. within CPP #if statements) in its
current form requires us to actually define every possible bool/tristate
Kconfig option twice (__enabled_* and __enabled_*_MODULE variants).

This results in a huge autoconf.h file, on the order of 16k lines
for a x86_64 defconfig.

Fixing IS_ENABLED to be able to work on the smaller subset of just
things that we really have defined is step one to fixing this.  Which
means it has to not choke when fed non-enabled options, such as:

  include/linux/netdevice.h:964:1: warning: "__enabled_CONFIG_FCOE_MODULE" is not defined [-Wundef]

The original prototype of how to implement a C and preprocessor
compatible way of doing this came from the Google+ user "comex ."
in response to Linus' crowdsourcing challenge for a possible
improvement on his earlier C specific solution:

	#define config_enabled(x)       (__stringify(x)[0] == '1')

In this implementation, I've chosen variable names that hopefully
make how it works more understandable.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 include/linux/kconfig.h |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index 067eda0..be342b9 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -4,29 +4,43 @@
 #include <generated/autoconf.h>
 
 /*
- * Helper macros to use CONFIG_ options in C expressions. Note that
+ * Helper macros to use CONFIG_ options in C/CPP expressions. Note that
  * these only work with boolean and tristate options.
  */
 
 /*
+ * Getting something that works in C and CPP for an arg that may or may
+ * not be defined is tricky.  Here, if we have "#define CONFIG_BOOGER 1"
+ * we match on the placeholder define, insert the "0," for arg1 and generate
+ * the triplet (0, 1, 0).  Then the last step cherry picks the 2nd arg (a one).
+ * When CONFIG_BOOGER is not defined, we generate a (... 1, 0) pair, and when
+ * the last step cherry picks the 2nd arg, we get a zero.
+ */
+#define __ARG_PLACEHOLDER_1 0,
+#define config_enabled(cfg) _config_enabled(cfg)
+#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
+#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
+#define ___config_enabled(__ignored, val, ...) val
+
+/*
  * IS_ENABLED(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y' or 'm',
  * 0 otherwise.
  *
  */
 #define IS_ENABLED(option) \
-	(__enabled_ ## option || __enabled_ ## option ## _MODULE)
+	(config_enabled(option) || config_enabled(option##_MODULE))
 
 /*
  * IS_BUILTIN(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'y', 0
  * otherwise. For boolean options, this is equivalent to
  * IS_ENABLED(CONFIG_FOO).
  */
-#define IS_BUILTIN(option) __enabled_ ## option
+#define IS_BUILTIN(option) config_enabled(option)
 
 /*
  * IS_MODULE(CONFIG_FOO) evaluates to 1 if CONFIG_FOO is set to 'm', 0
  * otherwise.
  */
-#define IS_MODULE(option) __enabled_ ## option ## _MODULE
+#define IS_MODULE(option) config_enabled(option##_MODULE)
 
 #endif /* __LINUX_KCONFIG_H */
-- 
1.7.9.1

Re: [PATCH 1/3] kconfig: fix IS_ENABLED to not require all options to be defined

[Dick Streefland]

| +#define __ARG_PLACEHOLDER_1 0,
| +#define config_enabled(cfg) _config_enabled(cfg)
| +#define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
| +#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
| +#define ___config_enabled(__ignored, val, ...) val

The ISO C99 standard requires that the variable argument macro
___config_enabled() is expanded with at least three arguments. When
a CONFIG_* macro is not defined, there are only two arguments, and
gcc will issue a somewhat cryptic warning message when you compile
with the -pedantic option:

  "warning: ISO C99 requires rest arguments to be used"

Adding an additional dummy argument makes the code ISO C99 compliant,
and eliminates the warning:

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index be342b9..eac65da 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -19,7 +19,7 @@
 #define __ARG_PLACEHOLDER_1 0,
 #define config_enabled(cfg) _config_enabled(cfg)
 #define _config_enabled(value) __config_enabled(__ARG_PLACEHOLDER_##value)
-#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0)
+#define __config_enabled(arg1_or_junk) ___config_enabled(arg1_or_junk 1, 0, 0)
 #define ___config_enabled(__ignored, val, ...) val
 
 /*

Signed-off-by: Dick Streefland <dick@streefland.net>

-- 
Dick

[PATCH 2/3] Revert "kconfig: fix __enabled_ macros definition for invisible and un-selected symbols"

[Paul Gortmaker]

This reverts commit 953742c8fe8ac45be453fee959d7be40cd89f920.

Dumping two lines into autoconf.h for all existing Kconfig
options results in a giant file (~16k lines) we have to process
each time we compile something.  We've weaned IS_ENABLED()
and similar off of requiring the __enabled_ definitions so
now we can revert the change which caused all the extra lines.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 scripts/kconfig/confdata.c |   49 +++++++++++--------------------------------
 1 files changed, 13 insertions(+), 36 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 0586085..9d06744 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -489,6 +489,17 @@ header_print_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg)
 			fprintf(fp, "#define %s%s%s 1\n",
 			    CONFIG_, sym->name, suffix);
 		}
+		/*
+		 * Generate the __enabled_CONFIG_* and
+		 * __enabled_CONFIG_*_MODULE macros for use by the
+		 * IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is
+		 * generated even for booleans so that the IS_ENABLED() macro
+		 * works.
+		 */
+		fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n",
+				sym->name, (*value == 'y'));
+		fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n",
+				sym->name, (*value == 'm'));
 		break;
 	}
 	case S_HEX: {
@@ -540,35 +551,6 @@ static struct conf_printer header_printer_cb =
 };
 
 /*
- * Generate the __enabled_CONFIG_* and __enabled_CONFIG_*_MODULE macros for
- * use by the IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is
- * generated even for booleans so that the IS_ENABLED() macro works.
- */
-static void
-header_print__enabled_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg)
-{
-
-	switch (sym->type) {
-	case S_BOOLEAN:
-	case S_TRISTATE: {
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n",
-		    sym->name, (*value == 'y'));
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n",
-		    sym->name, (*value == 'm'));
-		break;
-	}
-	default:
-		break;
-	}
-}
-
-static struct conf_printer header__enabled_printer_cb =
-{
-	.print_symbol = header_print__enabled_symbol,
-	.print_comment = header_print_comment,
-};
-
-/*
  * Tristate printer
  *
  * This printer is used when generating the `include/config/tristate.conf' file.
@@ -949,16 +931,11 @@ int conf_write_autoconf(void)
 	conf_write_heading(out_h, &header_printer_cb, NULL);
 
 	for_all_symbols(i, sym) {
-		if (!sym->name)
-			continue;
-
 		sym_calc_value(sym);
-
-		conf_write_symbol(out_h, sym, &header__enabled_printer_cb, NULL);
-
-		if (!(sym->flags & SYMBOL_WRITE))
+		if (!(sym->flags & SYMBOL_WRITE) || !sym->name)
 			continue;
 
+		/* write symbol to auto.conf, tristate and header files */
 		conf_write_symbol(out, sym, &kconfig_printer_cb, (void *)1);
 
 		conf_write_symbol(tristate, sym, &tristate_printer_cb, (void *)1);
-- 
1.7.9.1

[PATCH 3/3] kconfig: delete last traces of __enabled_ from autoconf.h

[Paul Gortmaker]

We've now fixed IS_ENABLED() and friends to not require any
special "__enabled_" prefixed versions of the normal Kconfig
options, so delete the last traces of them being generated.

Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
---
 scripts/kconfig/confdata.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 9d06744..52577f0 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -489,17 +489,6 @@ header_print_symbol(FILE *fp, struct symbol *sym, const char *value, void *arg)
 			fprintf(fp, "#define %s%s%s 1\n",
 			    CONFIG_, sym->name, suffix);
 		}
-		/*
-		 * Generate the __enabled_CONFIG_* and
-		 * __enabled_CONFIG_*_MODULE macros for use by the
-		 * IS_{ENABLED,BUILTIN,MODULE} macros. The _MODULE variant is
-		 * generated even for booleans so that the IS_ENABLED() macro
-		 * works.
-		 */
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s %d\n",
-				sym->name, (*value == 'y'));
-		fprintf(fp, "#define __enabled_" CONFIG_ "%s_MODULE %d\n",
-				sym->name, (*value == 'm'));
 		break;
 	}
 	case S_HEX: {
-- 
1.7.9.1

Re: [PATCH 0/5] RFC: strip 15,000 lines from a typical autoconf.h

[Michal Marek]

On 12.4.2012 03:19, Linus Torvalds wrote:
> On Wed, Apr 11, 2012 at 4:58 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>>
>> I think we can easily drop IS_ENABLED() if it has been causing
>> problems. And it clearly is.

What is really causing problems is that kconfig has no view of the
entire kernel configuration.


>> And I had never even looked at that generated file. It's disgusting.
>> We need to stop doing that. It must be driving compile-times up too -
>> just parsing that monster is RealWork(tm).

You forgot that it also contributes to global warming.


> Oh, but I have a sick and twisted mind. And I'm incredibly smart and
> photogenic too.
> 
> Here's an implementation of IS_ENABLED() that works in C. Well,
> "gcc-C". It depends on the optimizer DTRT, but gcc really does seem to
> do it.
> 
> Ready? You *will* go blind - blinded by the pure beauty and intellect
> in this thing:
> 
>     #define IS_DEFINED(x) (__stringify(CONFIG_##x)[0]=='1')

Nice. Maybe we could make it simply

#define IS_DEFINED(x) (__stringify(x)[0]=='1')

so that the users are easily grepable and it is obvious that a config
option is being tested, like in the current implementation. The only
thing we lose is the check if 'x' is really a boolean or tristate
option. But it turns out that even now this does not work in all cases.

Michal