[Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro

[Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro

[Andrew Murray]

A common pattern found in header files is a function declaration dependent
on a CONFIG_ option being enabled, followed by an empty function for when
that option isn't enabled. This boilerplate code can often take up a lot
of space and impact code readability.

This series introduces a STUB_UNLESS macro that simplifies header files as
follows:

STUB_UNLESS(CONFIG_FOO, [body], prototype)

This evaluates to 'prototype' prepended with 'extern' if CONFIG_FOO is set
to 'y'. Otherwise it will evaluate to 'prototype' prepended with 'static
inline' followed by an empty function body. Where optional argument 'body'
is present then 'body' will be used as the function body, intended to allow
simple return statements. Using the macro results in hunks such as this:

-#ifdef CONFIG_HAVE_HW_BREAKPOINT
-extern void hw_breakpoint_thread_switch(struct task_struct *next);
-extern void ptrace_hw_copy_thread(struct task_struct *task);
-#else
-static inline void hw_breakpoint_thread_switch(struct task_struct *next)
-{
-}
-static inline void ptrace_hw_copy_thread(struct task_struct *task)
-{
-}
-#endif
+STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
+void hw_breakpoint_thread_switch(struct task_struct *next));
+
+STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
+void ptrace_hw_copy_thread(struct task_struct *task));

This may or may not be considered as more desirable than the status quo.

This series updates arm64 and perf to use the macro as an example.

Andrew Murray (3):
  kconfig.h: abstract empty functions with STUB_UNLESS macro
  cpufreq: update headers to use STUB_UNLESS macro
  arm64: update headers to use STUB_UNLESS macro

 arch/arm64/include/asm/acpi.h           | 38 +++++++++-------------
 arch/arm64/include/asm/alternative.h    |  6 +---
 arch/arm64/include/asm/cpufeature.h     |  6 +---
 arch/arm64/include/asm/cpuidle.h        | 18 +++--------
 arch/arm64/include/asm/debug-monitors.h | 10 ++----
 arch/arm64/include/asm/fpsimd.h         | 57 +++++++++++++--------------------
 arch/arm64/include/asm/hw_breakpoint.h  | 16 +++------
 arch/arm64/include/asm/kasan.h          |  9 ++----
 arch/arm64/include/asm/numa.h           | 19 ++++-------
 arch/arm64/include/asm/ptdump.h         | 13 +++-----
 arch/arm64/include/asm/signal32.h       | 29 +++++------------
 include/linux/cpufreq.h                 | 21 ++++--------
 include/linux/kconfig.h                 | 31 ++++++++++++++++++
 13 files changed, 110 insertions(+), 163 deletions(-)

-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] kconfig.h: abstract empty functions with STUB_UNLESS macro

[Andrew Murray]

A common pattern found in header files is a function declaration dependent
on a CONFIG_ option being enabled, followed by an empty function for when
that option isn't enabled. This can often take up a lot of space and impact
code readability.

Let's introduce the following STUB_UNLESS macro:

STUB_UNLESS(CONFIG_FOO, [body], prototype)

This evaluates to 'prototype' prepended with 'extern' if CONFIG_FOO is set
to 'y'. Otherwise it will evaluate to 'prototype' prepended with 'static
inline' followed by an empty function body. Where optional argument 'body'
then 'body' will be used as the function body, intended to allow for simple
return statements.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 include/linux/kconfig.h | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
index cc8fa10..216a27f 100644
--- a/include/linux/kconfig.h
+++ b/include/linux/kconfig.h
@@ -12,6 +12,7 @@
 
 #define __ARG_PLACEHOLDER_1 0,
 #define __take_second_arg(__ignored, val, ...) val
+#define __take_fourth_arg(__ignored, __ignored2, __ignored3, val, ...) val
 
 /*
  * The use of "&&" / "||" is limited in certain expressions.
@@ -70,4 +71,34 @@
  */
 #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
 
+
+
+/*
+ * Allow for __stub to be overloaded by making the second argument optional
+ */
+#define __stub_overload(...)		__take_fourth_arg(__VA_ARGS__, \
+					__stub_body, __stub_void)(__VA_ARGS__)
+#define __stub_body(option, body, ptype) __stub(option, body, ptype)
+#define __stub_void(option, ptype)	__stub(option, , ptype)
+
+/*
+ * Using the same trick as __defined we use the config option to conditionally
+ * expand to an extern function declaration or a static stub function.
+ */
+#define __stub(option, body, ptype)	___stub( \
+					__ARG_PLACEHOLDER_##option, body, ptype)
+#define ___stub(arg1_or_junk, body, ptype) __take_second_arg( \
+		arg1_or_junk __extern_ptype(ptype), __static_ptype(body, ptype))
+#define __static_ptype(body, ptype)	static inline ptype { body; }
+#define __extern_ptype(ptype)		extern ptype
+
+/*
+ * STUB_UNLESS(CONFIG_FOO, [body], prototype) evaluates to 'prototype' prepended
+ * with 'extern' if CONFIG_FOO is set to 'y'. Otherwise it will evaluate to
+ * 'prototype' prepended with 'static inline' followed by an empty function
+ * body. Where optional argument 'body' is present then 'body' will be used
+ * as the function body, intended to allow for simple return statements.
+ */
+#define STUB_UNLESS(...) __stub_overload(__VA_ARGS__)
+
 #endif /* __LINUX_KCONFIG_H */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] cpufreq: update headers to use STUB_UNLESS macro

[Andrew Murray]

Use the STUB_UNLESS macro to simplify function declaration.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 include/linux/cpufreq.h | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
index c86d6d8..2c53cae 100644
--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -159,21 +159,12 @@ struct cpufreq_policy {
 #define CPUFREQ_SHARED_TYPE_ALL	 (2) /* All dependent CPUs should set freq */
 #define CPUFREQ_SHARED_TYPE_ANY	 (3) /* Freq can be set from any dependent CPU*/
 
-#ifdef CONFIG_CPU_FREQ
-struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu);
-struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
-void cpufreq_cpu_put(struct cpufreq_policy *policy);
-#else
-static inline struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu)
-{
-	return NULL;
-}
-static inline struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
-{
-	return NULL;
-}
-static inline void cpufreq_cpu_put(struct cpufreq_policy *policy) { }
-#endif
+STUB_UNLESS(CONFIG_CPU_FREQ, return NULL,
+struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu));
+STUB_UNLESS(CONFIG_CPU_FREQ, return NULL,
+struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu));
+STUB_UNLESS(CONFIG_CPU_FREQ,
+void cpufreq_cpu_put(struct cpufreq_policy *policy));
 
 static inline bool policy_is_shared(struct cpufreq_policy *policy)
 {
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] arm64: update headers to use STUB_UNLESS macro

[Andrew Murray]

Use the STUB_UNLESS macro to simplify function declaration.

Signed-off-by: Andrew Murray <andrew.murray@arm.com>
---
 arch/arm64/include/asm/acpi.h           | 38 +++++++++-------------
 arch/arm64/include/asm/alternative.h    |  6 +---
 arch/arm64/include/asm/cpufeature.h     |  6 +---
 arch/arm64/include/asm/cpuidle.h        | 18 +++--------
 arch/arm64/include/asm/debug-monitors.h | 10 ++----
 arch/arm64/include/asm/fpsimd.h         | 57 +++++++++++++--------------------
 arch/arm64/include/asm/hw_breakpoint.h  | 16 +++------
 arch/arm64/include/asm/kasan.h          |  9 ++----
 arch/arm64/include/asm/numa.h           | 19 ++++-------
 arch/arm64/include/asm/ptdump.h         | 13 +++-----
 arch/arm64/include/asm/signal32.h       | 29 +++++------------
 11 files changed, 73 insertions(+), 148 deletions(-)

diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 2def77e..81836ff 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -109,22 +109,17 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)
 }
 
 static inline void arch_fix_phys_package_id(int num, u32 slot) { }
-void __init acpi_init_cpus(void);
-
-#else
-static inline void acpi_init_cpus(void) { }
 #endif /* CONFIG_ACPI */
 
-#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
-bool acpi_parking_protocol_valid(int cpu);
+STUB_UNLESS(CONFIG_ACPI,
+void __init acpi_init_cpus(void));
+
+STUB_UNLESS(CONFIG_ARM64_ACPI_PARKING_PROTOCOL, return false,
+bool acpi_parking_protocol_valid(int cpu));
+
+STUB_UNLESS(CONFIG_ARM64_ACPI_PARKING_PROTOCOL,
 void __init
-acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
-#else
-static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
-static inline void
-acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
-{}
-#endif
+acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor));
 
 static inline const char *acpi_get_enable_method(int cpu)
 {
@@ -152,15 +147,14 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
 }
 #endif /* CONFIG_ACPI_APEI */
 
-#ifdef CONFIG_ACPI_NUMA
-int arm64_acpi_numa_init(void);
-int acpi_numa_get_nid(unsigned int cpu);
-void acpi_map_cpus_to_nodes(void);
-#else
-static inline int arm64_acpi_numa_init(void) { return -ENOSYS; }
-static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
-static inline void acpi_map_cpus_to_nodes(void) { }
-#endif /* CONFIG_ACPI_NUMA */
+STUB_UNLESS(CONFIG_ACPI_NUMA, return -ENOSYS,
+int arm64_acpi_numa_init(void));
+
+STUB_UNLESS(CONFIG_ACPI_NUMA, return NUMA_NO_NODE,
+int acpi_numa_get_nid(unsigned int cpu));
+
+STUB_UNLESS(CONFIG_ACPI_NUMA,
+void acpi_map_cpus_to_nodes(void));
 
 #define ACPI_TABLE_UPGRADE_MAX_PHYS MEMBLOCK_ALLOC_ACCESSIBLE
 
diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
index 4b650ec..3dfbc15 100644
--- a/arch/arm64/include/asm/alternative.h
+++ b/arch/arm64/include/asm/alternative.h
@@ -29,11 +29,7 @@ typedef void (*alternative_cb_t)(struct alt_instr *alt,
 
 void __init apply_alternatives_all(void);
 
-#ifdef CONFIG_MODULES
-void apply_alternatives_module(void *start, size_t length);
-#else
-static inline void apply_alternatives_module(void *start, size_t length) { }
-#endif
+STUB_UNLESS(CONFIG_MODULES, void apply_alternatives_module(void *start, size_t length));
 
 #define ALTINSTR_ENTRY(feature,cb)					      \
 	" .word 661b - .\n"				/* label           */ \
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index dfcfba7..27edb5e 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -628,11 +628,7 @@ static inline int arm64_get_ssbd_state(void)
 #endif
 }
 
-#ifdef CONFIG_ARM64_SSBD
-void arm64_set_ssbd_mitigation(bool state);
-#else
-static inline void arm64_set_ssbd_mitigation(bool state) {}
-#endif
+STUB_UNLESS(CONFIG_ARM64_SSBD, void arm64_set_ssbd_mitigation(bool state));
 
 extern int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
 
diff --git a/arch/arm64/include/asm/cpuidle.h b/arch/arm64/include/asm/cpuidle.h
index 3c5ddb4..fecde4f 100644
--- a/arch/arm64/include/asm/cpuidle.h
+++ b/arch/arm64/include/asm/cpuidle.h
@@ -4,18 +4,10 @@
 
 #include <asm/proc-fns.h>
 
-#ifdef CONFIG_CPU_IDLE
-extern int arm_cpuidle_init(unsigned int cpu);
-extern int arm_cpuidle_suspend(int index);
-#else
-static inline int arm_cpuidle_init(unsigned int cpu)
-{
-	return -EOPNOTSUPP;
-}
+STUB_UNLESS(CONFIG_CPU_IDLE, return -EOPNOTSUPP,
+int arm_cpuidle_init(unsigned int cpu));
+
+STUB_UNLESS(CONFIG_CPU_IDLE, return -EOPNOTSUPP,
+int arm_cpuidle_suspend(int index));
 
-static inline int arm_cpuidle_suspend(int index)
-{
-	return -EOPNOTSUPP;
-}
-#endif
 #endif
diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
index a44cf52..8b91d8e 100644
--- a/arch/arm64/include/asm/debug-monitors.h
+++ b/arch/arm64/include/asm/debug-monitors.h
@@ -124,14 +124,8 @@ void kernel_enable_single_step(struct pt_regs *regs);
 void kernel_disable_single_step(void);
 int kernel_active_single_step(void);
 
-#ifdef CONFIG_HAVE_HW_BREAKPOINT
-int reinstall_suspended_bps(struct pt_regs *regs);
-#else
-static inline int reinstall_suspended_bps(struct pt_regs *regs)
-{
-	return -ENODEV;
-}
-#endif
+STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT, return -ENODEV,
+int reinstall_suspended_bps(struct pt_regs *regs));
 
 int aarch32_break_handler(struct pt_regs *regs);
 
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index dd1ad39..f247eb1 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -92,17 +92,10 @@ extern int __ro_after_init sve_max_vl;
 
 extern size_t sve_state_size(struct task_struct const *task);
 
-extern void sve_alloc(struct task_struct *task);
-extern void fpsimd_release_task(struct task_struct *task);
-extern void fpsimd_sync_to_sve(struct task_struct *task);
-extern void sve_sync_to_fpsimd(struct task_struct *task);
-extern void sve_sync_from_fpsimd_zeropad(struct task_struct *task);
 
 extern int sve_set_vector_length(struct task_struct *task,
 				 unsigned long vl, unsigned long flags);
 
-extern int sve_set_current_vl(unsigned long arg);
-extern int sve_get_current_vl(void);
 
 static inline void sve_user_disable(void)
 {
@@ -113,43 +106,37 @@ static inline void sve_user_enable(void)
 {
 	sysreg_clear_set(cpacr_el1, 0, CPACR_EL1_ZEN_EL0EN);
 }
-
-/*
- * Probing and setup functions.
- * Calls to these functions must be serialised with one another.
- */
-extern void __init sve_init_vq_map(void);
-extern void sve_update_vq_map(void);
-extern int sve_verify_vq_map(void);
-extern void __init sve_setup(void);
+extern void fpsimd_sync_to_sve(struct task_struct *task);
 
 #else /* ! CONFIG_ARM64_SVE */
 
-static inline void sve_alloc(struct task_struct *task) { }
-static inline void fpsimd_release_task(struct task_struct *task) { }
-static inline void sve_sync_to_fpsimd(struct task_struct *task) { }
-static inline void sve_sync_from_fpsimd_zeropad(struct task_struct *task) { }
-
-static inline int sve_set_current_vl(unsigned long arg)
-{
-	return -EINVAL;
-}
-
-static inline int sve_get_current_vl(void)
-{
-	return -EINVAL;
-}
-
 static inline void sve_user_disable(void) { BUILD_BUG(); }
 static inline void sve_user_enable(void) { BUILD_BUG(); }
 
-static inline void sve_init_vq_map(void) { }
-static inline void sve_update_vq_map(void) { }
-static inline int sve_verify_vq_map(void) { return 0; }
-static inline void sve_setup(void) { }
 
 #endif /* ! CONFIG_ARM64_SVE */
 
+STUB_UNLESS(CONFIG_ARM64_SVE, return -EINVAL,
+int sve_set_current_vl(unsigned long arg));
+
+STUB_UNLESS(CONFIG_ARM64_SVE, return -EINVAL,
+int sve_get_current_vl(void));
+
+STUB_UNLESS(CONFIG_ARM64_SVE, void sve_alloc(struct task_struct *task));
+STUB_UNLESS(CONFIG_ARM64_SVE, void fpsimd_release_task(struct task_struct *task));
+STUB_UNLESS(CONFIG_ARM64_SVE, void sve_sync_to_fpsimd(struct task_struct *task));
+STUB_UNLESS(CONFIG_ARM64_SVE, void sve_sync_from_fpsimd_zeropad(struct task_struct *task));
+
+/*
+ * Probing and setup functions.
+ * Calls to these functions must be serialised with one another.
+ */
+STUB_UNLESS(CONFIG_ARM64_SVE, void __init sve_init_vq_map(void));
+STUB_UNLESS(CONFIG_ARM64_SVE, void sve_update_vq_map(void));
+STUB_UNLESS(CONFIG_ARM64_SVE, return 0, int sve_verify_vq_map(void));
+STUB_UNLESS(CONFIG_ARM64_SVE, void __init sve_setup(void));
+
+
 /* For use by EFI runtime services calls only */
 extern void __efi_fpsimd_begin(void);
 extern void __efi_fpsimd_end(void);
diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 6a53e59..ba451fc 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -137,17 +137,11 @@ extern void arch_uninstall_hw_breakpoint(struct perf_event *bp);
 extern void hw_breakpoint_pmu_read(struct perf_event *bp);
 extern int hw_breakpoint_slots(int type);
 
-#ifdef CONFIG_HAVE_HW_BREAKPOINT
-extern void hw_breakpoint_thread_switch(struct task_struct *next);
-extern void ptrace_hw_copy_thread(struct task_struct *task);
-#else
-static inline void hw_breakpoint_thread_switch(struct task_struct *next)
-{
-}
-static inline void ptrace_hw_copy_thread(struct task_struct *task)
-{
-}
-#endif
+STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
+void hw_breakpoint_thread_switch(struct task_struct *next));
+
+STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
+void ptrace_hw_copy_thread(struct task_struct *task));
 
 /* Determine number of BRP registers available. */
 static inline int get_num_brps(void)
diff --git a/arch/arm64/include/asm/kasan.h b/arch/arm64/include/asm/kasan.h
index b52aacd..e67d1a3 100644
--- a/arch/arm64/include/asm/kasan.h
+++ b/arch/arm64/include/asm/kasan.h
@@ -36,14 +36,11 @@
 #define KASAN_SHADOW_OFFSET     (KASAN_SHADOW_END - (1ULL << \
 					(64 - KASAN_SHADOW_SCALE_SHIFT)))
 
-void kasan_init(void);
-void kasan_copy_shadow(pgd_t *pgdir);
 asmlinkage void kasan_early_init(void);
-
-#else
-static inline void kasan_init(void) { }
-static inline void kasan_copy_shadow(pgd_t *pgdir) { }
 #endif
 
+STUB_UNLESS(CONFIG_KASAN, void kasan_init(void));
+STUB_UNLESS(CONFIG_KASAN, void kasan_copy_shadow(pgd_t *pgdir));
+
 #endif
 #endif
diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
index 626ad01..700a4a9 100644
--- a/arch/arm64/include/asm/numa.h
+++ b/arch/arm64/include/asm/numa.h
@@ -29,23 +29,16 @@ static inline const struct cpumask *cpumask_of_node(int node)
 }
 #endif
 
-void __init arm64_numa_init(void);
 int __init numa_add_memblk(int nodeid, u64 start, u64 end);
 void __init numa_set_distance(int from, int to, int distance);
 void __init numa_free_distance(void);
-void __init early_map_cpu_to_node(unsigned int cpu, int nid);
-void numa_store_cpu_info(unsigned int cpu);
-void numa_add_cpu(unsigned int cpu);
-void numa_remove_cpu(unsigned int cpu);
-
-#else	/* CONFIG_NUMA */
-
-static inline void numa_store_cpu_info(unsigned int cpu) { }
-static inline void numa_add_cpu(unsigned int cpu) { }
-static inline void numa_remove_cpu(unsigned int cpu) { }
-static inline void arm64_numa_init(void) { }
-static inline void early_map_cpu_to_node(unsigned int cpu, int nid) { }
 
 #endif	/* CONFIG_NUMA */
 
+STUB_UNLESS(CONFIG_NUMA, void __init arm64_numa_init(void));
+STUB_UNLESS(CONFIG_NUMA, void __init early_map_cpu_to_node(unsigned int cpu, int nid));
+STUB_UNLESS(CONFIG_NUMA, void numa_store_cpu_info(unsigned int cpu));
+STUB_UNLESS(CONFIG_NUMA, void numa_add_cpu(unsigned int cpu));
+STUB_UNLESS(CONFIG_NUMA, void numa_remove_cpu(unsigned int cpu));
+
 #endif	/* __ASM_NUMA_H */
diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
index 6afd847..8d27672 100644
--- a/arch/arm64/include/asm/ptdump.h
+++ b/arch/arm64/include/asm/ptdump.h
@@ -33,15 +33,10 @@ struct ptdump_info {
 };
 
 void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
-#ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
-int ptdump_debugfs_register(struct ptdump_info *info, const char *name);
-#else
-static inline int ptdump_debugfs_register(struct ptdump_info *info,
-					const char *name)
-{
-	return 0;
-}
-#endif
+
+STUB_UNLESS(CONFIG_ARM64_PTDUMP_DEBUGFS, return 0,
+int ptdump_debugfs_register(struct ptdump_info *info, const char *name));
+
 void ptdump_check_wx(void);
 #endif /* CONFIG_ARM64_PTDUMP_CORE */
 
diff --git a/arch/arm64/include/asm/signal32.h b/arch/arm64/include/asm/signal32.h
index 81abea0..9dc06bd 100644
--- a/arch/arm64/include/asm/signal32.h
+++ b/arch/arm64/include/asm/signal32.h
@@ -21,30 +21,17 @@
 #include <linux/compat.h>
 
 #define AARCH32_KERN_SIGRET_CODE_OFFSET	0x500
+#endif /* CONFIG_COMPAT */
 
+STUB_UNLESS(CONFIG_COMPAT, return -ENOSYS,
 int compat_setup_frame(int usig, struct ksignal *ksig, sigset_t *set,
-		       struct pt_regs *regs);
-int compat_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
-			  struct pt_regs *regs);
-
-void compat_setup_restart_syscall(struct pt_regs *regs);
-#else
+		       struct pt_regs *regs));
 
-static inline int compat_setup_frame(int usid, struct ksignal *ksig,
-				     sigset_t *set, struct pt_regs *regs)
-{
-	return -ENOSYS;
-}
-
-static inline int compat_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
-					struct pt_regs *regs)
-{
-	return -ENOSYS;
-}
+STUB_UNLESS(CONFIG_COMPAT, return -ENOSYS,
+int compat_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
+			  struct pt_regs *regs));
 
-static inline void compat_setup_restart_syscall(struct pt_regs *regs)
-{
-}
-#endif /* CONFIG_COMPAT */
+STUB_UNLESS(CONFIG_COMPAT,
+void compat_setup_restart_syscall(struct pt_regs *regs));
 #endif /* __KERNEL__ */
 #endif /* __ASM_SIGNAL32_H */
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] kconfig.h: abstract empty functions with STUB_UNLESS macro

[Steven Price]

On 18/01/2019 16:00, Andrew Murray wrote:
> A common pattern found in header files is a function declaration dependent
> on a CONFIG_ option being enabled, followed by an empty function for when
> that option isn't enabled. This can often take up a lot of space and impact
> code readability.
> 
> Let's introduce the following STUB_UNLESS macro:
> 
> STUB_UNLESS(CONFIG_FOO, [body], prototype)
> 
> This evaluates to 'prototype' prepended with 'extern' if CONFIG_FOO is set
> to 'y'. Otherwise it will evaluate to 'prototype' prepended with 'static
> inline' followed by an empty function body. Where optional argument 'body'
> then 'body' will be used as the function body, intended to allow for simple
> return statements.
> 
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>

Seems like it should remove a lot of boilerplate code, so looks good to me.

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  include/linux/kconfig.h | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
> 
> diff --git a/include/linux/kconfig.h b/include/linux/kconfig.h
> index cc8fa10..216a27f 100644
> --- a/include/linux/kconfig.h
> +++ b/include/linux/kconfig.h
> @@ -12,6 +12,7 @@
>  
>  #define __ARG_PLACEHOLDER_1 0,
>  #define __take_second_arg(__ignored, val, ...) val
> +#define __take_fourth_arg(__ignored, __ignored2, __ignored3, val, ...) val
>  
>  /*
>   * The use of "&&" / "||" is limited in certain expressions.
> @@ -70,4 +71,34 @@
>   */
>  #define IS_ENABLED(option) __or(IS_BUILTIN(option), IS_MODULE(option))
>  
> +
> +
> +/*
> + * Allow for __stub to be overloaded by making the second argument optional
> + */
> +#define __stub_overload(...)		__take_fourth_arg(__VA_ARGS__, \
> +					__stub_body, __stub_void)(__VA_ARGS__)
> +#define __stub_body(option, body, ptype) __stub(option, body, ptype)
> +#define __stub_void(option, ptype)	__stub(option, , ptype)
> +
> +/*
> + * Using the same trick as __defined we use the config option to conditionally
> + * expand to an extern function declaration or a static stub function.
> + */
> +#define __stub(option, body, ptype)	___stub( \
> +					__ARG_PLACEHOLDER_##option, body, ptype)
> +#define ___stub(arg1_or_junk, body, ptype) __take_second_arg( \
> +		arg1_or_junk __extern_ptype(ptype), __static_ptype(body, ptype))
> +#define __static_ptype(body, ptype)	static inline ptype { body; }
> +#define __extern_ptype(ptype)		extern ptype
> +
> +/*
> + * STUB_UNLESS(CONFIG_FOO, [body], prototype) evaluates to 'prototype' prepended
> + * with 'extern' if CONFIG_FOO is set to 'y'. Otherwise it will evaluate to
> + * 'prototype' prepended with 'static inline' followed by an empty function
> + * body. Where optional argument 'body' is present then 'body' will be used
> + * as the function body, intended to allow for simple return statements.
> + */
> +#define STUB_UNLESS(...) __stub_overload(__VA_ARGS__)
> +
>  #endif /* __LINUX_KCONFIG_H */
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] cpufreq: update headers to use STUB_UNLESS macro

[Steven Price]

On 18/01/2019 16:00, Andrew Murray wrote:
> Use the STUB_UNLESS macro to simplify function declaration.
> 
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>

Reviewed-by: Steven Price <steven.price@arm.com>

> ---
>  include/linux/cpufreq.h | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
> 
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index c86d6d8..2c53cae 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -159,21 +159,12 @@ struct cpufreq_policy {
>  #define CPUFREQ_SHARED_TYPE_ALL	 (2) /* All dependent CPUs should set freq */
>  #define CPUFREQ_SHARED_TYPE_ANY	 (3) /* Freq can be set from any dependent CPU*/
>  
> -#ifdef CONFIG_CPU_FREQ
> -struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu);
> -struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu);
> -void cpufreq_cpu_put(struct cpufreq_policy *policy);
> -#else
> -static inline struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu)
> -{
> -	return NULL;
> -}
> -static inline struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu)
> -{
> -	return NULL;
> -}
> -static inline void cpufreq_cpu_put(struct cpufreq_policy *policy) { }
> -#endif
> +STUB_UNLESS(CONFIG_CPU_FREQ, return NULL,
> +struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu));
> +STUB_UNLESS(CONFIG_CPU_FREQ, return NULL,
> +struct cpufreq_policy *cpufreq_cpu_get(unsigned int cpu));
> +STUB_UNLESS(CONFIG_CPU_FREQ,
> +void cpufreq_cpu_put(struct cpufreq_policy *policy));
>  
>  static inline bool policy_is_shared(struct cpufreq_policy *policy)
>  {
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro

[Russell King - ARM Linux admin]

On Fri, Jan 18, 2019 at 04:00:27PM +0000, Andrew Murray wrote:
> A common pattern found in header files is a function declaration dependent
> on a CONFIG_ option being enabled, followed by an empty function for when
> that option isn't enabled. This boilerplate code can often take up a lot
> of space and impact code readability.
> 
> This series introduces a STUB_UNLESS macro that simplifies header files as
> follows:
> 
> STUB_UNLESS(CONFIG_FOO, [body], prototype)

Can you explain the desire to make the second argument optional,
rather than having the mandatory arguments first and the optional body
last?  It will mean more lines at each site, but I don't think that's
a bad thing:

STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
void hw_breakpoint_thread_switch(struct task_struct *next));

STUB_UNLESS(CONFIG_CPU_FREQ,
struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu), return NULL);

or:

STUB_UNLESS(CONFIG_CPU_FREQ,
struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu),
	return NULL);

Seems to be more readable in terms of the flow.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro

[Dave Martin]

On Fri, Jan 18, 2019 at 04:37:36PM +0000, Russell King - ARM Linux admin wrote:
> On Fri, Jan 18, 2019 at 04:00:27PM +0000, Andrew Murray wrote:
> > A common pattern found in header files is a function declaration dependent
> > on a CONFIG_ option being enabled, followed by an empty function for when
> > that option isn't enabled. This boilerplate code can often take up a lot
> > of space and impact code readability.
> > 
> > This series introduces a STUB_UNLESS macro that simplifies header files as
> > follows:
> > 
> > STUB_UNLESS(CONFIG_FOO, [body], prototype)
> 
> Can you explain the desire to make the second argument optional,
> rather than having the mandatory arguments first and the optional body
> last?  It will mean more lines at each site, but I don't think that's
> a bad thing:
> 
> STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> void hw_breakpoint_thread_switch(struct task_struct *next));
> 
> STUB_UNLESS(CONFIG_CPU_FREQ,
> struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu), return NULL);
> 
> or:
> 
> STUB_UNLESS(CONFIG_CPU_FREQ,
> struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu),
> 	return NULL);
> 
> Seems to be more readable in terms of the flow.

Hmmm, looking at that, I probably prefer that too.

In the unlikely case that <body> uses the function arguments it would be
quite confusing to have the body before the function prototype.

If we can keep this down to two lines so much the better, but still
seems fine.

Provided we don't end up needing a trailing comma in the void case, to
supply the empty body argument, that is.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] arm64: update headers to use STUB_UNLESS macro

[Steven Price]

On 18/01/2019 16:00, Andrew Murray wrote:
> Use the STUB_UNLESS macro to simplify function declaration.
> 
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>  arch/arm64/include/asm/acpi.h           | 38 +++++++++-------------
>  arch/arm64/include/asm/alternative.h    |  6 +---
>  arch/arm64/include/asm/cpufeature.h     |  6 +---
>  arch/arm64/include/asm/cpuidle.h        | 18 +++--------
>  arch/arm64/include/asm/debug-monitors.h | 10 ++----
>  arch/arm64/include/asm/fpsimd.h         | 57 +++++++++++++--------------------
>  arch/arm64/include/asm/hw_breakpoint.h  | 16 +++------
>  arch/arm64/include/asm/kasan.h          |  9 ++----
>  arch/arm64/include/asm/numa.h           | 19 ++++-------
>  arch/arm64/include/asm/ptdump.h         | 13 +++-----
>  arch/arm64/include/asm/signal32.h       | 29 +++++------------
>  11 files changed, 73 insertions(+), 148 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
> index 2def77e..81836ff 100644
> --- a/arch/arm64/include/asm/acpi.h
> +++ b/arch/arm64/include/asm/acpi.h
> @@ -109,22 +109,17 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)
>  }
>  
>  static inline void arch_fix_phys_package_id(int num, u32 slot) { }
> -void __init acpi_init_cpus(void);
> -
> -#else
> -static inline void acpi_init_cpus(void) { }
>  #endif /* CONFIG_ACPI */
>  
> -#ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> -bool acpi_parking_protocol_valid(int cpu);
> +STUB_UNLESS(CONFIG_ACPI,
> +void __init acpi_init_cpus(void));
> +
> +STUB_UNLESS(CONFIG_ARM64_ACPI_PARKING_PROTOCOL, return false,
> +bool acpi_parking_protocol_valid(int cpu));
> +
> +STUB_UNLESS(CONFIG_ARM64_ACPI_PARKING_PROTOCOL,
>  void __init
> -acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor);
> -#else
> -static inline bool acpi_parking_protocol_valid(int cpu) { return false; }
> -static inline void
> -acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor)
> -{}
> -#endif
> +acpi_set_mailbox_entry(int cpu, struct acpi_madt_generic_interrupt *processor));

For these definitions we end up with a "static inline __init" function
which seems a bit weird to me, but apparently we already have many of them.

>  static inline const char *acpi_get_enable_method(int cpu)
>  {
> @@ -152,15 +147,14 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
>  }
>  #endif /* CONFIG_ACPI_APEI */
>  
> -#ifdef CONFIG_ACPI_NUMA
> -int arm64_acpi_numa_init(void);
> -int acpi_numa_get_nid(unsigned int cpu);
> -void acpi_map_cpus_to_nodes(void);
> -#else
> -static inline int arm64_acpi_numa_init(void) { return -ENOSYS; }
> -static inline int acpi_numa_get_nid(unsigned int cpu) { return NUMA_NO_NODE; }
> -static inline void acpi_map_cpus_to_nodes(void) { }
> -#endif /* CONFIG_ACPI_NUMA */
> +STUB_UNLESS(CONFIG_ACPI_NUMA, return -ENOSYS,
> +int arm64_acpi_numa_init(void));
> +
> +STUB_UNLESS(CONFIG_ACPI_NUMA, return NUMA_NO_NODE,
> +int acpi_numa_get_nid(unsigned int cpu));
> +
> +STUB_UNLESS(CONFIG_ACPI_NUMA,
> +void acpi_map_cpus_to_nodes(void));
>  
>  #define ACPI_TABLE_UPGRADE_MAX_PHYS MEMBLOCK_ALLOC_ACCESSIBLE
>  
> diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h
> index 4b650ec..3dfbc15 100644
> --- a/arch/arm64/include/asm/alternative.h
> +++ b/arch/arm64/include/asm/alternative.h
> @@ -29,11 +29,7 @@ typedef void (*alternative_cb_t)(struct alt_instr *alt,
>  
>  void __init apply_alternatives_all(void);
>  
> -#ifdef CONFIG_MODULES
> -void apply_alternatives_module(void *start, size_t length);
> -#else
> -static inline void apply_alternatives_module(void *start, size_t length) { }
> -#endif
> +STUB_UNLESS(CONFIG_MODULES, void apply_alternatives_module(void *start, size_t length));

Nit: You should probably wrap this line.

>  
>  #define ALTINSTR_ENTRY(feature,cb)					      \
>  	" .word 661b - .\n"				/* label           */ \
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index dfcfba7..27edb5e 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -628,11 +628,7 @@ static inline int arm64_get_ssbd_state(void)
>  #endif
>  }
>  
> -#ifdef CONFIG_ARM64_SSBD
> -void arm64_set_ssbd_mitigation(bool state);
> -#else
> -static inline void arm64_set_ssbd_mitigation(bool state) {}
> -#endif
> +STUB_UNLESS(CONFIG_ARM64_SSBD, void arm64_set_ssbd_mitigation(bool state));
>  
>  extern int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
>  
> diff --git a/arch/arm64/include/asm/cpuidle.h b/arch/arm64/include/asm/cpuidle.h
> index 3c5ddb4..fecde4f 100644
> --- a/arch/arm64/include/asm/cpuidle.h
> +++ b/arch/arm64/include/asm/cpuidle.h
> @@ -4,18 +4,10 @@
>  
>  #include <asm/proc-fns.h>
>  
> -#ifdef CONFIG_CPU_IDLE
> -extern int arm_cpuidle_init(unsigned int cpu);
> -extern int arm_cpuidle_suspend(int index);
> -#else
> -static inline int arm_cpuidle_init(unsigned int cpu)
> -{
> -	return -EOPNOTSUPP;
> -}
> +STUB_UNLESS(CONFIG_CPU_IDLE, return -EOPNOTSUPP,
> +int arm_cpuidle_init(unsigned int cpu));
> +
> +STUB_UNLESS(CONFIG_CPU_IDLE, return -EOPNOTSUPP,
> +int arm_cpuidle_suspend(int index));
>  
> -static inline int arm_cpuidle_suspend(int index)
> -{
> -	return -EOPNOTSUPP;
> -}
> -#endif
>  #endif
> diff --git a/arch/arm64/include/asm/debug-monitors.h b/arch/arm64/include/asm/debug-monitors.h
> index a44cf52..8b91d8e 100644
> --- a/arch/arm64/include/asm/debug-monitors.h
> +++ b/arch/arm64/include/asm/debug-monitors.h
> @@ -124,14 +124,8 @@ void kernel_enable_single_step(struct pt_regs *regs);
>  void kernel_disable_single_step(void);
>  int kernel_active_single_step(void);
>  
> -#ifdef CONFIG_HAVE_HW_BREAKPOINT
> -int reinstall_suspended_bps(struct pt_regs *regs);
> -#else
> -static inline int reinstall_suspended_bps(struct pt_regs *regs)
> -{
> -	return -ENODEV;
> -}
> -#endif
> +STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT, return -ENODEV,
> +int reinstall_suspended_bps(struct pt_regs *regs));
>  
>  int aarch32_break_handler(struct pt_regs *regs);
>  
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index dd1ad39..f247eb1 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -92,17 +92,10 @@ extern int __ro_after_init sve_max_vl;
>  
>  extern size_t sve_state_size(struct task_struct const *task);
>  
> -extern void sve_alloc(struct task_struct *task);
> -extern void fpsimd_release_task(struct task_struct *task);
> -extern void fpsimd_sync_to_sve(struct task_struct *task);
> -extern void sve_sync_to_fpsimd(struct task_struct *task);
> -extern void sve_sync_from_fpsimd_zeropad(struct task_struct *task);
>  
>  extern int sve_set_vector_length(struct task_struct *task,
>  				 unsigned long vl, unsigned long flags);
>  
> -extern int sve_set_current_vl(unsigned long arg);
> -extern int sve_get_current_vl(void);
>  
>  static inline void sve_user_disable(void)
>  {
> @@ -113,43 +106,37 @@ static inline void sve_user_enable(void)
>  {
>  	sysreg_clear_set(cpacr_el1, 0, CPACR_EL1_ZEN_EL0EN);
>  }
> -
> -/*
> - * Probing and setup functions.
> - * Calls to these functions must be serialised with one another.
> - */
> -extern void __init sve_init_vq_map(void);
> -extern void sve_update_vq_map(void);
> -extern int sve_verify_vq_map(void);
> -extern void __init sve_setup(void);
> +extern void fpsimd_sync_to_sve(struct task_struct *task);

Why has this declaration been moved down? It would seem neater to leave
it with the others that remain above.

>  
>  #else /* ! CONFIG_ARM64_SVE */
>  
> -static inline void sve_alloc(struct task_struct *task) { }
> -static inline void fpsimd_release_task(struct task_struct *task) { }
> -static inline void sve_sync_to_fpsimd(struct task_struct *task) { }
> -static inline void sve_sync_from_fpsimd_zeropad(struct task_struct *task) { }
> -
> -static inline int sve_set_current_vl(unsigned long arg)
> -{
> -	return -EINVAL;
> -}
> -
> -static inline int sve_get_current_vl(void)
> -{
> -	return -EINVAL;
> -}
> -
>  static inline void sve_user_disable(void) { BUILD_BUG(); }
>  static inline void sve_user_enable(void) { BUILD_BUG(); }
>  
> -static inline void sve_init_vq_map(void) { }
> -static inline void sve_update_vq_map(void) { }
> -static inline int sve_verify_vq_map(void) { return 0; }
> -static inline void sve_setup(void) { }
>  
>  #endif /* ! CONFIG_ARM64_SVE */
>  
> +STUB_UNLESS(CONFIG_ARM64_SVE, return -EINVAL,
> +int sve_set_current_vl(unsigned long arg));
> +
> +STUB_UNLESS(CONFIG_ARM64_SVE, return -EINVAL,
> +int sve_get_current_vl(void));
> +
> +STUB_UNLESS(CONFIG_ARM64_SVE, void sve_alloc(struct task_struct *task));
> +STUB_UNLESS(CONFIG_ARM64_SVE, void fpsimd_release_task(struct task_struct *task));
> +STUB_UNLESS(CONFIG_ARM64_SVE, void sve_sync_to_fpsimd(struct task_struct *task));
> +STUB_UNLESS(CONFIG_ARM64_SVE, void sve_sync_from_fpsimd_zeropad(struct task_struct *task));

Nit: Your lines are getting long again, consider wrapping.

> +
> +/*
> + * Probing and setup functions.
> + * Calls to these functions must be serialised with one another.
> + */
> +STUB_UNLESS(CONFIG_ARM64_SVE, void __init sve_init_vq_map(void));
> +STUB_UNLESS(CONFIG_ARM64_SVE, void sve_update_vq_map(void));
> +STUB_UNLESS(CONFIG_ARM64_SVE, return 0, int sve_verify_vq_map(void));

Nit: Personally I'd find the "return 0" easier to see if you
consistently wrapped the declaration onto a new line.

> +STUB_UNLESS(CONFIG_ARM64_SVE, void __init sve_setup(void));
> +
> +
>  /* For use by EFI runtime services calls only */
>  extern void __efi_fpsimd_begin(void);
>  extern void __efi_fpsimd_end(void);
> diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
> index 6a53e59..ba451fc 100644
> --- a/arch/arm64/include/asm/hw_breakpoint.h
> +++ b/arch/arm64/include/asm/hw_breakpoint.h
> @@ -137,17 +137,11 @@ extern void arch_uninstall_hw_breakpoint(struct perf_event *bp);
>  extern void hw_breakpoint_pmu_read(struct perf_event *bp);
>  extern int hw_breakpoint_slots(int type);
>  
> -#ifdef CONFIG_HAVE_HW_BREAKPOINT
> -extern void hw_breakpoint_thread_switch(struct task_struct *next);
> -extern void ptrace_hw_copy_thread(struct task_struct *task);
> -#else
> -static inline void hw_breakpoint_thread_switch(struct task_struct *next)
> -{
> -}
> -static inline void ptrace_hw_copy_thread(struct task_struct *task)
> -{
> -}
> -#endif
> +STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> +void hw_breakpoint_thread_switch(struct task_struct *next));
> +
> +STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> +void ptrace_hw_copy_thread(struct task_struct *task));
>  
>  /* Determine number of BRP registers available. */
>  static inline int get_num_brps(void)
> diff --git a/arch/arm64/include/asm/kasan.h b/arch/arm64/include/asm/kasan.h
> index b52aacd..e67d1a3 100644
> --- a/arch/arm64/include/asm/kasan.h
> +++ b/arch/arm64/include/asm/kasan.h
> @@ -36,14 +36,11 @@
>  #define KASAN_SHADOW_OFFSET     (KASAN_SHADOW_END - (1ULL << \
>  					(64 - KASAN_SHADOW_SCALE_SHIFT)))
>  
> -void kasan_init(void);
> -void kasan_copy_shadow(pgd_t *pgdir);
>  asmlinkage void kasan_early_init(void);
> -
> -#else
> -static inline void kasan_init(void) { }
> -static inline void kasan_copy_shadow(pgd_t *pgdir) { }
>  #endif
>  
> +STUB_UNLESS(CONFIG_KASAN, void kasan_init(void));
> +STUB_UNLESS(CONFIG_KASAN, void kasan_copy_shadow(pgd_t *pgdir));
> +
>  #endif
>  #endif
> diff --git a/arch/arm64/include/asm/numa.h b/arch/arm64/include/asm/numa.h
> index 626ad01..700a4a9 100644
> --- a/arch/arm64/include/asm/numa.h
> +++ b/arch/arm64/include/asm/numa.h
> @@ -29,23 +29,16 @@ static inline const struct cpumask *cpumask_of_node(int node)
>  }
>  #endif
>  
> -void __init arm64_numa_init(void);
>  int __init numa_add_memblk(int nodeid, u64 start, u64 end);
>  void __init numa_set_distance(int from, int to, int distance);
>  void __init numa_free_distance(void);
> -void __init early_map_cpu_to_node(unsigned int cpu, int nid);
> -void numa_store_cpu_info(unsigned int cpu);
> -void numa_add_cpu(unsigned int cpu);
> -void numa_remove_cpu(unsigned int cpu);
> -
> -#else	/* CONFIG_NUMA */
> -
> -static inline void numa_store_cpu_info(unsigned int cpu) { }
> -static inline void numa_add_cpu(unsigned int cpu) { }
> -static inline void numa_remove_cpu(unsigned int cpu) { }
> -static inline void arm64_numa_init(void) { }
> -static inline void early_map_cpu_to_node(unsigned int cpu, int nid) { }
>  
>  #endif	/* CONFIG_NUMA */
>  
> +STUB_UNLESS(CONFIG_NUMA, void __init arm64_numa_init(void));
> +STUB_UNLESS(CONFIG_NUMA, void __init early_map_cpu_to_node(unsigned int cpu, int nid));

Nit: Long line, consider wrapping

Steve

> +STUB_UNLESS(CONFIG_NUMA, void numa_store_cpu_info(unsigned int cpu));
> +STUB_UNLESS(CONFIG_NUMA, void numa_add_cpu(unsigned int cpu));
> +STUB_UNLESS(CONFIG_NUMA, void numa_remove_cpu(unsigned int cpu));
> +
>  #endif	/* __ASM_NUMA_H */
> diff --git a/arch/arm64/include/asm/ptdump.h b/arch/arm64/include/asm/ptdump.h
> index 6afd847..8d27672 100644
> --- a/arch/arm64/include/asm/ptdump.h
> +++ b/arch/arm64/include/asm/ptdump.h
> @@ -33,15 +33,10 @@ struct ptdump_info {
>  };
>  
>  void ptdump_walk_pgd(struct seq_file *s, struct ptdump_info *info);
> -#ifdef CONFIG_ARM64_PTDUMP_DEBUGFS
> -int ptdump_debugfs_register(struct ptdump_info *info, const char *name);
> -#else
> -static inline int ptdump_debugfs_register(struct ptdump_info *info,
> -					const char *name)
> -{
> -	return 0;
> -}
> -#endif
> +
> +STUB_UNLESS(CONFIG_ARM64_PTDUMP_DEBUGFS, return 0,
> +int ptdump_debugfs_register(struct ptdump_info *info, const char *name));
> +
>  void ptdump_check_wx(void);
>  #endif /* CONFIG_ARM64_PTDUMP_CORE */
>  
> diff --git a/arch/arm64/include/asm/signal32.h b/arch/arm64/include/asm/signal32.h
> index 81abea0..9dc06bd 100644
> --- a/arch/arm64/include/asm/signal32.h
> +++ b/arch/arm64/include/asm/signal32.h
> @@ -21,30 +21,17 @@
>  #include <linux/compat.h>
>  
>  #define AARCH32_KERN_SIGRET_CODE_OFFSET	0x500
> +#endif /* CONFIG_COMPAT */
>  
> +STUB_UNLESS(CONFIG_COMPAT, return -ENOSYS,
>  int compat_setup_frame(int usig, struct ksignal *ksig, sigset_t *set,
> -		       struct pt_regs *regs);
> -int compat_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
> -			  struct pt_regs *regs);
> -
> -void compat_setup_restart_syscall(struct pt_regs *regs);
> -#else
> +		       struct pt_regs *regs));
>  
> -static inline int compat_setup_frame(int usid, struct ksignal *ksig,
> -				     sigset_t *set, struct pt_regs *regs)
> -{
> -	return -ENOSYS;
> -}
> -
> -static inline int compat_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
> -					struct pt_regs *regs)
> -{
> -	return -ENOSYS;
> -}
> +STUB_UNLESS(CONFIG_COMPAT, return -ENOSYS,
> +int compat_setup_rt_frame(int usig, struct ksignal *ksig, sigset_t *set,
> +			  struct pt_regs *regs));
>  
> -static inline void compat_setup_restart_syscall(struct pt_regs *regs)
> -{
> -}
> -#endif /* CONFIG_COMPAT */
> +STUB_UNLESS(CONFIG_COMPAT,
> +void compat_setup_restart_syscall(struct pt_regs *regs));
>  #endif /* __KERNEL__ */
>  #endif /* __ASM_SIGNAL32_H */
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro

[Andrew Murray]

On Fri, Jan 18, 2019 at 04:44:25PM +0000, Dave Martin wrote:
> On Fri, Jan 18, 2019 at 04:37:36PM +0000, Russell King - ARM Linux admin wrote:
> > On Fri, Jan 18, 2019 at 04:00:27PM +0000, Andrew Murray wrote:
> > > A common pattern found in header files is a function declaration dependent
> > > on a CONFIG_ option being enabled, followed by an empty function for when
> > > that option isn't enabled. This boilerplate code can often take up a lot
> > > of space and impact code readability.
> > > 
> > > This series introduces a STUB_UNLESS macro that simplifies header files as
> > > follows:
> > > 
> > > STUB_UNLESS(CONFIG_FOO, [body], prototype)
> > 
> > Can you explain the desire to make the second argument optional,
> > rather than having the mandatory arguments first and the optional body
> > last?  It will mean more lines at each site, but I don't think that's
> > a bad thing:

My intent was to make the function prototype look like an ordinary prototype
but with all this special macro stuff on the preceding line. Much like this:

> > 
> > STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> > void hw_breakpoint_thread_switch(struct task_struct *next));

Besides the extra ')' at the end it looks like a normal prototype. I felt this
may be important as existing tooling (ctags etc) might have a better chance of
recognising it and it wouldn't be so alien to new developers. I feared that if
the 'prototype' argument was in the middle then it would get lost in all the
other arguments and be less readable as a prototype.

> > 
> > STUB_UNLESS(CONFIG_CPU_FREQ,
> > struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu), return NULL);
> > 
> > or:
> > 
> > STUB_UNLESS(CONFIG_CPU_FREQ,
> > struct cpufreq_policy *cpufreq_cpu_get_raw(unsigned int cpu),
> > 	return NULL);

As you indicate here, it's possible to spread this to three lines and keep the
readability of the prototype - though I was keen to condense it to as few lines
as possible (I was probably putting too much focus on the diff stat).

> > 
> > Seems to be more readable in terms of the flow.
> 
> Hmmm, looking at that, I probably prefer that too.

Feedback I've had so far suggests that there is a preference to putting the
optional argument at the end, I have no objection to this.

> 
> In the unlikely case that <body> uses the function arguments it would be
> quite confusing to have the body before the function prototype.
> 
> If we can keep this down to two lines so much the better, but still
> seems fine.

This is the compromise - having the optional argument after the prototype
will likely result in wrapping to the next line as prototypes tend to be
long. Perhaps this is more readable.

> 
> Provided we don't end up needing a trailing comma in the void case, to
> supply the empty body argument, that is.

No this isn't necessary.

Thanks,

Andrew Murray

> 
> Cheers
> ---Dave

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [Linux-eng] [RFC 0/3] Abstract empty functions with STUB_UNLESS macro

[Masahiro Yamada]

On Sat, Jan 19, 2019 at 1:02 AM Andrew Murray <andrew.murray@arm.com> wrote:
>
> A common pattern found in header files is a function declaration dependent
> on a CONFIG_ option being enabled, followed by an empty function for when
> that option isn't enabled. This boilerplate code can often take up a lot
> of space and impact code readability.
>
> This series introduces a STUB_UNLESS macro that simplifies header files as
> follows:
>
> STUB_UNLESS(CONFIG_FOO, [body], prototype)
>
> This evaluates to 'prototype' prepended with 'extern' if CONFIG_FOO is set
> to 'y'. Otherwise it will evaluate to 'prototype' prepended with 'static
> inline' followed by an empty function body. Where optional argument 'body'
> is present then 'body' will be used as the function body, intended to allow
> simple return statements. Using the macro results in hunks such as this:
>
> -#ifdef CONFIG_HAVE_HW_BREAKPOINT
> -extern void hw_breakpoint_thread_switch(struct task_struct *next);
> -extern void ptrace_hw_copy_thread(struct task_struct *task);
> -#else
> -static inline void hw_breakpoint_thread_switch(struct task_struct *next)
> -{
> -}
> -static inline void ptrace_hw_copy_thread(struct task_struct *task)
> -{
> -}
> -#endif
> +STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> +void hw_breakpoint_thread_switch(struct task_struct *next));
> +
> +STUB_UNLESS(CONFIG_HAVE_HW_BREAKPOINT,
> +void ptrace_hw_copy_thread(struct task_struct *task));
>
> This may or may not be considered as more desirable than the status quo.
>
> This series updates arm64 and perf to use the macro as an example.
>
> Andrew Murray (3):
>   kconfig.h: abstract empty functions with STUB_UNLESS macro
>   cpufreq: update headers to use STUB_UNLESS macro
>   arm64: update headers to use STUB_UNLESS macro
>
>  arch/arm64/include/asm/acpi.h           | 38 +++++++++-------------
>  arch/arm64/include/asm/alternative.h    |  6 +---
>  arch/arm64/include/asm/cpufeature.h     |  6 +---
>  arch/arm64/include/asm/cpuidle.h        | 18 +++--------
>  arch/arm64/include/asm/debug-monitors.h | 10 ++----
>  arch/arm64/include/asm/fpsimd.h         | 57 +++++++++++++--------------------
>  arch/arm64/include/asm/hw_breakpoint.h  | 16 +++------
>  arch/arm64/include/asm/kasan.h          |  9 ++----
>  arch/arm64/include/asm/numa.h           | 19 ++++-------
>  arch/arm64/include/asm/ptdump.h         | 13 +++-----
>  arch/arm64/include/asm/signal32.h       | 29 +++++------------
>  include/linux/cpufreq.h                 | 21 ++++--------
>  include/linux/kconfig.h                 | 31 ++++++++++++++++++
>  13 files changed, 110 insertions(+), 163 deletions(-)
>
> --
> 2.7.4
>

Honestly, I am not a big fan of shorting the code
for the purpose of shorting.


This patch series is based on the assumption
the first argument is "defined or undefined".
In other words, it cannot handle tristate CONFIG option.


But, if you go this way, could you make
it work in a more generic manner?

And, decouple this from <linux/kconfig.h>



For example, see the following cases:

https://github.com/torvalds/linux/blob/v5.0-rc2/include/acpi/button.h#L7

https://github.com/torvalds/linux/blob/v5.0-rc2/include/linux/firmware.h#L42




The latter is a good example.

In many cases, the kernel headers look like this:


#ifdef CONFIG_FOO

  { large block of prototype declaration }

#else

  { large block of nop stubs }

#endif


So, this approach shifts "duplication of prototype"
into "duplication of conditional part".


-- 
Best Regards
Masahiro Yamada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel