CPU may spend a lot of time in a guest mode while other CPUs wait for
rcu grace period to elapse. This patch series makes guest mode into
quiescent state to shorten wait time.
Gleb Natapov (2):
rcu: export rcu_note_context_switch() function
KVM: make guest mode entry to be rcu quiescent state
include/linux/kvm_host.h | 1 +
include/linux/rcutiny.h | 6 +-----
kernel/rcutiny.c | 7 +++++++
kernel/rcutree.c | 1 +
4 files changed, 10 insertions(+), 5 deletions(-)
--
1.7.2.3
KVM does not hold any references to rcu protected data when it switches
CPU into a guest mode. In fact switching to a guest mode is very similar
to exiting to userspase from rcu point of view. In addition CPU may stay
in a guest mode for quite a long time (up to one time slice). Lets treat
guest mode as quiescent state, just like we do with user-mode execution.
Signed-off-by: Gleb Natapov <[email protected]>
---
include/linux/kvm_host.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 0bc3d37..a347bce 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -593,6 +593,7 @@ static inline void kvm_guest_enter(void)
{
account_system_vtime(current);
current->flags |= PF_VCPU;
+ rcu_note_context_switch(smp_processor_id());
}
static inline void kvm_guest_exit(void)
--
1.7.2.3
Signed-off-by: Gleb Natapov <[email protected]>
---
include/linux/rcutiny.h | 6 +-----
kernel/rcutiny.c | 7 +++++++
kernel/rcutree.c | 1 +
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 30ebd7c..8e5f7cf 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
#endif /* #else #ifdef CONFIG_TINY_RCU */
-static inline void rcu_note_context_switch(int cpu)
-{
- rcu_sched_qs(cpu);
- rcu_preempt_note_context_switch();
-}
+extern void rcu_note_context_switch(int cpu);
/*
* Return the number of grace periods.
diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
index 0c343b9..3d715a4 100644
--- a/kernel/rcutiny.c
+++ b/kernel/rcutiny.c
@@ -78,6 +78,13 @@ void rcu_exit_nohz(void)
#endif /* #ifdef CONFIG_NO_HZ */
+void rcu_note_context_switch(int cpu)
+{
+ rcu_sched_qs(cpu);
+ rcu_preempt_note_context_switch();
+}
+EXPORT_SYMBOL_GPL(rcu_note_context_switch);
+
/*
* Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc().
* Also disable irqs to avoid confusion due to interrupt handlers
diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index dd4aea8..0837d63 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -124,6 +124,7 @@ void rcu_note_context_switch(int cpu)
rcu_sched_qs(cpu);
rcu_preempt_note_context_switch(cpu);
}
+EXPORT_SYMBOL_GPL(rcu_note_context_switch);
#ifdef CONFIG_NO_HZ
DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
--
1.7.2.3
On 04/28/2011 12:52 PM, Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov<[email protected]>
> @@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
>
> #endif /* #else #ifdef CONFIG_TINY_RCU */
>
> -static inline void rcu_note_context_switch(int cpu)
> -{
> - rcu_sched_qs(cpu);
> - rcu_preempt_note_context_switch();
> -}
> +extern void rcu_note_context_switch(int cpu);
>
Why are you uninlining this function? Why not export the two functions
it calls instead?
--
error compiling committee.c: too many arguments to function
On Thu, Apr 28, 2011 at 01:01:04PM +0300, Avi Kivity wrote:
> On 04/28/2011 12:52 PM, Gleb Natapov wrote:
> >Signed-off-by: Gleb Natapov<[email protected]>
> >@@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
> >
> > #endif /* #else #ifdef CONFIG_TINY_RCU */
> >
> >-static inline void rcu_note_context_switch(int cpu)
> >-{
> >- rcu_sched_qs(cpu);
> >- rcu_preempt_note_context_switch();
> >-}
> >+extern void rcu_note_context_switch(int cpu);
> >
>
> Why are you uninlining this function? Why not export the two
> functions it calls instead?
>
To keep exported interface uniformal between both rcu implementations.
I do not think that rcu_note_context_switch() is inlined for
performance reason since two functions it calls are quite big in rcutiny
implementation. Paul what do you think?
--
Gleb.
On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
Hmmm.... This is interesting. KVM being a module, we either expand
TINY_RCU's size a bit by making rcu_note_context_switch() be a real
function in rcutiny.c and adding an export, or we expand it by adding
two exports.
I would like to solve this without making TINY_RCU larger, and preferably
by making it smaller. Any ideas come to mind? (Other than making
KVM depend on CONFIG_SMP, which sounds too much like throwing out the
baby with the bathwater.)
Thanx, Paul
> Signed-off-by: Gleb Natapov <[email protected]>
> ---
> include/linux/rcutiny.h | 6 +-----
> kernel/rcutiny.c | 7 +++++++
> kernel/rcutree.c | 1 +
> 3 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index 30ebd7c..8e5f7cf 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
>
> #endif /* #else #ifdef CONFIG_TINY_RCU */
>
> -static inline void rcu_note_context_switch(int cpu)
> -{
> - rcu_sched_qs(cpu);
> - rcu_preempt_note_context_switch();
> -}
> +extern void rcu_note_context_switch(int cpu);
>
> /*
> * Return the number of grace periods.
> diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> index 0c343b9..3d715a4 100644
> --- a/kernel/rcutiny.c
> +++ b/kernel/rcutiny.c
> @@ -78,6 +78,13 @@ void rcu_exit_nohz(void)
>
> #endif /* #ifdef CONFIG_NO_HZ */
>
> +void rcu_note_context_switch(int cpu)
> +{
> + rcu_sched_qs(cpu);
> + rcu_preempt_note_context_switch();
> +}
> +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> +
> /*
> * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc().
> * Also disable irqs to avoid confusion due to interrupt handlers
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index dd4aea8..0837d63 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -124,6 +124,7 @@ void rcu_note_context_switch(int cpu)
> rcu_sched_qs(cpu);
> rcu_preempt_note_context_switch(cpu);
> }
> +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
>
> #ifdef CONFIG_NO_HZ
> DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
>
> Hmmm.... This is interesting. KVM being a module, we either expand
> TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> function in rcutiny.c and adding an export, or we expand it by adding
> two exports.
>
> I would like to solve this without making TINY_RCU larger, and preferably
> by making it smaller. Any ideas come to mind? (Other than making
> KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> baby with the bathwater.)
Nothing quite like hitting "send" to make an idea show up...
In a UP kernel, does it actually help anything to have KVM
tell RCU about executing in a guest? If not, could we have a
rcu_note_context_switch_kvm() that is a static inline empty function in
TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
Thanx, Paul
> > Signed-off-by: Gleb Natapov <[email protected]>
> > ---
> > include/linux/rcutiny.h | 6 +-----
> > kernel/rcutiny.c | 7 +++++++
> > kernel/rcutree.c | 1 +
> > 3 files changed, 9 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> > index 30ebd7c..8e5f7cf 100644
> > --- a/include/linux/rcutiny.h
> > +++ b/include/linux/rcutiny.h
> > @@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
> >
> > #endif /* #else #ifdef CONFIG_TINY_RCU */
> >
> > -static inline void rcu_note_context_switch(int cpu)
> > -{
> > - rcu_sched_qs(cpu);
> > - rcu_preempt_note_context_switch();
> > -}
> > +extern void rcu_note_context_switch(int cpu);
> >
> > /*
> > * Return the number of grace periods.
> > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> > index 0c343b9..3d715a4 100644
> > --- a/kernel/rcutiny.c
> > +++ b/kernel/rcutiny.c
> > @@ -78,6 +78,13 @@ void rcu_exit_nohz(void)
> >
> > #endif /* #ifdef CONFIG_NO_HZ */
> >
> > +void rcu_note_context_switch(int cpu)
> > +{
> > + rcu_sched_qs(cpu);
> > + rcu_preempt_note_context_switch();
> > +}
> > +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> > +
> > /*
> > * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc().
> > * Also disable irqs to avoid confusion due to interrupt handlers
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index dd4aea8..0837d63 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -124,6 +124,7 @@ void rcu_note_context_switch(int cpu)
> > rcu_sched_qs(cpu);
> > rcu_preempt_note_context_switch(cpu);
> > }
> > +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> >
> > #ifdef CONFIG_NO_HZ
> > DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
> > --
> > 1.7.2.3
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> >
> > Hmmm.... This is interesting. KVM being a module, we either expand
> > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > function in rcutiny.c and adding an export, or we expand it by adding
> > two exports.
> >
> > I would like to solve this without making TINY_RCU larger, and preferably
> > by making it smaller. Any ideas come to mind? (Other than making
> > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > baby with the bathwater.)
>
> Nothing quite like hitting "send" to make an idea show up...
>
> In a UP kernel, does it actually help anything to have KVM
> tell RCU about executing in a guest? If not, could we have a
> rcu_note_context_switch_kvm() that is a static inline empty function in
> TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
>
That will work, but does making rcu_note_context_switch() out of line
actually increase kernel size? The function is called in two places
currently, so by making it out of line we make two calling site smaller.
Will measure it next week.
> Thanx, Paul
>
> > > Signed-off-by: Gleb Natapov <[email protected]>
> > > ---
> > > include/linux/rcutiny.h | 6 +-----
> > > kernel/rcutiny.c | 7 +++++++
> > > kernel/rcutree.c | 1 +
> > > 3 files changed, 9 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> > > index 30ebd7c..8e5f7cf 100644
> > > --- a/include/linux/rcutiny.h
> > > +++ b/include/linux/rcutiny.h
> > > @@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
> > >
> > > #endif /* #else #ifdef CONFIG_TINY_RCU */
> > >
> > > -static inline void rcu_note_context_switch(int cpu)
> > > -{
> > > - rcu_sched_qs(cpu);
> > > - rcu_preempt_note_context_switch();
> > > -}
> > > +extern void rcu_note_context_switch(int cpu);
> > >
> > > /*
> > > * Return the number of grace periods.
> > > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> > > index 0c343b9..3d715a4 100644
> > > --- a/kernel/rcutiny.c
> > > +++ b/kernel/rcutiny.c
> > > @@ -78,6 +78,13 @@ void rcu_exit_nohz(void)
> > >
> > > #endif /* #ifdef CONFIG_NO_HZ */
> > >
> > > +void rcu_note_context_switch(int cpu)
> > > +{
> > > + rcu_sched_qs(cpu);
> > > + rcu_preempt_note_context_switch();
> > > +}
> > > +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> > > +
> > > /*
> > > * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc().
> > > * Also disable irqs to avoid confusion due to interrupt handlers
> > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > index dd4aea8..0837d63 100644
> > > --- a/kernel/rcutree.c
> > > +++ b/kernel/rcutree.c
> > > @@ -124,6 +124,7 @@ void rcu_note_context_switch(int cpu)
> > > rcu_sched_qs(cpu);
> > > rcu_preempt_note_context_switch(cpu);
> > > }
> > > +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> > >
> > > #ifdef CONFIG_NO_HZ
> > > DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
> > > --
> > > 1.7.2.3
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to [email protected]
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at http://www.tux.org/lkml/
--
Gleb.
On Fri, Apr 29, 2011 at 09:02:39PM +0300, Gleb Natapov wrote:
> On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> > On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> > >
> > > Hmmm.... This is interesting. KVM being a module, we either expand
> > > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > > function in rcutiny.c and adding an export, or we expand it by adding
> > > two exports.
> > >
> > > I would like to solve this without making TINY_RCU larger, and preferably
> > > by making it smaller. Any ideas come to mind? (Other than making
> > > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > > baby with the bathwater.)
> >
> > Nothing quite like hitting "send" to make an idea show up...
> >
> > In a UP kernel, does it actually help anything to have KVM
> > tell RCU about executing in a guest? If not, could we have a
> > rcu_note_context_switch_kvm() that is a static inline empty function in
> > TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
> >
> That will work, but does making rcu_note_context_switch() out of line
> actually increase kernel size? The function is called in two places
> currently, so by making it out of line we make two calling site smaller.
> Will measure it next week.
>
Why wait for so long? Here is the result:
text data bss dec hex filename
4544134 590596 2023424 7158154 6d398a vmlinux inline
4544198 590532 2023424 7158154 6d398a vmlinux.ol out of line
So in out of line version text is 64 byte bigger, but data is 64 byte
smaller. Hmm.
--
Gleb.
On Fri, Apr 29, 2011 at 09:02:39PM +0300, Gleb Natapov wrote:
> On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> > On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> > >
> > > Hmmm.... This is interesting. KVM being a module, we either expand
> > > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > > function in rcutiny.c and adding an export, or we expand it by adding
> > > two exports.
> > >
> > > I would like to solve this without making TINY_RCU larger, and preferably
> > > by making it smaller. Any ideas come to mind? (Other than making
> > > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > > baby with the bathwater.)
> >
> > Nothing quite like hitting "send" to make an idea show up...
> >
> > In a UP kernel, does it actually help anything to have KVM
> > tell RCU about executing in a guest? If not, could we have a
> > rcu_note_context_switch_kvm() that is a static inline empty function in
> > TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
> >
> That will work, but does making rcu_note_context_switch() out of line
> actually increase kernel size? The function is called in two places
> currently, so by making it out of line we make two calling site smaller.
> Will measure it next week.
One thing to keep in mind... Calling an out-of-line function from
KVM requires an export, each of which significantly increases TINY_RCU's
memory footprint.
Thanx, Paul
> > > > Signed-off-by: Gleb Natapov <[email protected]>
> > > > ---
> > > > include/linux/rcutiny.h | 6 +-----
> > > > kernel/rcutiny.c | 7 +++++++
> > > > kernel/rcutree.c | 1 +
> > > > 3 files changed, 9 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> > > > index 30ebd7c..8e5f7cf 100644
> > > > --- a/include/linux/rcutiny.h
> > > > +++ b/include/linux/rcutiny.h
> > > > @@ -93,11 +93,7 @@ static inline int rcu_needs_cpu(int cpu)
> > > >
> > > > #endif /* #else #ifdef CONFIG_TINY_RCU */
> > > >
> > > > -static inline void rcu_note_context_switch(int cpu)
> > > > -{
> > > > - rcu_sched_qs(cpu);
> > > > - rcu_preempt_note_context_switch();
> > > > -}
> > > > +extern void rcu_note_context_switch(int cpu);
> > > >
> > > > /*
> > > > * Return the number of grace periods.
> > > > diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
> > > > index 0c343b9..3d715a4 100644
> > > > --- a/kernel/rcutiny.c
> > > > +++ b/kernel/rcutiny.c
> > > > @@ -78,6 +78,13 @@ void rcu_exit_nohz(void)
> > > >
> > > > #endif /* #ifdef CONFIG_NO_HZ */
> > > >
> > > > +void rcu_note_context_switch(int cpu)
> > > > +{
> > > > + rcu_sched_qs(cpu);
> > > > + rcu_preempt_note_context_switch();
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> > > > +
> > > > /*
> > > > * Helper function for rcu_qsctr_inc() and rcu_bh_qsctr_inc().
> > > > * Also disable irqs to avoid confusion due to interrupt handlers
> > > > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > > > index dd4aea8..0837d63 100644
> > > > --- a/kernel/rcutree.c
> > > > +++ b/kernel/rcutree.c
> > > > @@ -124,6 +124,7 @@ void rcu_note_context_switch(int cpu)
> > > > rcu_sched_qs(cpu);
> > > > rcu_preempt_note_context_switch(cpu);
> > > > }
> > > > +EXPORT_SYMBOL_GPL(rcu_note_context_switch);
> > > >
> > > > #ifdef CONFIG_NO_HZ
> > > > DEFINE_PER_CPU(struct rcu_dynticks, rcu_dynticks) = {
> > > > --
> > > > 1.7.2.3
> > > >
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > > the body of a message to [email protected]
> > > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > > Please read the FAQ at http://www.tux.org/lkml/
>
> --
> Gleb.
On 04/30/2011 03:59 PM, Paul E. McKenney wrote:
> > That will work, but does making rcu_note_context_switch() out of line
> > actually increase kernel size? The function is called in two places
> > currently, so by making it out of line we make two calling site smaller.
> > Will measure it next week.
>
> One thing to keep in mind... Calling an out-of-line function from
> KVM requires an export, each of which significantly increases TINY_RCU's
> memory footprint.
I would expect that most kvm configs will actually be smp (perhaps with
an exception for embedded ppc).
A completely random idea - how about trimming exports that aren't
actually used? so if you have a minimal setup you only get the hit if
you actually use kvm.
(The trimming would need to be optional so external modules can continue
to work for those who want them)
--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.
On Sat, Apr 30, 2011 at 05:59:28AM -0700, Paul E. McKenney wrote:
> On Fri, Apr 29, 2011 at 09:02:39PM +0300, Gleb Natapov wrote:
> > On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> > > On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > > > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> > > >
> > > > Hmmm.... This is interesting. KVM being a module, we either expand
> > > > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > > > function in rcutiny.c and adding an export, or we expand it by adding
> > > > two exports.
> > > >
> > > > I would like to solve this without making TINY_RCU larger, and preferably
> > > > by making it smaller. Any ideas come to mind? (Other than making
> > > > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > > > baby with the bathwater.)
> > >
> > > Nothing quite like hitting "send" to make an idea show up...
> > >
> > > In a UP kernel, does it actually help anything to have KVM
> > > tell RCU about executing in a guest? If not, could we have a
> > > rcu_note_context_switch_kvm() that is a static inline empty function in
> > > TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
> > >
> > That will work, but does making rcu_note_context_switch() out of line
> > actually increase kernel size? The function is called in two places
> > currently, so by making it out of line we make two calling site smaller.
> > Will measure it next week.
>
> One thing to keep in mind... Calling an out-of-line function from
> KVM requires an export, each of which significantly increases TINY_RCU's
> memory footprint.
>
> Thanx, Paul
>
How significantly? As I wrote in other mail I compiled two TINY_RCU
kernel with and without the patch and I didn't see memory footprint
increase at all. May be I measure it incorrectly, but what I see is that
with out of line function + export text section becomes 64 byte bigger, but
data section becomes 64 byte smaller:
text data bss dec hex filename
4544134 590596 2023424 7158154 6d398a vmlinux inline
4544198 590532 2023424 7158154 6d398a vmlinux.ol out of line
--
Gleb.
On Mon, May 02, 2011 at 01:56:12PM +0300, Gleb Natapov wrote:
> On Sat, Apr 30, 2011 at 05:59:28AM -0700, Paul E. McKenney wrote:
> > On Fri, Apr 29, 2011 at 09:02:39PM +0300, Gleb Natapov wrote:
> > > On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> > > > On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > > > > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> > > > >
> > > > > Hmmm.... This is interesting. KVM being a module, we either expand
> > > > > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > > > > function in rcutiny.c and adding an export, or we expand it by adding
> > > > > two exports.
> > > > >
> > > > > I would like to solve this without making TINY_RCU larger, and preferably
> > > > > by making it smaller. Any ideas come to mind? (Other than making
> > > > > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > > > > baby with the bathwater.)
> > > >
> > > > Nothing quite like hitting "send" to make an idea show up...
> > > >
> > > > In a UP kernel, does it actually help anything to have KVM
> > > > tell RCU about executing in a guest? If not, could we have a
> > > > rcu_note_context_switch_kvm() that is a static inline empty function in
> > > > TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
> > > >
> > > That will work, but does making rcu_note_context_switch() out of line
> > > actually increase kernel size? The function is called in two places
> > > currently, so by making it out of line we make two calling site smaller.
> > > Will measure it next week.
> >
> > One thing to keep in mind... Calling an out-of-line function from
> > KVM requires an export, each of which significantly increases TINY_RCU's
> > memory footprint.
> >
> > Thanx, Paul
> >
> How significantly? As I wrote in other mail I compiled two TINY_RCU
> kernel with and without the patch and I didn't see memory footprint
> increase at all. May be I measure it incorrectly, but what I see is that
> with out of line function + export text section becomes 64 byte bigger, but
> data section becomes 64 byte smaller:
>
> text data bss dec hex filename
> 4544134 590596 2023424 7158154 6d398a vmlinux inline
> 4544198 590532 2023424 7158154 6d398a vmlinux.ol out of line
Did you add the exports that would be needed to allow KVM to call
the functions in the inline case?
Thanx, Paul
On Mon, May 02, 2011 at 06:36:08AM -0700, Paul E. McKenney wrote:
> On Mon, May 02, 2011 at 01:56:12PM +0300, Gleb Natapov wrote:
> > On Sat, Apr 30, 2011 at 05:59:28AM -0700, Paul E. McKenney wrote:
> > > On Fri, Apr 29, 2011 at 09:02:39PM +0300, Gleb Natapov wrote:
> > > > On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> > > > > On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > > > > > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> > > > > >
> > > > > > Hmmm.... This is interesting. KVM being a module, we either expand
> > > > > > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > > > > > function in rcutiny.c and adding an export, or we expand it by adding
> > > > > > two exports.
> > > > > >
> > > > > > I would like to solve this without making TINY_RCU larger, and preferably
> > > > > > by making it smaller. Any ideas come to mind? (Other than making
> > > > > > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > > > > > baby with the bathwater.)
> > > > >
> > > > > Nothing quite like hitting "send" to make an idea show up...
> > > > >
> > > > > In a UP kernel, does it actually help anything to have KVM
> > > > > tell RCU about executing in a guest? If not, could we have a
> > > > > rcu_note_context_switch_kvm() that is a static inline empty function in
> > > > > TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
> > > > >
> > > > That will work, but does making rcu_note_context_switch() out of line
> > > > actually increase kernel size? The function is called in two places
> > > > currently, so by making it out of line we make two calling site smaller.
> > > > Will measure it next week.
> > >
> > > One thing to keep in mind... Calling an out-of-line function from
> > > KVM requires an export, each of which significantly increases TINY_RCU's
> > > memory footprint.
> > >
> > > Thanx, Paul
> > >
> > How significantly? As I wrote in other mail I compiled two TINY_RCU
> > kernel with and without the patch and I didn't see memory footprint
> > increase at all. May be I measure it incorrectly, but what I see is that
> > with out of line function + export text section becomes 64 byte bigger, but
> > data section becomes 64 byte smaller:
> >
> > text data bss dec hex filename
> > 4544134 590596 2023424 7158154 6d398a vmlinux inline
> > 4544198 590532 2023424 7158154 6d398a vmlinux.ol out of line
>
> Did you add the exports that would be needed to allow KVM to call
> the functions in the inline case?
>
Yes, this is with and without patch applied. When patch is applied the
function is out of line and exported.
--
Gleb.
On Mon, May 02, 2011 at 05:10:03PM +0300, Gleb Natapov wrote:
> On Mon, May 02, 2011 at 06:36:08AM -0700, Paul E. McKenney wrote:
> > On Mon, May 02, 2011 at 01:56:12PM +0300, Gleb Natapov wrote:
> > > On Sat, Apr 30, 2011 at 05:59:28AM -0700, Paul E. McKenney wrote:
> > > > On Fri, Apr 29, 2011 at 09:02:39PM +0300, Gleb Natapov wrote:
> > > > > On Fri, Apr 29, 2011 at 01:39:04AM -0700, Paul E. McKenney wrote:
> > > > > > On Fri, Apr 29, 2011 at 01:36:18AM -0700, Paul E. McKenney wrote:
> > > > > > > On Thu, Apr 28, 2011 at 12:52:02PM +0300, Gleb Natapov wrote:
> > > > > > >
> > > > > > > Hmmm.... This is interesting. KVM being a module, we either expand
> > > > > > > TINY_RCU's size a bit by making rcu_note_context_switch() be a real
> > > > > > > function in rcutiny.c and adding an export, or we expand it by adding
> > > > > > > two exports.
> > > > > > >
> > > > > > > I would like to solve this without making TINY_RCU larger, and preferably
> > > > > > > by making it smaller. Any ideas come to mind? (Other than making
> > > > > > > KVM depend on CONFIG_SMP, which sounds too much like throwing out the
> > > > > > > baby with the bathwater.)
> > > > > >
> > > > > > Nothing quite like hitting "send" to make an idea show up...
> > > > > >
> > > > > > In a UP kernel, does it actually help anything to have KVM
> > > > > > tell RCU about executing in a guest? If not, could we have a
> > > > > > rcu_note_context_switch_kvm() that is a static inline empty function in
> > > > > > TINY_RCU and maps to rcu_note_context_switch() for TREE_RCU?
> > > > > >
> > > > > That will work, but does making rcu_note_context_switch() out of line
> > > > > actually increase kernel size? The function is called in two places
> > > > > currently, so by making it out of line we make two calling site smaller.
> > > > > Will measure it next week.
> > > >
> > > > One thing to keep in mind... Calling an out-of-line function from
> > > > KVM requires an export, each of which significantly increases TINY_RCU's
> > > > memory footprint.
> > > >
> > > > Thanx, Paul
> > > >
> > > How significantly? As I wrote in other mail I compiled two TINY_RCU
> > > kernel with and without the patch and I didn't see memory footprint
> > > increase at all. May be I measure it incorrectly, but what I see is that
> > > with out of line function + export text section becomes 64 byte bigger, but
> > > data section becomes 64 byte smaller:
> > >
> > > text data bss dec hex filename
> > > 4544134 590596 2023424 7158154 6d398a vmlinux inline
> > > 4544198 590532 2023424 7158154 6d398a vmlinux.ol out of line
> >
> > Did you add the exports that would be needed to allow KVM to call
> > the functions in the inline case?
> >
> Yes, this is with and without patch applied. When patch is applied the
> function is out of line and exported.
OK, here is what I am suggesting -- create a separate API for virtualization,
make it be an empty static inline function for TINY, and make it a wrapper
for TREE. This gets rid of the export in the TINY case, and takes advantage
of the single-CPU constraint in the TINY case. So this gains the benefit
of uninlining rcu_note_context_switch(), but avoids paying the cost of the
EXPORT_SYMBOL_GPL().
Then you call rcu_virt_note_context_switch() in place of
rcu_note_context_switch() from KVM.
Does this make sense?
Thanx, Paul
------------------------------------------------------------------------
include/linux/rcutiny.h | 8 ++++++++
include/linux/rcutree.h | 10 ++++++++++
kernel/rcutiny.c | 1 -
3 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 8e5f7cf..3cc60c0 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -96,6 +96,14 @@ static inline int rcu_needs_cpu(int cpu)
extern void rcu_note_context_switch(int cpu);
/*
+ * Take advantage of the fact that there is only one CPU, which
+ * allows us to ignore virtualization-based context switches.
+ */
+static inline void rcu_virt_note_context_switch(int cpu)
+{
+}
+
+/*
* Return the number of grace periods.
*/
static inline long rcu_batches_completed(void)
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 284dad1..e65d066 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -35,6 +35,16 @@ extern void rcu_note_context_switch(int cpu);
extern int rcu_needs_cpu(int cpu);
extern void rcu_cpu_stall_reset(void);
+/*
+ * Note a virtualization-based context switch. This is simply a
+ * wrapper around rcu_note_context_switch(), which allows TINY_RCU
+ * to save a few bytes.
+ */
+static inline void rcu_virt_note_context_switch(int cpu)
+{
+ rcu_note_context_switch(cpu);
+}
+
#ifdef CONFIG_TREE_PREEMPT_RCU
extern void exit_rcu(void);
diff --git a/kernel/rcutiny.c b/kernel/rcutiny.c
index 44d6479..8071010 100644
--- a/kernel/rcutiny.c
+++ b/kernel/rcutiny.c
@@ -83,7 +83,6 @@ void rcu_note_context_switch(int cpu)
rcu_sched_qs(cpu);
rcu_preempt_note_context_switch();
}
-EXPORT_SYMBOL_GPL(rcu_note_context_switch);
/*
* Helper function for rcu_sched_qs() and rcu_bh_qs().
Hi Gleb,
Thats a neat idea.
On Thu, Apr 28, 2011 at 12:52:03PM +0300, Gleb Natapov wrote:
> KVM does not hold any references to rcu protected data when it switches
> CPU into a guest mode. In fact switching to a guest mode is very similar
> to exiting to userspase from rcu point of view. In addition CPU may stay
> in a guest mode for quite a long time (up to one time slice). Lets treat
> guest mode as quiescent state, just like we do with user-mode execution.
>
> Signed-off-by: Gleb Natapov <[email protected]>
> ---
> include/linux/kvm_host.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 0bc3d37..a347bce 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -593,6 +593,7 @@ static inline void kvm_guest_enter(void)
> {
> account_system_vtime(current);
> current->flags |= PF_VCPU;
> + rcu_note_context_switch(smp_processor_id());
> }
>
> static inline void kvm_guest_exit(void)
> --
> 1.7.2.3
Please have it in x86's vcpu_enter_guest, then its more explicit
(uncertain about the context of kvm_guest_enter call in other arches).
On 05/03/2011 04:13 PM, Marcelo Tosatti wrote:
> Please have it in x86's vcpu_enter_guest, then its more explicit
> (uncertain about the context of kvm_guest_enter call in other arches).
We do need it for other archs, don't we?
--
error compiling committee.c: too many arguments to function
On Tue, May 03, 2011 at 10:13:17AM -0300, Marcelo Tosatti wrote:
> Hi Gleb,
>
> Thats a neat idea.
>
> On Thu, Apr 28, 2011 at 12:52:03PM +0300, Gleb Natapov wrote:
> > KVM does not hold any references to rcu protected data when it switches
> > CPU into a guest mode. In fact switching to a guest mode is very similar
> > to exiting to userspase from rcu point of view. In addition CPU may stay
> > in a guest mode for quite a long time (up to one time slice). Lets treat
> > guest mode as quiescent state, just like we do with user-mode execution.
> >
> > Signed-off-by: Gleb Natapov <[email protected]>
> > ---
> > include/linux/kvm_host.h | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index 0bc3d37..a347bce 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -593,6 +593,7 @@ static inline void kvm_guest_enter(void)
> > {
> > account_system_vtime(current);
> > current->flags |= PF_VCPU;
> > + rcu_note_context_switch(smp_processor_id());
> > }
> >
> > static inline void kvm_guest_exit(void)
> > --
> > 1.7.2.3
>
> Please have it in x86's vcpu_enter_guest, then its more explicit
> (uncertain about the context of kvm_guest_enter call in other arches).
>
I checked all of them and kvm_guest_enter() is always called with local
irq disabled. Paul confirmed that rcu_note_context_switch() can be
called in such context.
--
Gleb.
On Tue, May 03, 2011 at 04:21:23PM +0300, Gleb Natapov wrote:
> On Tue, May 03, 2011 at 10:13:17AM -0300, Marcelo Tosatti wrote:
> > Hi Gleb,
> >
> > Thats a neat idea.
> >
> > On Thu, Apr 28, 2011 at 12:52:03PM +0300, Gleb Natapov wrote:
> > > KVM does not hold any references to rcu protected data when it switches
> > > CPU into a guest mode. In fact switching to a guest mode is very similar
> > > to exiting to userspase from rcu point of view. In addition CPU may stay
> > > in a guest mode for quite a long time (up to one time slice). Lets treat
> > > guest mode as quiescent state, just like we do with user-mode execution.
> > >
> > > Signed-off-by: Gleb Natapov <[email protected]>
> > > ---
> > > include/linux/kvm_host.h | 1 +
> > > 1 files changed, 1 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > index 0bc3d37..a347bce 100644
> > > --- a/include/linux/kvm_host.h
> > > +++ b/include/linux/kvm_host.h
> > > @@ -593,6 +593,7 @@ static inline void kvm_guest_enter(void)
> > > {
> > > account_system_vtime(current);
> > > current->flags |= PF_VCPU;
> > > + rcu_note_context_switch(smp_processor_id());
> > > }
> > >
> > > static inline void kvm_guest_exit(void)
> > > --
> > > 1.7.2.3
> >
> > Please have it in x86's vcpu_enter_guest, then its more explicit
> > (uncertain about the context of kvm_guest_enter call in other arches).
> >
> I checked all of them and kvm_guest_enter() is always called with local
> irq disabled. Paul confirmed that rcu_note_context_switch() can be
> called in such context.
OK then. Perhaps have an assert to verify interrupts are disabled?
On Tue, May 03, 2011 at 10:29:52AM -0300, Marcelo Tosatti wrote:
> On Tue, May 03, 2011 at 04:21:23PM +0300, Gleb Natapov wrote:
> > On Tue, May 03, 2011 at 10:13:17AM -0300, Marcelo Tosatti wrote:
> > > Hi Gleb,
> > >
> > > Thats a neat idea.
> > >
> > > On Thu, Apr 28, 2011 at 12:52:03PM +0300, Gleb Natapov wrote:
> > > > KVM does not hold any references to rcu protected data when it switches
> > > > CPU into a guest mode. In fact switching to a guest mode is very similar
> > > > to exiting to userspase from rcu point of view. In addition CPU may stay
> > > > in a guest mode for quite a long time (up to one time slice). Lets treat
> > > > guest mode as quiescent state, just like we do with user-mode execution.
> > > >
> > > > Signed-off-by: Gleb Natapov <[email protected]>
> > > > ---
> > > > include/linux/kvm_host.h | 1 +
> > > > 1 files changed, 1 insertions(+), 0 deletions(-)
> > > >
> > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > > index 0bc3d37..a347bce 100644
> > > > --- a/include/linux/kvm_host.h
> > > > +++ b/include/linux/kvm_host.h
> > > > @@ -593,6 +593,7 @@ static inline void kvm_guest_enter(void)
> > > > {
> > > > account_system_vtime(current);
> > > > current->flags |= PF_VCPU;
> > > > + rcu_note_context_switch(smp_processor_id());
> > > > }
> > > >
> > > > static inline void kvm_guest_exit(void)
> > > > --
> > > > 1.7.2.3
> > >
> > > Please have it in x86's vcpu_enter_guest, then its more explicit
> > > (uncertain about the context of kvm_guest_enter call in other arches).
> > >
> > I checked all of them and kvm_guest_enter() is always called with local
> > irq disabled. Paul confirmed that rcu_note_context_switch() can be
> > called in such context.
>
> OK then. Perhaps have an assert to verify interrupts are disabled?
Yes. Can add BUG_ON(preemptible()).
--
Gleb.
On Tue, May 03, 2011 at 04:39:01PM +0300, Gleb Natapov wrote:
> > > > On Thu, Apr 28, 2011 at 12:52:03PM +0300, Gleb Natapov wrote:
> > > > > KVM does not hold any references to rcu protected data when it switches
> > > > > CPU into a guest mode. In fact switching to a guest mode is very similar
> > > > > to exiting to userspase from rcu point of view. In addition CPU may stay
> > > > > in a guest mode for quite a long time (up to one time slice). Lets treat
> > > > > guest mode as quiescent state, just like we do with user-mode execution.
> > > > >
> > > > > Signed-off-by: Gleb Natapov <[email protected]>
> > > > > ---
> > > > > include/linux/kvm_host.h | 1 +
> > > > > 1 files changed, 1 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > > > > index 0bc3d37..a347bce 100644
> > > > > --- a/include/linux/kvm_host.h
> > > > > +++ b/include/linux/kvm_host.h
> > > > > @@ -593,6 +593,7 @@ static inline void kvm_guest_enter(void)
> > > > > {
> > > > > account_system_vtime(current);
> > > > > current->flags |= PF_VCPU;
> > > > > + rcu_note_context_switch(smp_processor_id());
> > > > > }
> > > > >
> > > > > static inline void kvm_guest_exit(void)
> > > > > --
> > > > > 1.7.2.3
> > > >
> > > > Please have it in x86's vcpu_enter_guest, then its more explicit
> > > > (uncertain about the context of kvm_guest_enter call in other arches).
> > > >
> > > I checked all of them and kvm_guest_enter() is always called with local
> > > irq disabled. Paul confirmed that rcu_note_context_switch() can be
> > > called in such context.
> >
> > OK then. Perhaps have an assert to verify interrupts are disabled?
> Yes. Can add BUG_ON(preemptible()).
Also please add a comment to explain whats going on. The commit message
above seems appropriate.
On Mon, May 02, 2011 at 11:25:28PM -0700, Paul E. McKenney wrote:
> > > > How significantly? As I wrote in other mail I compiled two TINY_RCU
> > > > kernel with and without the patch and I didn't see memory footprint
> > > > increase at all. May be I measure it incorrectly, but what I see is that
> > > > with out of line function + export text section becomes 64 byte bigger, but
> > > > data section becomes 64 byte smaller:
> > > >
> > > > text data bss dec hex filename
> > > > 4544134 590596 2023424 7158154 6d398a vmlinux inline
> > > > 4544198 590532 2023424 7158154 6d398a vmlinux.ol out of line
> > >
> > > Did you add the exports that would be needed to allow KVM to call
> > > the functions in the inline case?
> > >
> > Yes, this is with and without patch applied. When patch is applied the
> > function is out of line and exported.
>
> OK, here is what I am suggesting -- create a separate API for virtualization,
> make it be an empty static inline function for TINY, and make it a wrapper
> for TREE. This gets rid of the export in the TINY case, and takes advantage
> of the single-CPU constraint in the TINY case. So this gains the benefit
> of uninlining rcu_note_context_switch(), but avoids paying the cost of the
> EXPORT_SYMBOL_GPL().
>
> Then you call rcu_virt_note_context_switch() in place of
> rcu_note_context_switch() from KVM.
>
> Does this make sense?
>
If TINY RCU has such strict code size requirement then yes. I will make
another patch based on this and resend.
--
Gleb.