[PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

[PATCH v2] srcu: Isolate srcu sections using CONFIG_SRCU

[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.

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 <bobby.prani@gmail.com>
CC: Scott Wood <scottwood@freescale.com>
---
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(-)

diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
index 4f3cfe1..f55302f 100644
--- a/arch/powerpc/kernel/setup_64.c
+++ b/arch/powerpc/kernel/setup_64.c
@@ -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.
diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c
index 71e186d..0001daa 100644
--- a/arch/powerpc/kernel/smp.c
+++ b/arch/powerpc/kernel/smp.c
@@ -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 */
diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
index ef279a1..2813a3c 100644
--- a/arch/s390/kernel/asm-offsets.c
+++ b/arch/s390/kernel/asm-offsets.c
@@ -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;
 }
diff --git a/include/linux/notifier.h b/include/linux/notifier.h
index d14a4c3..fe4f02a 100644
--- a/include/linux/notifier.h
+++ b/include/linux/notifier.h
@@ -47,6 +47,8 @@
  * runtime initialization.
  */
 
+struct notifier_block;
+
 typedef	int (*notifier_fn_t)(struct notifier_block *nb,
 			unsigned long action, void *data);
 
@@ -70,12 +72,6 @@ struct raw_notifier_head {
 	struct notifier_block __rcu *head;
 };
 
-struct srcu_notifier_head {
-	struct mutex mutex;
-	struct srcu_struct srcu;
-	struct notifier_block __rcu *head;
-};
-
 #define ATOMIC_INIT_NOTIFIER_HEAD(name) do {	\
 		spin_lock_init(&(name)->lock);	\
 		(name)->head = NULL;		\
@@ -88,11 +84,6 @@ struct srcu_notifier_head {
 		(name)->head = NULL;		\
 	} while (0)
 
-/* srcu_notifier_heads must be initialized and cleaned up dynamically */
-extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
-#define srcu_cleanup_notifier_head(name)	\
-		cleanup_srcu_struct(&(name)->srcu);
-
 #define ATOMIC_NOTIFIER_INIT(name) {				\
 		.lock = __SPIN_LOCK_UNLOCKED(name.lock),	\
 		.head = NULL }
@@ -101,7 +92,6 @@ extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
 		.head = NULL }
 #define RAW_NOTIFIER_INIT(name)	{				\
 		.head = NULL }
-/* srcu_notifier_heads cannot be initialized statically */
 
 #define ATOMIC_NOTIFIER_HEAD(name)				\
 	struct atomic_notifier_head name =			\
@@ -121,8 +111,6 @@ extern int blocking_notifier_chain_register(struct blocking_notifier_head *nh,
 		struct notifier_block *nb);
 extern int raw_notifier_chain_register(struct raw_notifier_head *nh,
 		struct notifier_block *nb);
-extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
-		struct notifier_block *nb);
 
 extern int blocking_notifier_chain_cond_register(
 		struct blocking_notifier_head *nh,
@@ -134,8 +122,6 @@ extern int blocking_notifier_chain_unregister(struct blocking_notifier_head *nh,
 		struct notifier_block *nb);
 extern int raw_notifier_chain_unregister(struct raw_notifier_head *nh,
 		struct notifier_block *nb);
-extern int srcu_notifier_chain_unregister(struct srcu_notifier_head *nh,
-		struct notifier_block *nb);
 
 extern int atomic_notifier_call_chain(struct atomic_notifier_head *nh,
 		unsigned long val, void *v);
@@ -149,10 +135,6 @@ extern int raw_notifier_call_chain(struct raw_notifier_head *nh,
 		unsigned long val, void *v);
 extern int __raw_notifier_call_chain(struct raw_notifier_head *nh,
 	unsigned long val, void *v, int nr_to_call, int *nr_calls);
-extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
-		unsigned long val, void *v);
-extern int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
-	unsigned long val, void *v, int nr_to_call, int *nr_calls);
 
 #define NOTIFY_DONE		0x0000		/* Don't care */
 #define NOTIFY_OK		0x0001		/* Suits me */
@@ -211,5 +193,30 @@ static inline int notifier_to_errno(int ret)
 
 extern struct blocking_notifier_head reboot_notifier_list;
 
+#ifdef CONFIG_SRCU
+
+struct srcu_notifier_head {
+	struct mutex mutex;
+	struct srcu_struct srcu;
+	struct notifier_block __rcu *head;
+};
+
+/* srcu_notifier_heads must be initialized and cleaned up dynamically
+ * srcu_notifier_heads cannot be initialized statically
+ */
+extern void srcu_init_notifier_head(struct srcu_notifier_head *nh);
+#define srcu_cleanup_notifier_head(name) cleanup_srcu_struct(&(name)->srcu)
+
+extern int srcu_notifier_chain_register(struct srcu_notifier_head *nh,
+		struct notifier_block *nb);
+extern int srcu_notifier_chain_unregister(struct srcu_notifier_head *nh,
+		struct notifier_block *nb);
+extern int srcu_notifier_call_chain(struct srcu_notifier_head *nh,
+		unsigned long val, void *v);
+extern int __srcu_notifier_call_chain(struct srcu_notifier_head *nh,
+	unsigned long val, void *v, int nr_to_call, int *nr_calls);
+
+#endif /* CONFIG_SRCU */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_NOTIFIER_H */
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 9cfd962..ed9c389 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -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 */
-- 
1.9.1

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

[Peter Zijlstra]

On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
> 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?

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

[Pranith Kumar]

On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
>> 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!
-- 
Pranith

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

[Peter Zijlstra]

On Tue, Dec 30, 2014 at 01:54:07PM -0500, Pranith Kumar wrote:
> On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
> >> 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.

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

[Paul E. McKenney]

On Tue, Dec 30, 2014 at 08:07:40PM +0100, Peter Zijlstra wrote:
> On Tue, Dec 30, 2014 at 01:54:07PM -0500, Pranith Kumar wrote:
> > On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
> > >> 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.

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

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

[Michael Ellerman]

On Tue, 2014-12-30 at 13:54 -0500, Pranith Kumar wrote:
> On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
> >> 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

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

[Paul E. McKenney]

On Sun, Jan 04, 2015 at 08:35:52PM +1100, Michael Ellerman wrote:
> On Tue, 2014-12-30 at 13:54 -0500, Pranith Kumar wrote:
> > On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
> > >> 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.

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

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

[Josh Triplett]

On January 4, 2015 8:14:16 AM PST, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> wrote:
>On Sun, Jan 04, 2015 at 08:35:52PM +1100, Michael Ellerman wrote:
>> On Tue, 2014-12-30 at 13:54 -0500, Pranith Kumar wrote:
>> > On Tue, Dec 30, 2014 at 1:50 PM, Peter Zijlstra
><peterz@infradead.org> wrote:
>> > > On Tue, Dec 30, 2014 at 12:46:22AM -0500, Pranith Kumar wrote:
>> > >> 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.
>
>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