Skip to contentSkip to conversation

assoc_array: Include rcupdate.h for call_rcu() definition

PATCH Applied Pranith Kumar 6 participants 10 messages Dec 30, 2014 – Jan 5, 2015

Subsystems (4)
PowerPC Library code s390 Sleepable read-copy update (SRCU)
lore
Share & tools
lore

Pranith Kumar

author
#0 lore raw

PATCH 1/1 assoc_array: Include rcupdate.h for call_rcu() definition

Include rcupdate.h header to provide call_rcu() definition. This was implicitly being provided by slab.h file which include srcu.h somewhere in its include hierarchy which in-turn included rcupdate.h.

Lately, tinification effort added support to remove srcu entirely because of which we are encountering build errors like

lib/assoc_array.c: In function 'assoc_array_apply_edit':
lib/assoc_array.c:1426:2: error: implicit declaration of function 'call_rcu' [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors

Fix these by including rcupdate.h explicitly.

Signed-off-by Pranith Kumar <[email protected]>
Reported-by Scott Wood <[email protected]>
---
 lib/assoc_array.c | 1 +
 1 file changed, 1 insertion(+)
lib/assoc_array.c +1 -0
index 2404d03..03dd576 100644
@@ -11,6 +11,7 @@
  * 2 of the Licence, or (at your option) any later version.
  */
 //#define DEBUG
+#include <linux/rcupdate.h>
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/assoc_array_priv.h>
Signature · 1 line
1.9.1
author
#1 lore raw

PATCH v2 1/1 srcu: Isolate srcu sections using CONFIG_SRCU

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

There are places which include kvm headers and utilize kvm data structures without checking if KVM is enabled. In two such archs(s390, ppc64), the current patch makes the uses of KVM conditional on KVM being enabled. The other option, which is to enable KVM unconditionally seemed a bit too much as we could easily figure out KVM only parts and enclose them in ifdefs.

Signed-off-by Pranith Kumar <[email protected]>
CC Scott Wood <[email protected]>

---
v2:
- fix build failures reported by Scott Wood

 arch/powerpc/kernel/setup_64.c |  7 ++++++-
 arch/powerpc/kernel/smp.c      |  9 +++++++-
 arch/s390/kernel/asm-offsets.c |  7 ++++++-
 include/linux/notifier.h       | 47 ++++++++++++++++++++++++------------------
 include/linux/srcu.h           |  6 +++++-
 5 files changed, 52 insertions(+), 24 deletions(-)
arch/powerpc/kernel/setup_64.c +6 -1
index 4f3cfe1..f55302f 100644
@@ -65,10 +65,13 @@
 #include <asm/kexec.h>
 #include <asm/mmu_context.h>
 #include <asm/code-patching.h>
-#include <asm/kvm_ppc.h>
 #include <asm/hugetlb.h>
 #include <asm/epapr_hcalls.h>
 
+#if IS_ENABLED(CONFIG_KVM)
+#include <asm/kvm_ppc.h>
+#endif
+
 #ifdef DEBUG
 #define DBG(fmt...) udbg_printf(fmt)
 #else

@@ -286,8 +289,10 @@ void __init early_setup(unsigned long dt_ptr)
 	 */
 	cpu_ready_for_interrupts();
 
+#if IS_ENABLED(CONFIG_KVM)
 	/* Reserve large chunks of memory for use by CMA for KVM */
 	kvm_cma_reserve();
+#endif
 
 	/*
 	 * Reserve any gigantic pages requested on the command line.
arch/powerpc/kernel/smp.c +8 -1
index 71e186d..0001daa 100644
@@ -36,7 +36,6 @@
 #include <linux/atomic.h>
 #include <asm/irq.h>
 #include <asm/hw_irq.h>
-#include <asm/kvm_ppc.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
 #include <asm/prom.h>

@@ -54,6 +53,10 @@
 #include <asm/debug.h>
 #include <asm/kexec.h>
 
+#if IS_ENABLED(CONFIG_KVM)
+#include <asm/kvm_ppc.h>
+#endif
+
 #ifdef DEBUG
 #include <asm/udbg.h>
 #define DBG(fmt...) udbg_printf(fmt)

@@ -470,7 +473,11 @@ int generic_check_cpu_restart(unsigned int cpu)
 
 static bool secondaries_inhibited(void)
 {
+#if IS_ENABLED(CONFIG_KVM)
 	return kvm_hv_mode_active();
+#else
+	return false;
+#endif
 }
 
 #else /* HOTPLUG_CPU */
arch/s390/kernel/asm-offsets.c +6 -1
index ef279a1..2813a3c 100644
@@ -7,12 +7,15 @@
 #define ASM_OFFSETS_C
 
 #include <linux/kbuild.h>
-#include <linux/kvm_host.h>
 #include <linux/sched.h>
 #include <asm/idle.h>
 #include <asm/vdso.h>
 #include <asm/pgtable.h>
 
+#if IS_ENABLED(CONFIG_KVM)
+#include <linux/kvm_host.h>
+#endif
+
 /*
  * Make sure that the compiler is new enough. We want a compiler that
  * is known to work with the "Q" assembler constraint.

@@ -182,8 +185,10 @@ int main(void)
 	DEFINE(__LC_PGM_TDB, offsetof(struct _lowcore, pgm_tdb));
 	DEFINE(__THREAD_trap_tdb, offsetof(struct task_struct, thread.trap_tdb));
 	DEFINE(__GMAP_ASCE, offsetof(struct gmap, asce));
+#if IS_ENABLED(CONFIG_KVM)
 	DEFINE(__SIE_PROG0C, offsetof(struct kvm_s390_sie_block, prog0c));
 	DEFINE(__SIE_PROG20, offsetof(struct kvm_s390_sie_block, prog20));
+#endif /* CONFIG_KVM */
 #endif /* CONFIG_32BIT */
 	return 0;
 }
include/linux/notifier.h +27 -20

94 lines not on this page · raw message

include/linux/srcu.h +5 -1
index 9cfd962..ed9c389 100644
@@ -26,6 +26,8 @@
  *
  */
 
+#ifdef CONFIG_SRCU
+
 #ifndef _LINUX_SRCU_H
 #define _LINUX_SRCU_H
 

@@ -249,4 +251,6 @@ static inline void smp_mb__after_srcu_read_unlock(void)
 	/* __srcu_read_unlock has smp_mb() internally so nothing to do here. */
 }
 
-#endif
+#endif /* _LINUX_SRCU_H */
+
+#endif /* CONFIG_SRCU */
Signature · 1 line
1.9.1
#2 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

Quoted 3 lines · Pranith Kumar

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

Why?

author
#3 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

Quoted 6 lines · Peter Zijlstra
Quoted 3 lines · Pranith Kumar

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

Why?

This is part of the kernel tinification efforts. The first patch was posted here: https://lkml.org/lkml/2014/12/4/848. This patch enables a compile time failure instead of a link time failure.

Thanks!

Signature · 1 line
Pranith
#4 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

Quoted 11 lines · Pranith Kumar
Quoted 6 lines · Peter Zijlstra
Quoted 3 lines · Pranith Kumar

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

Why?

This is part of the kernel tinification efforts. The first patch was posted here: https://lkml.org/lkml/2014/12/4/848. This patch enables a compile time failure instead of a link time failure.

can't be arsed to click. again, why does it matter when it fails, neither stages produces a working kernel so tinification cannot be the purpose.

#5 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

Quoted 9 lines · Peter Zijlstra
Quoted 8 lines · Pranith Kumar
Quoted 6 lines · Peter Zijlstra
Quoted 3 lines · Pranith Kumar

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

Why?

Quoted 7 lines
Quoted 3 lines

This is part of the kernel tinification efforts. The first patch was posted here: https://lkml.org/lkml/2014/12/4/848. This patch enables a compile time failure instead of a link time failure.

can't be arsed to click. again, why does it matter when it fails, neither stages produces a working kernel so tinification cannot be the purpose.

In fairness, Pranith's original submission did force the failure at link time. One piece of feedback was to force the failure at compile time (can't remember from who). But given the hassles encountered with compile-time failure, it might well be getting to the point where it is time to fall back to the link-time-failure approach.

Thanx, Paul

reviewer
#6 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

In reply to Pranith Kumar
Quoted 11 lines · Pranith Kumar
Quoted 6 lines · Peter Zijlstra
Quoted 3 lines · Pranith Kumar

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

Why?

This is part of the kernel tinification efforts. The first patch was posted here: https://lkml.org/lkml/2014/12/4/848. This patch enables a compile time failure instead of a link time failure.

The punch line was:

"so the savings are about ~2000 bytes."

Which is utterly not worth the effort IMO. There have got to be more attractive targets for tinification than this.

cheers

#7 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

Quoted 13 lines · Michael Ellerman
Quoted 11 lines · Pranith Kumar
Quoted 6 lines · Peter Zijlstra
Quoted 3 lines · Pranith Kumar

Isolate the SRCU functions and data structures within CONFIG_SRCU so that there is a compile time failure if srcu is used when not enabled. This was decided to be better than waiting until link time for a failure to occur.

Why?

This is part of the kernel tinification efforts. The first patch was posted here: https://lkml.org/lkml/2014/12/4/848. This patch enables a compile time failure instead of a link time failure.

Quoted 6 lines

The punch line was:

"so the savings are about ~2000 bytes."

Which is utterly not worth the effort IMO. There have got to be more attractive targets for tinification than this.

There probably are. But if the tinification effort is to come anywhere near reaching its goals, it is going to need 2000-byte savings, especially on the small systems that are this effort's main target.

That said, Peter's suggestion of falling back to the link-time diagnostic does simplify things a bit, and might be a good approach.

Thanx, Paul

maintainer
#8 lore raw

Re: [PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

Quoted 27 lines · Paul E. McKenney
Quoted 2 lines · Michael Ellerman
Quoted 1 line · Pranith Kumar

On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra

Quoted 2 lines · <[email protected]> wrote:
Quoted 2 lines
Quoted 2 lines · Pranith Kumar
Quoted 1 line

Isolate the SRCU functions and data structures within

CONFIG_SRCU so that there

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

is a compile time failure if srcu is used when not enabled. This

was decided to

Quoted 5 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line

be better than waiting until link time for a failure to occur.

Why?

This is part of the kernel tinification efforts. The first patch

was

Quoted 1 line
Quoted 1 line

posted here: https://lkml.org/lkml/2014/12/4/848. This patch

enables a

Quoted 7 lines
Quoted 1 line

compile time failure instead of a link time failure.

The punch line was:

"so the savings are about ~2000 bytes."

Which is utterly not worth the effort IMO. There have got to be more

attractive

Quoted 1 line

targets for tinification than this.

Quoted 8 lines

There probably are. But if the tinification effort is to come anywhere near reaching its goals, it is going to need 2000-byte savings, especially
on the small systems that are this effort's main target.

That said, Peter's suggestion of falling back to the link-time diagnostic
does simplify things a bit, and might be a good approach.

Agreed on both fronts; I don't think we should add any significant complexity just to turn link errors into compile errors.

- Josh Triplett

Applied
#9 lore raw
In reply to Pranith Kumar
Quoted 7 lines · Pranith Kumar

Include rcupdate.h header to provide call_rcu() definition. This was implicitly being provided by slab.h file which include srcu.h somewhere in its include hierarchy which in-turn included rcupdate.h.

Lately, tinification effort added support to remove srcu entirely because of which we are encountering build errors like

Quoted 8 lines

lib/assoc_array.c: In function 'assoc_array_apply_edit':
lib/assoc_array.c:1426:2: error: implicit declaration of function 'call_rcu' [-Werror=implicit-function-declaration]
cc1: some warnings being treated as errors

Fix these by including rcupdate.h explicitly.

Signed-off-by Pranith Kumar <[email protected]>
Reported-by Scott Wood <[email protected]>

Applied.