All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS
@ 2014-02-22 19:08 Josh Triplett
  2014-02-22 19:09 ` [PATCH 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
                   ` (2 more replies)
  0 siblings, 3 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 19:08 UTC (permalink / raw)
  To: Andi Kleen, Andrew Morton, Borislav Petkov, Feng Tang,
	H. Peter Anvin, Ingo Molnar, Jacob Shin, Jan Beulich,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra, Qiaowei Ren,
	Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, Thomas Renninger,
	linux-kernel, x86

arch/x86/kernel/cpu/proc.c only exists to support files in /proc; omit that
file when compiling without CONFIG_PROC_FS.

Saves 645 additional bytes on 32-bit x86 when !CONFIG_PROC_FS:

add/remove: 0/5 grow/shrink: 0/0 up/down: 0/-645 (-645)
function                                     old     new   delta
c_stop                                         1       -      -1
c_next                                        11       -     -11
cpuinfo_op                                    16       -     -16
c_start                                       24       -     -24
show_cpuinfo                                 593       -    -593

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 arch/x86/kernel/cpu/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 7fd54f0..64038d8 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -13,10 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o scattered.o topology.o
-obj-y			+= proc.o capflags.o powerflags.o common.o
+obj-y			+= capflags.o powerflags.o common.o
 obj-y			+= rdrand.o
 obj-y			+= match.o
 
+obj-$(CONFIG_PROC_FS)	+= proc.o
+
 obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:08 [PATCH 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
@ 2014-02-22 19:09 ` Josh Triplett
  2014-02-22 19:16   ` H. Peter Anvin
  2014-02-22 19:19   ` H. Peter Anvin
  2014-02-22 19:55 ` [PATCH v2 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
  2014-02-22 19:57 ` [PATCH v2 " Josh Triplett
  2 siblings, 2 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 19:09 UTC (permalink / raw)
  To: Andi Kleen, Andrew Morton, Borislav Petkov, Feng Tang,
	H. Peter Anvin, Ingo Molnar, Jacob Shin, Jan Beulich,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra, Qiaowei Ren,
	Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, Thomas Renninger,
	linux-kernel, x86

The table mapping CPUID bits to human-readable strings takes up a
non-trivial amount of space, and only exists to support /proc/cpuinfo
and a couple of kernel messages.  Since programs depend on the format of
/proc/cpuinfo, force inclusion of the table when building with /proc
support; otherwise, support omitting that table to save space, in which
case the kernel messages will print features numerically instead.

In addition to saving 1408 bytes out of vmlinux, this also saves 1373
bytes out of the uncompressed setup code, which contributes directly to
the size of bzImage.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
 arch/x86/Kconfig                  | 12 +++++++
 arch/x86/boot/Makefile            |  5 ++-
 arch/x86/boot/cpu.c               | 68 +++++++++++++++++++++++----------------
 arch/x86/include/asm/cpufeature.h | 13 ++++++++
 arch/x86/kernel/cpu/Makefile      |  5 ++-
 arch/x86/kernel/cpu/common.c      |  4 +--
 6 files changed, 76 insertions(+), 31 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..4dfdbdc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
 	select HAVE_CC_STACKPROTECTOR
+	select X86_FEATURE_NAMES if PROC_FS
 
 config INSTRUCTION_DECODER
 	def_bool y
@@ -304,6 +305,17 @@ config SMP
 
 	  If you don't know what to do here, say N.
 
+config X86_FEATURE_NAMES
+	bool "Processor feature human-readable names"
+	default y
+	---help---
+	  This option compiles in a table of x86 feature bits and corresponding
+	  names.  This is required to support /proc/cpuinfo and a few kernel
+	  messages.  You can disable this to save space, at the expense of
+	  making those few kernel messages show numeric feature bits instead.
+
+	  If in doubt, say Y.
+
 config X86_X2APIC
 	bool "Support x2apic"
 	depends on X86_LOCAL_APIC && X86_64 && IRQ_REMAP
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 878df7e..ca6f7ed 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -35,7 +35,7 @@ setup-y		+= video-vesa.o
 setup-y		+= video-bios.o
 
 targets		+= $(setup-y)
-hostprogs-y	:= mkcpustr tools/build
+hostprogs-y	:= tools/build
 
 HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
 		    -include include/generated/autoconf.h \
@@ -43,11 +43,14 @@ HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
 
 $(obj)/cpu.o: $(obj)/cpustr.h
 
+ifdef CONFIG_X86_FEATURE_NAMES
+hostprogs-y += mkcpustr
 quiet_cmd_cpustr = CPUSTR  $@
       cmd_cpustr = $(obj)/mkcpustr > $@
 targets		+= cpustr.h
 $(obj)/cpustr.h: $(obj)/mkcpustr FORCE
 	$(call if_changed,cpustr)
+endif
 
 # ---------------------------------------------------------------------------
 
diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
index 6ec6bb6..29207f6 100644
--- a/arch/x86/boot/cpu.c
+++ b/arch/x86/boot/cpu.c
@@ -16,7 +16,9 @@
  */
 
 #include "boot.h"
+#ifdef CONFIG_X86_FEATURE_NAMES
 #include "cpustr.h"
+#endif
 
 static char *cpu_name(int level)
 {
@@ -32,11 +34,48 @@ static char *cpu_name(int level)
 	}
 }
 
+static void show_cap_strs(u32 *err_flags)
+{
+	int i, j;
+#ifdef CONFIG_X86_FEATURE_NAMES
+	const unsigned char *msg_strs = (const unsigned char *)x86_cap_strs;
+	for (i = 0; i < NCAPINTS; i++) {
+		u32 e = err_flags[i];
+		for (j = 0; j < 32; j++) {
+			if (msg_strs[0] < i ||
+			    (msg_strs[0] == i && msg_strs[1] < j)) {
+				/* Skip to the next string */
+				msg_strs += 2;
+				while (*msg_strs++)
+					;
+			}
+			if (e & 1) {
+				if (msg_strs[0] == i &&
+				    msg_strs[1] == j &&
+				    msg_strs[2])
+					printf("%s ", msg_strs+2);
+				else
+					printf("%d:%d ", i, j);
+			}
+			e >>= 1;
+		}
+	}
+#else
+	for (i = 0; i < NCAPINTS; i++) {
+		u32 e = err_flags[i];
+		for (j = 0; j < 32; j++) {
+			if (e & 1)
+				printf("%d:%d ", i, j);
+			e >>= 1;
+		}
+	}
+#endif
+}
+
 int validate_cpu(void)
 {
 	u32 *err_flags;
 	int cpu_level, req_level;
-	const unsigned char *msg_strs;
 
 	check_cpu(&cpu_level, &req_level, &err_flags);
 
@@ -49,34 +88,9 @@ int validate_cpu(void)
 	}
 
 	if (err_flags) {
-		int i, j;
 		puts("This kernel requires the following features "
 		     "not present on the CPU:\n");
-
-		msg_strs = (const unsigned char *)x86_cap_strs;
-
-		for (i = 0; i < NCAPINTS; i++) {
-			u32 e = err_flags[i];
-
-			for (j = 0; j < 32; j++) {
-				if (msg_strs[0] < i ||
-				    (msg_strs[0] == i && msg_strs[1] < j)) {
-					/* Skip to the next string */
-					msg_strs += 2;
-					while (*msg_strs++)
-						;
-				}
-				if (e & 1) {
-					if (msg_strs[0] == i &&
-					    msg_strs[1] == j &&
-					    msg_strs[2])
-						printf("%s ", msg_strs+2);
-					else
-						printf("%d:%d ", i, j);
-				}
-				e >>= 1;
-			}
-		}
+		show_cap_strs(err_flags);
 		putchar('\n');
 		return -1;
 	} else {
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e099f95..af6d7d5 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -237,8 +237,21 @@
 #include <asm/asm.h>
 #include <linux/bitops.h>
 
+#ifdef CONFIG_X86_FEATURE_NAMES
 extern const char * const x86_cap_flags[NCAPINTS*32];
 extern const char * const x86_power_flags[32];
+#define X86_CAP_FMT "%s"
+static inline const char *x86_cap_flag(u32 flag)
+{
+	return x86_cap_flags[flag];
+}
+#else
+#define X86_CAP_FMT "%x"
+static inline u32 x86_cap_flag(u32 flag)
+{
+	return flag;
+}
+#endif
 
 #define test_cpu_cap(c, bit)						\
 	 test_bit(bit, (unsigned long *)((c)->x86_capability))
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 64038d8..77dcab2 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -13,11 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o scattered.o topology.o
-obj-y			+= capflags.o powerflags.o common.o
+obj-y			+= common.o
 obj-y			+= rdrand.o
 obj-y			+= match.o
 
 obj-$(CONFIG_PROC_FS)	+= proc.o
+obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
 
 obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
@@ -50,6 +51,7 @@ obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o perf_event_amd_ibs.o
 
 obj-$(CONFIG_HYPERVISOR_GUEST)		+= vmware.o hypervisor.o mshyperv.o
 
+ifdef CONFIG_X86_FEATURE_NAMES
 quiet_cmd_mkcapflags = MKCAP   $@
       cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $< $@
 
@@ -58,3 +60,4 @@ cpufeature = $(src)/../../include/asm/cpufeature.h
 targets += capflags.c
 $(obj)/capflags.c: $(cpufeature) $(src)/mkcapflags.sh FORCE
 	$(call if_changed,mkcapflags)
+endif
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8e28bf2..3991a81 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -336,8 +336,8 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 			continue;
 
 		printk(KERN_WARNING
-		       "CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
-				x86_cap_flags[df->feature], df->level);
+		       "CPU: CPU feature " X86_CAP_FMT " disabled, no CPUID level 0x%x\n",
+				x86_cap_flag(df->feature), df->level);
 	}
 }
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:09 ` [PATCH 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
@ 2014-02-22 19:16   ` H. Peter Anvin
  2014-02-22 19:37     ` Josh Triplett
  2014-02-22 19:19   ` H. Peter Anvin
  1 sibling, 1 reply; 19+ messages in thread
From: H. Peter Anvin @ 2014-02-22 19:16 UTC (permalink / raw)
  To: Josh Triplett, Andi Kleen, Andrew Morton, Borislav Petkov,
	Feng Tang, Ingo Molnar, Jacob Shin, Jan Beulich,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra, Qiaowei Ren,
	Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, Thomas Renninger,
	linux-kernel, x86

Is the goal here to shrink bzImage or the runtime kernel?

Also, there are eay too many ifdefs in this patch.  You don't have to conditionalize the rules to build things, just don't include them if not needed.

On February 22, 2014 11:09:30 AM PST, Josh Triplett <josh@joshtriplett.org> wrote:
>The table mapping CPUID bits to human-readable strings takes up a
>non-trivial amount of space, and only exists to support /proc/cpuinfo
>and a couple of kernel messages.  Since programs depend on the format
>of
>/proc/cpuinfo, force inclusion of the table when building with /proc
>support; otherwise, support omitting that table to save space, in which
>case the kernel messages will print features numerically instead.
>
>In addition to saving 1408 bytes out of vmlinux, this also saves 1373
>bytes out of the uncompressed setup code, which contributes directly to
>the size of bzImage.
>
>Signed-off-by: Josh Triplett <josh@joshtriplett.org>
>---
> arch/x86/Kconfig                  | 12 +++++++
> arch/x86/boot/Makefile            |  5 ++-
>arch/x86/boot/cpu.c               | 68
>+++++++++++++++++++++++----------------
> arch/x86/include/asm/cpufeature.h | 13 ++++++++
> arch/x86/kernel/cpu/Makefile      |  5 ++-
> arch/x86/kernel/cpu/common.c      |  4 +--
> 6 files changed, 76 insertions(+), 31 deletions(-)
>
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index 0af5250..4dfdbdc 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -127,6 +127,7 @@ config X86
> 	select HAVE_DEBUG_STACKOVERFLOW
> 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
> 	select HAVE_CC_STACKPROTECTOR
>+	select X86_FEATURE_NAMES if PROC_FS
> 
> config INSTRUCTION_DECODER
> 	def_bool y
>@@ -304,6 +305,17 @@ config SMP
> 
> 	  If you don't know what to do here, say N.
> 
>+config X86_FEATURE_NAMES
>+	bool "Processor feature human-readable names"
>+	default y
>+	---help---
>+	  This option compiles in a table of x86 feature bits and
>corresponding
>+	  names.  This is required to support /proc/cpuinfo and a few kernel
>+	  messages.  You can disable this to save space, at the expense of
>+	  making those few kernel messages show numeric feature bits instead.
>+
>+	  If in doubt, say Y.
>+
> config X86_X2APIC
> 	bool "Support x2apic"
> 	depends on X86_LOCAL_APIC && X86_64 && IRQ_REMAP
>diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
>index 878df7e..ca6f7ed 100644
>--- a/arch/x86/boot/Makefile
>+++ b/arch/x86/boot/Makefile
>@@ -35,7 +35,7 @@ setup-y		+= video-vesa.o
> setup-y		+= video-bios.o
> 
> targets		+= $(setup-y)
>-hostprogs-y	:= mkcpustr tools/build
>+hostprogs-y	:= tools/build
> 
> HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
> 		    -include include/generated/autoconf.h \
>@@ -43,11 +43,14 @@ HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
> 
> $(obj)/cpu.o: $(obj)/cpustr.h
> 
>+ifdef CONFIG_X86_FEATURE_NAMES
>+hostprogs-y += mkcpustr
> quiet_cmd_cpustr = CPUSTR  $@
>       cmd_cpustr = $(obj)/mkcpustr > $@
> targets		+= cpustr.h
> $(obj)/cpustr.h: $(obj)/mkcpustr FORCE
> 	$(call if_changed,cpustr)
>+endif
> 
>#
>---------------------------------------------------------------------------
> 
>diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
>index 6ec6bb6..29207f6 100644
>--- a/arch/x86/boot/cpu.c
>+++ b/arch/x86/boot/cpu.c
>@@ -16,7 +16,9 @@
>  */
> 
> #include "boot.h"
>+#ifdef CONFIG_X86_FEATURE_NAMES
> #include "cpustr.h"
>+#endif
> 
> static char *cpu_name(int level)
> {
>@@ -32,11 +34,48 @@ static char *cpu_name(int level)
> 	}
> }
> 
>+static void show_cap_strs(u32 *err_flags)
>+{
>+	int i, j;
>+#ifdef CONFIG_X86_FEATURE_NAMES
>+	const unsigned char *msg_strs = (const unsigned char *)x86_cap_strs;
>+	for (i = 0; i < NCAPINTS; i++) {
>+		u32 e = err_flags[i];
>+		for (j = 0; j < 32; j++) {
>+			if (msg_strs[0] < i ||
>+			    (msg_strs[0] == i && msg_strs[1] < j)) {
>+				/* Skip to the next string */
>+				msg_strs += 2;
>+				while (*msg_strs++)
>+					;
>+			}
>+			if (e & 1) {
>+				if (msg_strs[0] == i &&
>+				    msg_strs[1] == j &&
>+				    msg_strs[2])
>+					printf("%s ", msg_strs+2);
>+				else
>+					printf("%d:%d ", i, j);
>+			}
>+			e >>= 1;
>+		}
>+	}
>+#else
>+	for (i = 0; i < NCAPINTS; i++) {
>+		u32 e = err_flags[i];
>+		for (j = 0; j < 32; j++) {
>+			if (e & 1)
>+				printf("%d:%d ", i, j);
>+			e >>= 1;
>+		}
>+	}
>+#endif
>+}
>+
> int validate_cpu(void)
> {
> 	u32 *err_flags;
> 	int cpu_level, req_level;
>-	const unsigned char *msg_strs;
> 
> 	check_cpu(&cpu_level, &req_level, &err_flags);
> 
>@@ -49,34 +88,9 @@ int validate_cpu(void)
> 	}
> 
> 	if (err_flags) {
>-		int i, j;
> 		puts("This kernel requires the following features "
> 		     "not present on the CPU:\n");
>-
>-		msg_strs = (const unsigned char *)x86_cap_strs;
>-
>-		for (i = 0; i < NCAPINTS; i++) {
>-			u32 e = err_flags[i];
>-
>-			for (j = 0; j < 32; j++) {
>-				if (msg_strs[0] < i ||
>-				    (msg_strs[0] == i && msg_strs[1] < j)) {
>-					/* Skip to the next string */
>-					msg_strs += 2;
>-					while (*msg_strs++)
>-						;
>-				}
>-				if (e & 1) {
>-					if (msg_strs[0] == i &&
>-					    msg_strs[1] == j &&
>-					    msg_strs[2])
>-						printf("%s ", msg_strs+2);
>-					else
>-						printf("%d:%d ", i, j);
>-				}
>-				e >>= 1;
>-			}
>-		}
>+		show_cap_strs(err_flags);
> 		putchar('\n');
> 		return -1;
> 	} else {
>diff --git a/arch/x86/include/asm/cpufeature.h
>b/arch/x86/include/asm/cpufeature.h
>index e099f95..af6d7d5 100644
>--- a/arch/x86/include/asm/cpufeature.h
>+++ b/arch/x86/include/asm/cpufeature.h
>@@ -237,8 +237,21 @@
> #include <asm/asm.h>
> #include <linux/bitops.h>
> 
>+#ifdef CONFIG_X86_FEATURE_NAMES
> extern const char * const x86_cap_flags[NCAPINTS*32];
> extern const char * const x86_power_flags[32];
>+#define X86_CAP_FMT "%s"
>+static inline const char *x86_cap_flag(u32 flag)
>+{
>+	return x86_cap_flags[flag];
>+}
>+#else
>+#define X86_CAP_FMT "%x"
>+static inline u32 x86_cap_flag(u32 flag)
>+{
>+	return flag;
>+}
>+#endif
> 
> #define test_cpu_cap(c, bit)						\
> 	 test_bit(bit, (unsigned long *)((c)->x86_capability))
>diff --git a/arch/x86/kernel/cpu/Makefile
>b/arch/x86/kernel/cpu/Makefile
>index 64038d8..77dcab2 100644
>--- a/arch/x86/kernel/cpu/Makefile
>+++ b/arch/x86/kernel/cpu/Makefile
>@@ -13,11 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
> CFLAGS_common.o		:= $(nostackp)
> 
> obj-y			:= intel_cacheinfo.o scattered.o topology.o
>-obj-y			+= capflags.o powerflags.o common.o
>+obj-y			+= common.o
> obj-y			+= rdrand.o
> obj-y			+= match.o
> 
> obj-$(CONFIG_PROC_FS)	+= proc.o
>+obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
> 
> obj-$(CONFIG_X86_32)	+= bugs.o
> obj-$(CONFIG_X86_64)	+= bugs_64.o
>@@ -50,6 +51,7 @@ obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o
>perf_event_amd_ibs.o
> 
> obj-$(CONFIG_HYPERVISOR_GUEST)		+= vmware.o hypervisor.o mshyperv.o
> 
>+ifdef CONFIG_X86_FEATURE_NAMES
> quiet_cmd_mkcapflags = MKCAP   $@
> cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $< $@
> 
>@@ -58,3 +60,4 @@ cpufeature = $(src)/../../include/asm/cpufeature.h
> targets += capflags.c
> $(obj)/capflags.c: $(cpufeature) $(src)/mkcapflags.sh FORCE
> 	$(call if_changed,mkcapflags)
>+endif
>diff --git a/arch/x86/kernel/cpu/common.c
>b/arch/x86/kernel/cpu/common.c
>index 8e28bf2..3991a81 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@ -336,8 +336,8 @@ static void filter_cpuid_features(struct
>cpuinfo_x86 *c, bool warn)
> 			continue;
> 
> 		printk(KERN_WARNING
>-		       "CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
>-				x86_cap_flags[df->feature], df->level);
>+		       "CPU: CPU feature " X86_CAP_FMT " disabled, no CPUID level
>0x%x\n",
>+				x86_cap_flag(df->feature), df->level);
> 	}
> }
> 

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:09 ` [PATCH 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
  2014-02-22 19:16   ` H. Peter Anvin
@ 2014-02-22 19:19   ` H. Peter Anvin
  2014-02-22 19:43     ` Josh Triplett
  1 sibling, 1 reply; 19+ messages in thread
From: H. Peter Anvin @ 2014-02-22 19:19 UTC (permalink / raw)
  To: Josh Triplett, Andi Kleen, Andrew Morton, Borislav Petkov,
	Feng Tang, Ingo Molnar, Jacob Shin, Jan Beulich,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra, Qiaowei Ren,
	Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, Thomas Renninger,
	linux-kernel, x86

Also, for the numeric message at least please show it in word:bit format.

On February 22, 2014 11:09:30 AM PST, Josh Triplett <josh@joshtriplett.org> wrote:
>The table mapping CPUID bits to human-readable strings takes up a
>non-trivial amount of space, and only exists to support /proc/cpuinfo
>and a couple of kernel messages.  Since programs depend on the format
>of
>/proc/cpuinfo, force inclusion of the table when building with /proc
>support; otherwise, support omitting that table to save space, in which
>case the kernel messages will print features numerically instead.
>
>In addition to saving 1408 bytes out of vmlinux, this also saves 1373
>bytes out of the uncompressed setup code, which contributes directly to
>the size of bzImage.
>
>Signed-off-by: Josh Triplett <josh@joshtriplett.org>
>---
> arch/x86/Kconfig                  | 12 +++++++
> arch/x86/boot/Makefile            |  5 ++-
>arch/x86/boot/cpu.c               | 68
>+++++++++++++++++++++++----------------
> arch/x86/include/asm/cpufeature.h | 13 ++++++++
> arch/x86/kernel/cpu/Makefile      |  5 ++-
> arch/x86/kernel/cpu/common.c      |  4 +--
> 6 files changed, 76 insertions(+), 31 deletions(-)
>
>diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>index 0af5250..4dfdbdc 100644
>--- a/arch/x86/Kconfig
>+++ b/arch/x86/Kconfig
>@@ -127,6 +127,7 @@ config X86
> 	select HAVE_DEBUG_STACKOVERFLOW
> 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
> 	select HAVE_CC_STACKPROTECTOR
>+	select X86_FEATURE_NAMES if PROC_FS
> 
> config INSTRUCTION_DECODER
> 	def_bool y
>@@ -304,6 +305,17 @@ config SMP
> 
> 	  If you don't know what to do here, say N.
> 
>+config X86_FEATURE_NAMES
>+	bool "Processor feature human-readable names"
>+	default y
>+	---help---
>+	  This option compiles in a table of x86 feature bits and
>corresponding
>+	  names.  This is required to support /proc/cpuinfo and a few kernel
>+	  messages.  You can disable this to save space, at the expense of
>+	  making those few kernel messages show numeric feature bits instead.
>+
>+	  If in doubt, say Y.
>+
> config X86_X2APIC
> 	bool "Support x2apic"
> 	depends on X86_LOCAL_APIC && X86_64 && IRQ_REMAP
>diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
>index 878df7e..ca6f7ed 100644
>--- a/arch/x86/boot/Makefile
>+++ b/arch/x86/boot/Makefile
>@@ -35,7 +35,7 @@ setup-y		+= video-vesa.o
> setup-y		+= video-bios.o
> 
> targets		+= $(setup-y)
>-hostprogs-y	:= mkcpustr tools/build
>+hostprogs-y	:= tools/build
> 
> HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
> 		    -include include/generated/autoconf.h \
>@@ -43,11 +43,14 @@ HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
> 
> $(obj)/cpu.o: $(obj)/cpustr.h
> 
>+ifdef CONFIG_X86_FEATURE_NAMES
>+hostprogs-y += mkcpustr
> quiet_cmd_cpustr = CPUSTR  $@
>       cmd_cpustr = $(obj)/mkcpustr > $@
> targets		+= cpustr.h
> $(obj)/cpustr.h: $(obj)/mkcpustr FORCE
> 	$(call if_changed,cpustr)
>+endif
> 
>#
>---------------------------------------------------------------------------
> 
>diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
>index 6ec6bb6..29207f6 100644
>--- a/arch/x86/boot/cpu.c
>+++ b/arch/x86/boot/cpu.c
>@@ -16,7 +16,9 @@
>  */
> 
> #include "boot.h"
>+#ifdef CONFIG_X86_FEATURE_NAMES
> #include "cpustr.h"
>+#endif
> 
> static char *cpu_name(int level)
> {
>@@ -32,11 +34,48 @@ static char *cpu_name(int level)
> 	}
> }
> 
>+static void show_cap_strs(u32 *err_flags)
>+{
>+	int i, j;
>+#ifdef CONFIG_X86_FEATURE_NAMES
>+	const unsigned char *msg_strs = (const unsigned char *)x86_cap_strs;
>+	for (i = 0; i < NCAPINTS; i++) {
>+		u32 e = err_flags[i];
>+		for (j = 0; j < 32; j++) {
>+			if (msg_strs[0] < i ||
>+			    (msg_strs[0] == i && msg_strs[1] < j)) {
>+				/* Skip to the next string */
>+				msg_strs += 2;
>+				while (*msg_strs++)
>+					;
>+			}
>+			if (e & 1) {
>+				if (msg_strs[0] == i &&
>+				    msg_strs[1] == j &&
>+				    msg_strs[2])
>+					printf("%s ", msg_strs+2);
>+				else
>+					printf("%d:%d ", i, j);
>+			}
>+			e >>= 1;
>+		}
>+	}
>+#else
>+	for (i = 0; i < NCAPINTS; i++) {
>+		u32 e = err_flags[i];
>+		for (j = 0; j < 32; j++) {
>+			if (e & 1)
>+				printf("%d:%d ", i, j);
>+			e >>= 1;
>+		}
>+	}
>+#endif
>+}
>+
> int validate_cpu(void)
> {
> 	u32 *err_flags;
> 	int cpu_level, req_level;
>-	const unsigned char *msg_strs;
> 
> 	check_cpu(&cpu_level, &req_level, &err_flags);
> 
>@@ -49,34 +88,9 @@ int validate_cpu(void)
> 	}
> 
> 	if (err_flags) {
>-		int i, j;
> 		puts("This kernel requires the following features "
> 		     "not present on the CPU:\n");
>-
>-		msg_strs = (const unsigned char *)x86_cap_strs;
>-
>-		for (i = 0; i < NCAPINTS; i++) {
>-			u32 e = err_flags[i];
>-
>-			for (j = 0; j < 32; j++) {
>-				if (msg_strs[0] < i ||
>-				    (msg_strs[0] == i && msg_strs[1] < j)) {
>-					/* Skip to the next string */
>-					msg_strs += 2;
>-					while (*msg_strs++)
>-						;
>-				}
>-				if (e & 1) {
>-					if (msg_strs[0] == i &&
>-					    msg_strs[1] == j &&
>-					    msg_strs[2])
>-						printf("%s ", msg_strs+2);
>-					else
>-						printf("%d:%d ", i, j);
>-				}
>-				e >>= 1;
>-			}
>-		}
>+		show_cap_strs(err_flags);
> 		putchar('\n');
> 		return -1;
> 	} else {
>diff --git a/arch/x86/include/asm/cpufeature.h
>b/arch/x86/include/asm/cpufeature.h
>index e099f95..af6d7d5 100644
>--- a/arch/x86/include/asm/cpufeature.h
>+++ b/arch/x86/include/asm/cpufeature.h
>@@ -237,8 +237,21 @@
> #include <asm/asm.h>
> #include <linux/bitops.h>
> 
>+#ifdef CONFIG_X86_FEATURE_NAMES
> extern const char * const x86_cap_flags[NCAPINTS*32];
> extern const char * const x86_power_flags[32];
>+#define X86_CAP_FMT "%s"
>+static inline const char *x86_cap_flag(u32 flag)
>+{
>+	return x86_cap_flags[flag];
>+}
>+#else
>+#define X86_CAP_FMT "%x"
>+static inline u32 x86_cap_flag(u32 flag)
>+{
>+	return flag;
>+}
>+#endif
> 
> #define test_cpu_cap(c, bit)						\
> 	 test_bit(bit, (unsigned long *)((c)->x86_capability))
>diff --git a/arch/x86/kernel/cpu/Makefile
>b/arch/x86/kernel/cpu/Makefile
>index 64038d8..77dcab2 100644
>--- a/arch/x86/kernel/cpu/Makefile
>+++ b/arch/x86/kernel/cpu/Makefile
>@@ -13,11 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
> CFLAGS_common.o		:= $(nostackp)
> 
> obj-y			:= intel_cacheinfo.o scattered.o topology.o
>-obj-y			+= capflags.o powerflags.o common.o
>+obj-y			+= common.o
> obj-y			+= rdrand.o
> obj-y			+= match.o
> 
> obj-$(CONFIG_PROC_FS)	+= proc.o
>+obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
> 
> obj-$(CONFIG_X86_32)	+= bugs.o
> obj-$(CONFIG_X86_64)	+= bugs_64.o
>@@ -50,6 +51,7 @@ obj-$(CONFIG_X86_LOCAL_APIC)		+= perfctr-watchdog.o
>perf_event_amd_ibs.o
> 
> obj-$(CONFIG_HYPERVISOR_GUEST)		+= vmware.o hypervisor.o mshyperv.o
> 
>+ifdef CONFIG_X86_FEATURE_NAMES
> quiet_cmd_mkcapflags = MKCAP   $@
> cmd_mkcapflags = $(CONFIG_SHELL) $(srctree)/$(src)/mkcapflags.sh $< $@
> 
>@@ -58,3 +60,4 @@ cpufeature = $(src)/../../include/asm/cpufeature.h
> targets += capflags.c
> $(obj)/capflags.c: $(cpufeature) $(src)/mkcapflags.sh FORCE
> 	$(call if_changed,mkcapflags)
>+endif
>diff --git a/arch/x86/kernel/cpu/common.c
>b/arch/x86/kernel/cpu/common.c
>index 8e28bf2..3991a81 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@ -336,8 +336,8 @@ static void filter_cpuid_features(struct
>cpuinfo_x86 *c, bool warn)
> 			continue;
> 
> 		printk(KERN_WARNING
>-		       "CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
>-				x86_cap_flags[df->feature], df->level);
>+		       "CPU: CPU feature " X86_CAP_FMT " disabled, no CPUID level
>0x%x\n",
>+				x86_cap_flag(df->feature), df->level);
> 	}
> }
> 

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:16   ` H. Peter Anvin
@ 2014-02-22 19:37     ` Josh Triplett
  0 siblings, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 19:37 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Andi Kleen, Andrew Morton, Borislav Petkov, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Kirill A. Shutemov,
	Paul Gortmaker, Peter Zijlstra, Qiaowei Ren, Rob Landley,
	Seiji Aguchi, Stephane Eranian, Suravee Suthikulpanit,
	Thomas Gleixner, Thomas Renninger, linux-kernel, x86

On Sat, Feb 22, 2014 at 11:16:57AM -0800, H. Peter Anvin wrote:
> Is the goal here to shrink bzImage or the runtime kernel?

Both, though I primarily care about the former.

> Also, there are eay too many ifdefs in this patch.  You don't have to
> conditionalize the rules to build things, just don't include them if
> not needed.

Fixed in v2.

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:19   ` H. Peter Anvin
@ 2014-02-22 19:43     ` Josh Triplett
  0 siblings, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 19:43 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Andi Kleen, Andrew Morton, Borislav Petkov, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Kirill A. Shutemov,
	Paul Gortmaker, Peter Zijlstra, Qiaowei Ren, Rob Landley,
	Seiji Aguchi, Stephane Eranian, Suravee Suthikulpanit,
	Thomas Gleixner, Thomas Renninger, linux-kernel, x86

On Sat, Feb 22, 2014 at 11:19:18AM -0800, H. Peter Anvin wrote:
> Also, for the numeric message at least please show it in word:bit format.

I can do that, but the only obvious way I see to do so (without adding
ifdefs to arch/x86/kernel/cpu/common.c) would require making
x86_cap_flag a macro generating two printk arguments.  Doing so in v2;
hope that isn't too ugly...

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v2 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS
  2014-02-22 19:08 [PATCH 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
  2014-02-22 19:09 ` [PATCH 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
@ 2014-02-22 19:55 ` Josh Triplett
  2014-02-22 21:05   ` [PATCH v3 " Josh Triplett
  2014-02-22 21:06   ` [PATCH v3 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
  2014-02-22 19:57 ` [PATCH v2 " Josh Triplett
  2 siblings, 2 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 19:55 UTC (permalink / raw)
  To: Andrew Morton, Andi Kleen, Borislav Petkov, Feng Tang,
	H. Peter Anvin, Ingo Molnar, Jacob Shin, Jan Beulich,
	Jussi Kivilinna, Kirill A. Shutemov, Paul Gortmaker,
	Peter Zijlstra, Rafael J. Wysocki, Rob Landley, Seiji Aguchi,
	Stephane Eranian, Suravee Suthikulpanit, Thomas Gleixner,
	linux-kernel, x86

arch/x86/kernel/cpu/proc.c only exists to support files in /proc; omit that
file when compiling without CONFIG_PROC_FS.

Saves 645 additional bytes on 32-bit x86 when !CONFIG_PROC_FS:

add/remove: 0/5 grow/shrink: 0/0 up/down: 0/-645 (-645)
function                                     old     new   delta
c_stop                                         1       -      -1
c_next                                        11       -     -11
cpuinfo_op                                    16       -     -16
c_start                                       24       -     -24
show_cpuinfo                                 593       -    -593

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v2: no changes to patch 1.

 arch/x86/kernel/cpu/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 7fd54f0..64038d8 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -13,10 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o scattered.o topology.o
-obj-y			+= proc.o capflags.o powerflags.o common.o
+obj-y			+= capflags.o powerflags.o common.o
 obj-y			+= rdrand.o
 obj-y			+= match.o
 
+obj-$(CONFIG_PROC_FS)	+= proc.o
+
 obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:08 [PATCH 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
  2014-02-22 19:09 ` [PATCH 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
  2014-02-22 19:55 ` [PATCH v2 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
@ 2014-02-22 19:57 ` Josh Triplett
  2014-02-22 20:49   ` Borislav Petkov
  2 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 19:57 UTC (permalink / raw)
  To: Andrew Morton, Andi Kleen, Borislav Petkov, Feng Tang,
	H. Peter Anvin, Ingo Molnar, Jacob Shin, Jan Beulich,
	Jussi Kivilinna, Kirill A. Shutemov, Paul Gortmaker,
	Peter Zijlstra, Rafael J. Wysocki, Rob Landley, Seiji Aguchi,
	Stephane Eranian, Suravee Suthikulpanit, Thomas Gleixner,
	linux-kernel, x86

The table mapping CPUID bits to human-readable strings takes up a
non-trivial amount of space, and only exists to support /proc/cpuinfo
and a couple of kernel messages.  Since programs depend on the format of
/proc/cpuinfo, force inclusion of the table when building with /proc
support; otherwise, support omitting that table to save space, in which
case the kernel messages will print features numerically instead.

In addition to saving 1408 bytes out of vmlinux, this also saves 1373
bytes out of the uncompressed setup code, which contributes directly to
the size of bzImage.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v2: Incorporate feedback from H. Peter Anvin:
    - Remove unnecessary ifdefs.
    - Print numeric feature flags in word:bit form.

 arch/x86/Kconfig                  | 12 +++++++
 arch/x86/boot/cpu.c               | 68 +++++++++++++++++++++++----------------
 arch/x86/include/asm/cpufeature.h |  7 ++++
 arch/x86/kernel/cpu/Makefile      |  3 +-
 arch/x86/kernel/cpu/common.c      |  4 +--
 5 files changed, 64 insertions(+), 30 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..4dfdbdc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
 	select HAVE_CC_STACKPROTECTOR
+	select X86_FEATURE_NAMES if PROC_FS
 
 config INSTRUCTION_DECODER
 	def_bool y
@@ -304,6 +305,17 @@ config SMP
 
 	  If you don't know what to do here, say N.
 
+config X86_FEATURE_NAMES
+	bool "Processor feature human-readable names"
+	default y
+	---help---
+	  This option compiles in a table of x86 feature bits and corresponding
+	  names.  This is required to support /proc/cpuinfo and a few kernel
+	  messages.  You can disable this to save space, at the expense of
+	  making those few kernel messages show numeric feature bits instead.
+
+	  If in doubt, say Y.
+
 config X86_X2APIC
 	bool "Support x2apic"
 	depends on X86_LOCAL_APIC && X86_64 && IRQ_REMAP
diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
index 6ec6bb6..29207f6 100644
--- a/arch/x86/boot/cpu.c
+++ b/arch/x86/boot/cpu.c
@@ -16,7 +16,9 @@
  */
 
 #include "boot.h"
+#ifdef CONFIG_X86_FEATURE_NAMES
 #include "cpustr.h"
+#endif
 
 static char *cpu_name(int level)
 {
@@ -32,11 +34,48 @@ static char *cpu_name(int level)
 	}
 }
 
+static void show_cap_strs(u32 *err_flags)
+{
+	int i, j;
+#ifdef CONFIG_X86_FEATURE_NAMES
+	const unsigned char *msg_strs = (const unsigned char *)x86_cap_strs;
+	for (i = 0; i < NCAPINTS; i++) {
+		u32 e = err_flags[i];
+		for (j = 0; j < 32; j++) {
+			if (msg_strs[0] < i ||
+			    (msg_strs[0] == i && msg_strs[1] < j)) {
+				/* Skip to the next string */
+				msg_strs += 2;
+				while (*msg_strs++)
+					;
+			}
+			if (e & 1) {
+				if (msg_strs[0] == i &&
+				    msg_strs[1] == j &&
+				    msg_strs[2])
+					printf("%s ", msg_strs+2);
+				else
+					printf("%d:%d ", i, j);
+			}
+			e >>= 1;
+		}
+	}
+#else
+	for (i = 0; i < NCAPINTS; i++) {
+		u32 e = err_flags[i];
+		for (j = 0; j < 32; j++) {
+			if (e & 1)
+				printf("%d:%d ", i, j);
+			e >>= 1;
+		}
+	}
+#endif
+}
+
 int validate_cpu(void)
 {
 	u32 *err_flags;
 	int cpu_level, req_level;
-	const unsigned char *msg_strs;
 
 	check_cpu(&cpu_level, &req_level, &err_flags);
 
@@ -49,34 +88,9 @@ int validate_cpu(void)
 	}
 
 	if (err_flags) {
-		int i, j;
 		puts("This kernel requires the following features "
 		     "not present on the CPU:\n");
-
-		msg_strs = (const unsigned char *)x86_cap_strs;
-
-		for (i = 0; i < NCAPINTS; i++) {
-			u32 e = err_flags[i];
-
-			for (j = 0; j < 32; j++) {
-				if (msg_strs[0] < i ||
-				    (msg_strs[0] == i && msg_strs[1] < j)) {
-					/* Skip to the next string */
-					msg_strs += 2;
-					while (*msg_strs++)
-						;
-				}
-				if (e & 1) {
-					if (msg_strs[0] == i &&
-					    msg_strs[1] == j &&
-					    msg_strs[2])
-						printf("%s ", msg_strs+2);
-					else
-						printf("%d:%d ", i, j);
-				}
-				e >>= 1;
-			}
-		}
+		show_cap_strs(err_flags);
 		putchar('\n');
 		return -1;
 	} else {
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e099f95..75bc12b 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -237,8 +237,15 @@
 #include <asm/asm.h>
 #include <linux/bitops.h>
 
+#ifdef CONFIG_X86_FEATURE_NAMES
 extern const char * const x86_cap_flags[NCAPINTS*32];
 extern const char * const x86_power_flags[32];
+#define X86_CAP_FMT "%s"
+#define x86_cap_flag(flag) x86_cap_flags[flag]
+#else
+#define X86_CAP_FMT "%d:%d"
+#define x86_cap_flag(flag) ((flag) >> 5), ((flag) & 31)
+#endif
 
 #define test_cpu_cap(c, bit)						\
 	 test_bit(bit, (unsigned long *)((c)->x86_capability))
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 64038d8..043c97d 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -13,11 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o scattered.o topology.o
-obj-y			+= capflags.o powerflags.o common.o
+obj-y			+= common.o
 obj-y			+= rdrand.o
 obj-y			+= match.o
 
 obj-$(CONFIG_PROC_FS)	+= proc.o
+obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
 
 obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8e28bf2..3991a81 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -336,8 +336,8 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 			continue;
 
 		printk(KERN_WARNING
-		       "CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
-				x86_cap_flags[df->feature], df->level);
+		       "CPU: CPU feature " X86_CAP_FMT " disabled, no CPUID level 0x%x\n",
+				x86_cap_flag(df->feature), df->level);
 	}
 }
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:57 ` [PATCH v2 " Josh Triplett
@ 2014-02-22 20:49   ` Borislav Petkov
  2014-02-22 21:00     ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: Borislav Petkov @ 2014-02-22 20:49 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Andrew Morton, Andi Kleen, Borislav Petkov, Feng Tang,
	H. Peter Anvin, Ingo Molnar, Jacob Shin, Jan Beulich,
	Jussi Kivilinna, Kirill A. Shutemov, Paul Gortmaker,
	Peter Zijlstra, Rafael J. Wysocki, Rob Landley, Seiji Aguchi,
	Stephane Eranian, Suravee Suthikulpanit, Thomas Gleixner,
	linux-kernel, x86

On Sat, Feb 22, 2014 at 11:57:10AM -0800, Josh Triplett wrote:

...

> diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
> index 6ec6bb6..29207f6 100644
> --- a/arch/x86/boot/cpu.c
> +++ b/arch/x86/boot/cpu.c
> @@ -16,7 +16,9 @@
>   */
>  
>  #include "boot.h"
> +#ifdef CONFIG_X86_FEATURE_NAMES
>  #include "cpustr.h"
> +#endif

You probably could get rid of this ifdef too by moving it into cpustr.h
after teaching arch/x86/boot/mkcpustr.c to issue it...

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 20:49   ` Borislav Petkov
@ 2014-02-22 21:00     ` Josh Triplett
  2014-02-22 21:18       ` H. Peter Anvin
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 21:00 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Andrew Morton, Andi Kleen, Feng Tang, H. Peter Anvin,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On Sat, Feb 22, 2014 at 09:49:36PM +0100, Borislav Petkov wrote:
> On Sat, Feb 22, 2014 at 11:57:10AM -0800, Josh Triplett wrote:
> > diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
> > index 6ec6bb6..29207f6 100644
> > --- a/arch/x86/boot/cpu.c
> > +++ b/arch/x86/boot/cpu.c
> > @@ -16,7 +16,9 @@
> >   */
> >  
> >  #include "boot.h"
> > +#ifdef CONFIG_X86_FEATURE_NAMES
> >  #include "cpustr.h"
> > +#endif
> 
> You probably could get rid of this ifdef too by moving it into cpustr.h
> after teaching arch/x86/boot/mkcpustr.c to issue it...

That would require building and running mkcpustr, which doesn't happen
when !CONFIG_X86_FEATURE_NAMES.  (And it'd require adding ifdefs to
mkcpustr instead, which seems counterproductive.)

However, in exploring this, I ran into some build issues with v2 on a
clean build; I'll send out v3 shortly with fixes to those.

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v3 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS
  2014-02-22 19:55 ` [PATCH v2 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
@ 2014-02-22 21:05   ` Josh Triplett
  2014-02-22 21:06   ` [PATCH v3 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
  1 sibling, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 21:05 UTC (permalink / raw)
  To: Andrew Morton, Borislav Petkov, Feng Tang, H. Peter Anvin,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra, Qiaowei Ren,
	Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

arch/x86/kernel/cpu/proc.c only exists to support files in /proc; omit that
file when compiling without CONFIG_PROC_FS.

Saves 645 additional bytes on 32-bit x86 when !CONFIG_PROC_FS:

add/remove: 0/5 grow/shrink: 0/0 up/down: 0/-645 (-645)
function                                     old     new   delta
c_stop                                         1       -      -1
c_next                                        11       -     -11
cpuinfo_op                                    16       -     -16
c_start                                       24       -     -24
show_cpuinfo                                 593       -    -593

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v2,v3: no changes from v1 for this patch

 arch/x86/kernel/cpu/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 7fd54f0..64038d8 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -13,10 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o scattered.o topology.o
-obj-y			+= proc.o capflags.o powerflags.o common.o
+obj-y			+= capflags.o powerflags.o common.o
 obj-y			+= rdrand.o
 obj-y			+= match.o
 
+obj-$(CONFIG_PROC_FS)	+= proc.o
+
 obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v3 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 19:55 ` [PATCH v2 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
  2014-02-22 21:05   ` [PATCH v3 " Josh Triplett
@ 2014-02-22 21:06   ` Josh Triplett
  1 sibling, 0 replies; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 21:06 UTC (permalink / raw)
  To: Andrew Morton, Borislav Petkov, Feng Tang, H. Peter Anvin,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra, Qiaowei Ren,
	Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

The table mapping CPUID bits to human-readable strings takes up a
non-trivial amount of space, and only exists to support /proc/cpuinfo
and a couple of kernel messages.  Since programs depend on the format of
/proc/cpuinfo, force inclusion of the table when building with /proc
support; otherwise, support omitting that table to save space, in which
case the kernel messages will print features numerically instead.

In addition to saving 1408 bytes out of vmlinux, this also saves 1373
bytes out of the uncompressed setup code, which contributes directly to
the size of bzImage.

Signed-off-by: Josh Triplett <josh@joshtriplett.org>
---
v3: Fix build issue for clean builds, introduced in v2.

v2: Incorporate feedback from H. Peter Anvin:
    - Remove unnecessary ifdefs.
    - Print numeric feature flags in word:bit form.

 arch/x86/Kconfig                  | 12 +++++++
 arch/x86/boot/Makefile            |  7 ++--
 arch/x86/boot/cpu.c               | 68 +++++++++++++++++++++++----------------
 arch/x86/include/asm/cpufeature.h |  7 ++++
 arch/x86/kernel/cpu/Makefile      |  3 +-
 arch/x86/kernel/cpu/common.c      |  4 +--
 6 files changed, 69 insertions(+), 32 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0af5250..4dfdbdc 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -127,6 +127,7 @@ config X86
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
 	select HAVE_CC_STACKPROTECTOR
+	select X86_FEATURE_NAMES if PROC_FS
 
 config INSTRUCTION_DECODER
 	def_bool y
@@ -304,6 +305,17 @@ config SMP
 
 	  If you don't know what to do here, say N.
 
+config X86_FEATURE_NAMES
+	bool "Processor feature human-readable names"
+	default y
+	---help---
+	  This option compiles in a table of x86 feature bits and corresponding
+	  names.  This is required to support /proc/cpuinfo and a few kernel
+	  messages.  You can disable this to save space, at the expense of
+	  making those few kernel messages show numeric feature bits instead.
+
+	  If in doubt, say Y.
+
 config X86_X2APIC
 	bool "Support x2apic"
 	depends on X86_LOCAL_APIC && X86_64 && IRQ_REMAP
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 878df7e..ce72b48 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -35,17 +35,20 @@ setup-y		+= video-vesa.o
 setup-y		+= video-bios.o
 
 targets		+= $(setup-y)
-hostprogs-y	:= mkcpustr tools/build
+hostprogs-y	:= tools/build
+hostprogs-$(CONFIG_X86_FEATURE_NAMES) += mkcpustr
 
 HOST_EXTRACFLAGS += -I$(srctree)/tools/include \
 		    -include include/generated/autoconf.h \
 	            -D__EXPORTED_HEADERS__
 
+ifdef CONFIG_X86_FEATURE_NAMES
 $(obj)/cpu.o: $(obj)/cpustr.h
+endif
 
 quiet_cmd_cpustr = CPUSTR  $@
       cmd_cpustr = $(obj)/mkcpustr > $@
-targets		+= cpustr.h
+targets-$(CONFIG_X86_FEATURE_NAMES) += cpustr.h
 $(obj)/cpustr.h: $(obj)/mkcpustr FORCE
 	$(call if_changed,cpustr)
 
diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
index 6ec6bb6..29207f6 100644
--- a/arch/x86/boot/cpu.c
+++ b/arch/x86/boot/cpu.c
@@ -16,7 +16,9 @@
  */
 
 #include "boot.h"
+#ifdef CONFIG_X86_FEATURE_NAMES
 #include "cpustr.h"
+#endif
 
 static char *cpu_name(int level)
 {
@@ -32,11 +34,48 @@ static char *cpu_name(int level)
 	}
 }
 
+static void show_cap_strs(u32 *err_flags)
+{
+	int i, j;
+#ifdef CONFIG_X86_FEATURE_NAMES
+	const unsigned char *msg_strs = (const unsigned char *)x86_cap_strs;
+	for (i = 0; i < NCAPINTS; i++) {
+		u32 e = err_flags[i];
+		for (j = 0; j < 32; j++) {
+			if (msg_strs[0] < i ||
+			    (msg_strs[0] == i && msg_strs[1] < j)) {
+				/* Skip to the next string */
+				msg_strs += 2;
+				while (*msg_strs++)
+					;
+			}
+			if (e & 1) {
+				if (msg_strs[0] == i &&
+				    msg_strs[1] == j &&
+				    msg_strs[2])
+					printf("%s ", msg_strs+2);
+				else
+					printf("%d:%d ", i, j);
+			}
+			e >>= 1;
+		}
+	}
+#else
+	for (i = 0; i < NCAPINTS; i++) {
+		u32 e = err_flags[i];
+		for (j = 0; j < 32; j++) {
+			if (e & 1)
+				printf("%d:%d ", i, j);
+			e >>= 1;
+		}
+	}
+#endif
+}
+
 int validate_cpu(void)
 {
 	u32 *err_flags;
 	int cpu_level, req_level;
-	const unsigned char *msg_strs;
 
 	check_cpu(&cpu_level, &req_level, &err_flags);
 
@@ -49,34 +88,9 @@ int validate_cpu(void)
 	}
 
 	if (err_flags) {
-		int i, j;
 		puts("This kernel requires the following features "
 		     "not present on the CPU:\n");
-
-		msg_strs = (const unsigned char *)x86_cap_strs;
-
-		for (i = 0; i < NCAPINTS; i++) {
-			u32 e = err_flags[i];
-
-			for (j = 0; j < 32; j++) {
-				if (msg_strs[0] < i ||
-				    (msg_strs[0] == i && msg_strs[1] < j)) {
-					/* Skip to the next string */
-					msg_strs += 2;
-					while (*msg_strs++)
-						;
-				}
-				if (e & 1) {
-					if (msg_strs[0] == i &&
-					    msg_strs[1] == j &&
-					    msg_strs[2])
-						printf("%s ", msg_strs+2);
-					else
-						printf("%d:%d ", i, j);
-				}
-				e >>= 1;
-			}
-		}
+		show_cap_strs(err_flags);
 		putchar('\n');
 		return -1;
 	} else {
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e099f95..75bc12b 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -237,8 +237,15 @@
 #include <asm/asm.h>
 #include <linux/bitops.h>
 
+#ifdef CONFIG_X86_FEATURE_NAMES
 extern const char * const x86_cap_flags[NCAPINTS*32];
 extern const char * const x86_power_flags[32];
+#define X86_CAP_FMT "%s"
+#define x86_cap_flag(flag) x86_cap_flags[flag]
+#else
+#define X86_CAP_FMT "%d:%d"
+#define x86_cap_flag(flag) ((flag) >> 5), ((flag) & 31)
+#endif
 
 #define test_cpu_cap(c, bit)						\
 	 test_bit(bit, (unsigned long *)((c)->x86_capability))
diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile
index 64038d8..043c97d 100644
--- a/arch/x86/kernel/cpu/Makefile
+++ b/arch/x86/kernel/cpu/Makefile
@@ -13,11 +13,12 @@ nostackp := $(call cc-option, -fno-stack-protector)
 CFLAGS_common.o		:= $(nostackp)
 
 obj-y			:= intel_cacheinfo.o scattered.o topology.o
-obj-y			+= capflags.o powerflags.o common.o
+obj-y			+= common.o
 obj-y			+= rdrand.o
 obj-y			+= match.o
 
 obj-$(CONFIG_PROC_FS)	+= proc.o
+obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o
 
 obj-$(CONFIG_X86_32)	+= bugs.o
 obj-$(CONFIG_X86_64)	+= bugs_64.o
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8e28bf2..3991a81 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -336,8 +336,8 @@ static void filter_cpuid_features(struct cpuinfo_x86 *c, bool warn)
 			continue;
 
 		printk(KERN_WARNING
-		       "CPU: CPU feature %s disabled, no CPUID level 0x%x\n",
-				x86_cap_flags[df->feature], df->level);
+		       "CPU: CPU feature " X86_CAP_FMT " disabled, no CPUID level 0x%x\n",
+				x86_cap_flag(df->feature), df->level);
 	}
 }
 
-- 
1.9.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 21:00     ` Josh Triplett
@ 2014-02-22 21:18       ` H. Peter Anvin
  2014-02-22 21:36         ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: H. Peter Anvin @ 2014-02-22 21:18 UTC (permalink / raw)
  To: Josh Triplett, Borislav Petkov
  Cc: Andrew Morton, Andi Kleen, Feng Tang, Ingo Molnar, Jacob Shin,
	Jan Beulich, Jussi Kivilinna, Kirill A. Shutemov, Paul Gortmaker,
	Peter Zijlstra, Rafael J. Wysocki, Rob Landley, Seiji Aguchi,
	Stephane Eranian, Suravee Suthikulpanit, Thomas Gleixner,
	linux-kernel, x86



On February 22, 2014 1:00:39 PM PST, Josh Triplett <josh@joshtriplett.org> wrote:
>On Sat, Feb 22, 2014 at 09:49:36PM +0100, Borislav Petkov wrote:
>> On Sat, Feb 22, 2014 at 11:57:10AM -0800, Josh Triplett wrote:
>> > diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
>> > index 6ec6bb6..29207f6 100644
>> > --- a/arch/x86/boot/cpu.c
>> > +++ b/arch/x86/boot/cpu.c
>> > @@ -16,7 +16,9 @@
>> >   */
>> >  
>> >  #include "boot.h"
>> > +#ifdef CONFIG_X86_FEATURE_NAMES
>> >  #include "cpustr.h"
>> > +#endif
>> 
>> You probably could get rid of this ifdef too by moving it into
>cpustr.h
>> after teaching arch/x86/boot/mkcpustr.c to issue it...
>
>That would require building and running mkcpustr, which doesn't happen
>when !CONFIG_X86_FEATURE_NAMES.  (And it'd require adding ifdefs to
>mkcpustr instead, which seems counterproductive.)
>

Didn't that change since v1?

>However, in exploring this, I ran into some build issues with v2 on a
>clean build; I'll send out v3 shortly with fixes to those.
>
>- Josh Triplett

-- 
Sent from my mobile phone.  Please pardon brevity and lack of formatting.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 21:18       ` H. Peter Anvin
@ 2014-02-22 21:36         ` Josh Triplett
  2014-02-23 17:56           ` H. Peter Anvin
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2014-02-22 21:36 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Andrew Morton, Andi Kleen, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On Sat, Feb 22, 2014 at 01:18:14PM -0800, H. Peter Anvin wrote:
> On February 22, 2014 1:00:39 PM PST, Josh Triplett <josh@joshtriplett.org> wrote:
> >On Sat, Feb 22, 2014 at 09:49:36PM +0100, Borislav Petkov wrote:
> >> On Sat, Feb 22, 2014 at 11:57:10AM -0800, Josh Triplett wrote:
> >> > diff --git a/arch/x86/boot/cpu.c b/arch/x86/boot/cpu.c
> >> > index 6ec6bb6..29207f6 100644
> >> > --- a/arch/x86/boot/cpu.c
> >> > +++ b/arch/x86/boot/cpu.c
> >> > @@ -16,7 +16,9 @@
> >> >   */
> >> >  
> >> >  #include "boot.h"
> >> > +#ifdef CONFIG_X86_FEATURE_NAMES
> >> >  #include "cpustr.h"
> >> > +#endif
> >> 
> >> You probably could get rid of this ifdef too by moving it into
> >cpustr.h
> >> after teaching arch/x86/boot/mkcpustr.c to issue it...
> >
> >That would require building and running mkcpustr, which doesn't happen
> >when !CONFIG_X86_FEATURE_NAMES.  (And it'd require adding ifdefs to
> >mkcpustr instead, which seems counterproductive.)
> 
> Didn't that change since v1?

No, even after removing the ifdefs around the build rules as you
suggested (and v3's fixes for the resulting build issues, notably
changing some -y's to -$(CONFIG_X86_FEATURE_NAMES)), the makefiles still
manage to not build mkcpustr or cpustr.h, because nothing depends on it.

I could change the build rules to generate an empty cpustr.h and avoid
this ifdef, but that'd require an additional ifdef block in the Makefile.

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-22 21:36         ` Josh Triplett
@ 2014-02-23 17:56           ` H. Peter Anvin
  2014-02-23 21:32             ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: H. Peter Anvin @ 2014-02-23 17:56 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Borislav Petkov, Andrew Morton, Andi Kleen, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On 02/22/2014 01:36 PM, Josh Triplett wrote:
> 
> No, even after removing the ifdefs around the build rules as you
> suggested (and v3's fixes for the resulting build issues, notably
> changing some -y's to -$(CONFIG_X86_FEATURE_NAMES)), the makefiles still
> manage to not build mkcpustr or cpustr.h, because nothing depends on it.
> 

How could it miss the rule:

$(obj)/cpu.o: $(obj)/cpustr.h

> I could change the build rules to generate an empty cpustr.h and avoid
> this ifdef, but that'd require an additional ifdef block in the Makefile.

Typically the way it is done is to generate the #ifdef *inside*
cpustr.h.  However, cpustr.h is kind of special anyway so it probably
doesn't matter.

	-hpa



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-23 17:56           ` H. Peter Anvin
@ 2014-02-23 21:32             ` Josh Triplett
  2014-02-23 21:44               ` H. Peter Anvin
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2014-02-23 21:32 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Andrew Morton, Andi Kleen, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On Sun, Feb 23, 2014 at 09:56:56AM -0800, H. Peter Anvin wrote:
> On 02/22/2014 01:36 PM, Josh Triplett wrote:
> > 
> > No, even after removing the ifdefs around the build rules as you
> > suggested (and v3's fixes for the resulting build issues, notably
> > changing some -y's to -$(CONFIG_X86_FEATURE_NAMES)), the makefiles still
> > manage to not build mkcpustr or cpustr.h, because nothing depends on it.
> 
> How could it miss the rule:
> 
> $(obj)/cpu.o: $(obj)/cpustr.h

Because, in order to un-break the build, v3 wraps an ifdef around that
dependency, to prevent building cpustr.h.  Otherwise, the rule for
cpustr.h tries and fails to build mkcpustr.

> > I could change the build rules to generate an empty cpustr.h and avoid
> > this ifdef, but that'd require an additional ifdef block in the Makefile.
> 
> Typically the way it is done is to generate the #ifdef *inside*
> cpustr.h.  However, cpustr.h is kind of special anyway so it probably
> doesn't matter.

Given that only one file includes cpustr.h, and that seems unlikely to
ever change, moving the ifdef inside the header doesn't seem that
worthwhile.  If it were included more than once, it would definitely
make sense to generate a stub header.

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-23 21:32             ` Josh Triplett
@ 2014-02-23 21:44               ` H. Peter Anvin
  2014-02-23 21:55                 ` Josh Triplett
  0 siblings, 1 reply; 19+ messages in thread
From: H. Peter Anvin @ 2014-02-23 21:44 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Borislav Petkov, Andrew Morton, Andi Kleen, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On 02/23/2014 01:32 PM, Josh Triplett wrote:
> 
> Because, in order to un-break the build, v3 wraps an ifdef around that
> dependency, to prevent building cpustr.h.  Otherwise, the rule for
> cpustr.h tries and fails to build mkcpustr.
> 

Why did it fail to build mkcpustr?  It would seem that mkcpustr is or at
least ought to be completely agnostic to any of these options.

The extra build machinery here seems completely pointless.

I agree that the #ifdef isn't a big deal, but all this extra machinery
really indicates something is odd.

Oh, and of course, looking at the v2 patchset, the problem is the ifdef
around the mkcapflags shell script which really shouldn't be necessary.
 We may have to add a rule to force capflags.c to be built even if
capflags.o is not requested, but that is fine.

That will cut down on the Makefile hacks considerably, and will avoid
this problem completely.

	-hpa


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-23 21:44               ` H. Peter Anvin
@ 2014-02-23 21:55                 ` Josh Triplett
  2014-02-24  4:17                   ` H. Peter Anvin
  0 siblings, 1 reply; 19+ messages in thread
From: Josh Triplett @ 2014-02-23 21:55 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Borislav Petkov, Andrew Morton, Andi Kleen, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On Sun, Feb 23, 2014 at 01:44:20PM -0800, H. Peter Anvin wrote:
> On 02/23/2014 01:32 PM, Josh Triplett wrote:
> > 
> > Because, in order to un-break the build, v3 wraps an ifdef around that
> > dependency, to prevent building cpustr.h.  Otherwise, the rule for
> > cpustr.h tries and fails to build mkcpustr.
> > 
> 
> Why did it fail to build mkcpustr?  It would seem that mkcpustr is or at
> least ought to be completely agnostic to any of these options.
> 
> The extra build machinery here seems completely pointless.
> 
> I agree that the #ifdef isn't a big deal, but all this extra machinery
> really indicates something is odd.
> 
> Oh, and of course, looking at the v2 patchset, the problem is the ifdef
> around the mkcapflags shell script which really shouldn't be necessary.
>  We may have to add a rule to force capflags.c to be built even if
> capflags.o is not requested, but that is fine.
> 
> That will cut down on the Makefile hacks considerably, and will avoid
> this problem completely.

Why have the build system waste time building several things that won't
be used?  It seems like the Makefiles are exactly where we *should* have
the ifdef machinery, rather than in source.  I'd happily add another
ifdef in the Makefile rule that builds cpustr.h, to generate a stub
cpustr.h header, and then remove one more ifdef in the source.

- Josh Triplett

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v2 2/2] x86: Support compiling out human-friendly processor feature names
  2014-02-23 21:55                 ` Josh Triplett
@ 2014-02-24  4:17                   ` H. Peter Anvin
  0 siblings, 0 replies; 19+ messages in thread
From: H. Peter Anvin @ 2014-02-24  4:17 UTC (permalink / raw)
  To: Josh Triplett
  Cc: Borislav Petkov, Andrew Morton, Andi Kleen, Feng Tang,
	Ingo Molnar, Jacob Shin, Jan Beulich, Jussi Kivilinna,
	Kirill A. Shutemov, Paul Gortmaker, Peter Zijlstra,
	Rafael J. Wysocki, Rob Landley, Seiji Aguchi, Stephane Eranian,
	Suravee Suthikulpanit, Thomas Gleixner, linux-kernel, x86

On 02/23/2014 01:55 PM, Josh Triplett wrote:
> 
> Why have the build system waste time building several things that won't
> be used?  It seems like the Makefiles are exactly where we *should* have
> the ifdef machinery, rather than in source.  I'd happily add another
> ifdef in the Makefile rule that builds cpustr.h, to generate a stub
> cpustr.h header, and then remove one more ifdef in the source.
> 

The ifdeffery in the Makefiles really tend to be more complicated than
in the source code.  The amount of code added from v2 to v3 when it
could be handled by *removing* two lines in the Makefile makes it worth it.

If it was enough build stuff to be anywhere near remotely significant it
would be one thing.

	-hpa



^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2014-02-24  4:19 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-22 19:08 [PATCH 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
2014-02-22 19:09 ` [PATCH 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
2014-02-22 19:16   ` H. Peter Anvin
2014-02-22 19:37     ` Josh Triplett
2014-02-22 19:19   ` H. Peter Anvin
2014-02-22 19:43     ` Josh Triplett
2014-02-22 19:55 ` [PATCH v2 1/2] x86: Drop support for /proc files when !CONFIG_PROC_FS Josh Triplett
2014-02-22 21:05   ` [PATCH v3 " Josh Triplett
2014-02-22 21:06   ` [PATCH v3 2/2] x86: Support compiling out human-friendly processor feature names Josh Triplett
2014-02-22 19:57 ` [PATCH v2 " Josh Triplett
2014-02-22 20:49   ` Borislav Petkov
2014-02-22 21:00     ` Josh Triplett
2014-02-22 21:18       ` H. Peter Anvin
2014-02-22 21:36         ` Josh Triplett
2014-02-23 17:56           ` H. Peter Anvin
2014-02-23 21:32             ` Josh Triplett
2014-02-23 21:44               ` H. Peter Anvin
2014-02-23 21:55                 ` Josh Triplett
2014-02-24  4:17                   ` H. Peter Anvin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.