| Message ID | 1423167832-17609-2-git-send-email-riel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com wrote: > From: Rik van Riel <riel@redhat.com> > > Add the expected ctx_state as a parameter to context_tracking_user_enter > and context_tracking_user_exit, allowing the same functions to not just > track kernel <> user space switching, but also kernel <> guest transitions. > > Signed-off-by: Rik van Riel <riel@redhat.com> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > --- > include/linux/context_tracking.h | 12 ++++++------ > kernel/context_tracking.c | 10 +++++----- > 2 files changed, 11 insertions(+), 11 deletions(-) > > diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h > index 37b81bd51ec0..bd9f000fc98d 100644 > --- a/include/linux/context_tracking.h > +++ b/include/linux/context_tracking.h > @@ -10,21 +10,21 @@ > #ifdef CONFIG_CONTEXT_TRACKING > extern void context_tracking_cpu_set(int cpu); > > -extern void context_tracking_user_enter(void); > -extern void context_tracking_user_exit(void); > +extern void context_tracking_user_enter(enum ctx_state state); > +extern void context_tracking_user_exit(enum ctx_state state); > extern void __context_tracking_task_switch(struct task_struct *prev, > struct task_struct *next); > > static inline void user_enter(void) > { > if (context_tracking_is_enabled()) > - context_tracking_user_enter(); > + context_tracking_user_enter(IN_USER); > > } > static inline void user_exit(void) > { > if (context_tracking_is_enabled()) > - context_tracking_user_exit(); > + context_tracking_user_exit(IN_USER); > } > > static inline enum ctx_state exception_enter(void) > @@ -35,7 +35,7 @@ static inline enum ctx_state exception_enter(void) > return 0; > > prev_ctx = this_cpu_read(context_tracking.state); > - context_tracking_user_exit(); > + context_tracking_user_exit(prev_ctx); > > return prev_ctx; > } > @@ -44,7 +44,7 @@ static inline void exception_exit(enum ctx_state prev_ctx) > { > if (context_tracking_is_enabled()) { > if (prev_ctx == IN_USER) > - context_tracking_user_enter(); > + context_tracking_user_enter(prev_ctx); > } > } > > diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c > index 937ecdfdf258..4c010787c9ec 100644 > --- a/kernel/context_tracking.c > +++ b/kernel/context_tracking.c > @@ -47,7 +47,7 @@ void context_tracking_cpu_set(int cpu) > * to execute won't use any RCU read side critical section because this > * function sets RCU in extended quiescent state. > */ > -void context_tracking_user_enter(void) > +void context_tracking_user_enter(enum ctx_state state) > { > unsigned long flags; > > @@ -75,7 +75,7 @@ void context_tracking_user_enter(void) > WARN_ON_ONCE(!current->mm); > > local_irq_save(flags); > - if ( __this_cpu_read(context_tracking.state) != IN_USER) { > + if ( __this_cpu_read(context_tracking.state) != state) { > if (__this_cpu_read(context_tracking.active)) { > trace_user_enter(0); > /* > @@ -101,7 +101,7 @@ void context_tracking_user_enter(void) > * OTOH we can spare the calls to vtime and RCU when context_tracking.active > * is false because we know that CPU is not tickless. > */ > - __this_cpu_write(context_tracking.state, IN_USER); > + __this_cpu_write(context_tracking.state, state); > } > local_irq_restore(flags); > } > @@ -118,7 +118,7 @@ NOKPROBE_SYMBOL(context_tracking_user_enter); > * This call supports re-entrancy. This way it can be called from any exception > * handler without needing to know if we came from userspace or not. > */ > -void context_tracking_user_exit(void) > +void context_tracking_user_exit(enum ctx_state state) > { > unsigned long flags; > > @@ -129,7 +129,7 @@ void context_tracking_user_exit(void) > return; > > local_irq_save(flags); > - if (__this_cpu_read(context_tracking.state) == IN_USER) { > + if (__this_cpu_read(context_tracking.state) == state) { > if (__this_cpu_read(context_tracking.active)) { > /* > * We are going to run code that may use RCU. Inform > -- > 1.9.3 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/02/2015 00:55, Paul E. McKenney wrote: > On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com wrote: >> From: Rik van Riel <riel@redhat.com> >> >> Add the expected ctx_state as a parameter to context_tracking_user_enter >> and context_tracking_user_exit, allowing the same functions to not just >> track kernel <> user space switching, but also kernel <> guest transitions. >> >> Signed-off-by: Rik van Riel <riel@redhat.com> > > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> /me wonders: whose tree is supposed to carry these patches? Paolo >> --- >> include/linux/context_tracking.h | 12 ++++++------ >> kernel/context_tracking.c | 10 +++++----- >> 2 files changed, 11 insertions(+), 11 deletions(-) >> >> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h >> index 37b81bd51ec0..bd9f000fc98d 100644 >> --- a/include/linux/context_tracking.h >> +++ b/include/linux/context_tracking.h >> @@ -10,21 +10,21 @@ >> #ifdef CONFIG_CONTEXT_TRACKING >> extern void context_tracking_cpu_set(int cpu); >> >> -extern void context_tracking_user_enter(void); >> -extern void context_tracking_user_exit(void); >> +extern void context_tracking_user_enter(enum ctx_state state); >> +extern void context_tracking_user_exit(enum ctx_state state); >> extern void __context_tracking_task_switch(struct task_struct *prev, >> struct task_struct *next); >> >> static inline void user_enter(void) >> { >> if (context_tracking_is_enabled()) >> - context_tracking_user_enter(); >> + context_tracking_user_enter(IN_USER); >> >> } >> static inline void user_exit(void) >> { >> if (context_tracking_is_enabled()) >> - context_tracking_user_exit(); >> + context_tracking_user_exit(IN_USER); >> } >> >> static inline enum ctx_state exception_enter(void) >> @@ -35,7 +35,7 @@ static inline enum ctx_state exception_enter(void) >> return 0; >> >> prev_ctx = this_cpu_read(context_tracking.state); >> - context_tracking_user_exit(); >> + context_tracking_user_exit(prev_ctx); >> >> return prev_ctx; >> } >> @@ -44,7 +44,7 @@ static inline void exception_exit(enum ctx_state prev_ctx) >> { >> if (context_tracking_is_enabled()) { >> if (prev_ctx == IN_USER) >> - context_tracking_user_enter(); >> + context_tracking_user_enter(prev_ctx); >> } >> } >> >> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c >> index 937ecdfdf258..4c010787c9ec 100644 >> --- a/kernel/context_tracking.c >> +++ b/kernel/context_tracking.c >> @@ -47,7 +47,7 @@ void context_tracking_cpu_set(int cpu) >> * to execute won't use any RCU read side critical section because this >> * function sets RCU in extended quiescent state. >> */ >> -void context_tracking_user_enter(void) >> +void context_tracking_user_enter(enum ctx_state state) >> { >> unsigned long flags; >> >> @@ -75,7 +75,7 @@ void context_tracking_user_enter(void) >> WARN_ON_ONCE(!current->mm); >> >> local_irq_save(flags); >> - if ( __this_cpu_read(context_tracking.state) != IN_USER) { >> + if ( __this_cpu_read(context_tracking.state) != state) { >> if (__this_cpu_read(context_tracking.active)) { >> trace_user_enter(0); >> /* >> @@ -101,7 +101,7 @@ void context_tracking_user_enter(void) >> * OTOH we can spare the calls to vtime and RCU when context_tracking.active >> * is false because we know that CPU is not tickless. >> */ >> - __this_cpu_write(context_tracking.state, IN_USER); >> + __this_cpu_write(context_tracking.state, state); >> } >> local_irq_restore(flags); >> } >> @@ -118,7 +118,7 @@ NOKPROBE_SYMBOL(context_tracking_user_enter); >> * This call supports re-entrancy. This way it can be called from any exception >> * handler without needing to know if we came from userspace or not. >> */ >> -void context_tracking_user_exit(void) >> +void context_tracking_user_exit(enum ctx_state state) >> { >> unsigned long flags; >> >> @@ -129,7 +129,7 @@ void context_tracking_user_exit(void) >> return; >> >> local_irq_save(flags); >> - if (__this_cpu_read(context_tracking.state) == IN_USER) { >> + if (__this_cpu_read(context_tracking.state) == state) { >> if (__this_cpu_read(context_tracking.active)) { >> /* >> * We are going to run code that may use RCU. Inform >> -- >> 1.9.3 >> > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 06, 2015 at 11:15:57AM +0100, Paolo Bonzini wrote: > > > On 06/02/2015 00:55, Paul E. McKenney wrote: > > On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com wrote: > >> From: Rik van Riel <riel@redhat.com> > >> > >> Add the expected ctx_state as a parameter to context_tracking_user_enter > >> and context_tracking_user_exit, allowing the same functions to not just > >> track kernel <> user space switching, but also kernel <> guest transitions. > >> > >> Signed-off-by: Rik van Riel <riel@redhat.com> > > > > Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com> > > /me wonders: whose tree is supposed to carry these patches? If no one else does, I would be happy to. I would be thinking in terms of 3.21, in other words, not the merge window starting in three days, but the one after that. Thanx, Paul > Paolo > > >> --- > >> include/linux/context_tracking.h | 12 ++++++------ > >> kernel/context_tracking.c | 10 +++++----- > >> 2 files changed, 11 insertions(+), 11 deletions(-) > >> > >> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h > >> index 37b81bd51ec0..bd9f000fc98d 100644 > >> --- a/include/linux/context_tracking.h > >> +++ b/include/linux/context_tracking.h > >> @@ -10,21 +10,21 @@ > >> #ifdef CONFIG_CONTEXT_TRACKING > >> extern void context_tracking_cpu_set(int cpu); > >> > >> -extern void context_tracking_user_enter(void); > >> -extern void context_tracking_user_exit(void); > >> +extern void context_tracking_user_enter(enum ctx_state state); > >> +extern void context_tracking_user_exit(enum ctx_state state); > >> extern void __context_tracking_task_switch(struct task_struct *prev, > >> struct task_struct *next); > >> > >> static inline void user_enter(void) > >> { > >> if (context_tracking_is_enabled()) > >> - context_tracking_user_enter(); > >> + context_tracking_user_enter(IN_USER); > >> > >> } > >> static inline void user_exit(void) > >> { > >> if (context_tracking_is_enabled()) > >> - context_tracking_user_exit(); > >> + context_tracking_user_exit(IN_USER); > >> } > >> > >> static inline enum ctx_state exception_enter(void) > >> @@ -35,7 +35,7 @@ static inline enum ctx_state exception_enter(void) > >> return 0; > >> > >> prev_ctx = this_cpu_read(context_tracking.state); > >> - context_tracking_user_exit(); > >> + context_tracking_user_exit(prev_ctx); > >> > >> return prev_ctx; > >> } > >> @@ -44,7 +44,7 @@ static inline void exception_exit(enum ctx_state prev_ctx) > >> { > >> if (context_tracking_is_enabled()) { > >> if (prev_ctx == IN_USER) > >> - context_tracking_user_enter(); > >> + context_tracking_user_enter(prev_ctx); > >> } > >> } > >> > >> diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c > >> index 937ecdfdf258..4c010787c9ec 100644 > >> --- a/kernel/context_tracking.c > >> +++ b/kernel/context_tracking.c > >> @@ -47,7 +47,7 @@ void context_tracking_cpu_set(int cpu) > >> * to execute won't use any RCU read side critical section because this > >> * function sets RCU in extended quiescent state. > >> */ > >> -void context_tracking_user_enter(void) > >> +void context_tracking_user_enter(enum ctx_state state) > >> { > >> unsigned long flags; > >> > >> @@ -75,7 +75,7 @@ void context_tracking_user_enter(void) > >> WARN_ON_ONCE(!current->mm); > >> > >> local_irq_save(flags); > >> - if ( __this_cpu_read(context_tracking.state) != IN_USER) { > >> + if ( __this_cpu_read(context_tracking.state) != state) { > >> if (__this_cpu_read(context_tracking.active)) { > >> trace_user_enter(0); > >> /* > >> @@ -101,7 +101,7 @@ void context_tracking_user_enter(void) > >> * OTOH we can spare the calls to vtime and RCU when context_tracking.active > >> * is false because we know that CPU is not tickless. > >> */ > >> - __this_cpu_write(context_tracking.state, IN_USER); > >> + __this_cpu_write(context_tracking.state, state); > >> } > >> local_irq_restore(flags); > >> } > >> @@ -118,7 +118,7 @@ NOKPROBE_SYMBOL(context_tracking_user_enter); > >> * This call supports re-entrancy. This way it can be called from any exception > >> * handler without needing to know if we came from userspace or not. > >> */ > >> -void context_tracking_user_exit(void) > >> +void context_tracking_user_exit(enum ctx_state state) > >> { > >> unsigned long flags; > >> > >> @@ -129,7 +129,7 @@ void context_tracking_user_exit(void) > >> return; > >> > >> local_irq_save(flags); > >> - if (__this_cpu_read(context_tracking.state) == IN_USER) { > >> + if (__this_cpu_read(context_tracking.state) == state) { > >> if (__this_cpu_read(context_tracking.active)) { > >> /* > >> * We are going to run code that may use RCU. Inform > >> -- > >> 1.9.3 > >> > > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com wrote: > From: Rik van Riel <riel@redhat.com> > > Add the expected ctx_state as a parameter to context_tracking_user_enter > and context_tracking_user_exit, allowing the same functions to not just > track kernel <> user space switching, but also kernel <> guest transitions. > > Signed-off-by: Rik van Riel <riel@redhat.com> You should consider using guest_enter() and guest_exit() instead. These are context tracking APIs too but specifically for guest. These can be uninlined if needed. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/06/2015 12:22 PM, Frederic Weisbecker wrote: > On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com wrote: >> From: Rik van Riel <riel@redhat.com> >> >> Add the expected ctx_state as a parameter to >> context_tracking_user_enter and context_tracking_user_exit, >> allowing the same functions to not just track kernel <> user >> space switching, but also kernel <> guest transitions. >> >> Signed-off-by: Rik van Riel <riel@redhat.com> > > You should consider using guest_enter() and guest_exit() instead. > These are context tracking APIs too but specifically for guest. What do you mean instead? KVM already uses those. I just wanted to avoid duplicating the code... - -- All rights reversed -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU1QXlAAoJEM553pKExN6D+l4H/1PPmFioxed9XyL+rJZf0XSt mATl5JcWGlNybL5c4Tnld/3FX5/vYwBXmgw2Rh5a84F+TJi8B+Hu2Uwetl6C6vUF EK2+ExJ1rla4lpiO3frxPDdfdOHJFw2bR0fhEb4GHqcN2ecfSdXtL4hKwFru5h5s IJ8dzNIW52vzqzmulkcvI1y+VkgQBwUXYbkiGyy/MPf4F0WvGC9g44eXHZNPRXoT V34/nMJCpFHlZ7FVuHqGGstmPjv19VUAYNhUkrlU8DOpZMKxT58Sb1CGLfwsGqvZ y0+pRca8eT+gX0vqg9YUBfoEBNy4MnHdQEwQ0EPZwPJkcQ3Leco3/1JLHyDogCg= =3AJV -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 06, 2015 at 01:20:21PM -0500, Rik van Riel wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 02/06/2015 12:22 PM, Frederic Weisbecker wrote: > > On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com wrote: > >> From: Rik van Riel <riel@redhat.com> > >> > >> Add the expected ctx_state as a parameter to > >> context_tracking_user_enter and context_tracking_user_exit, > >> allowing the same functions to not just track kernel <> user > >> space switching, but also kernel <> guest transitions. > >> > >> Signed-off-by: Rik van Riel <riel@redhat.com> > > > > You should consider using guest_enter() and guest_exit() instead. > > These are context tracking APIs too but specifically for guest. > > What do you mean instead? KVM already uses those. > > I just wanted to avoid duplicating the code... I mean you can call rcu_user APIs directly from guest_enter/exit. You don't really need to call the context_tracking_user functions since guest_enter/guest_exit already handle the vtime accounting. > > - -- > All rights reversed > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1 > > iQEcBAEBAgAGBQJU1QXlAAoJEM553pKExN6D+l4H/1PPmFioxed9XyL+rJZf0XSt > mATl5JcWGlNybL5c4Tnld/3FX5/vYwBXmgw2Rh5a84F+TJi8B+Hu2Uwetl6C6vUF > EK2+ExJ1rla4lpiO3frxPDdfdOHJFw2bR0fhEb4GHqcN2ecfSdXtL4hKwFru5h5s > IJ8dzNIW52vzqzmulkcvI1y+VkgQBwUXYbkiGyy/MPf4F0WvGC9g44eXHZNPRXoT > V34/nMJCpFHlZ7FVuHqGGstmPjv19VUAYNhUkrlU8DOpZMKxT58Sb1CGLfwsGqvZ > y0+pRca8eT+gX0vqg9YUBfoEBNy4MnHdQEwQ0EPZwPJkcQ3Leco3/1JLHyDogCg= > =3AJV > -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/06/2015 01:23 PM, Frederic Weisbecker wrote: > On Fri, Feb 06, 2015 at 01:20:21PM -0500, Rik van Riel wrote: On > 02/06/2015 12:22 PM, Frederic Weisbecker wrote: >>>> On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com >>>> wrote: >>>>> From: Rik van Riel <riel@redhat.com> >>>>> >>>>> Add the expected ctx_state as a parameter to >>>>> context_tracking_user_enter and >>>>> context_tracking_user_exit, allowing the same functions to >>>>> not just track kernel <> user space switching, but also >>>>> kernel <> guest transitions. >>>>> >>>>> Signed-off-by: Rik van Riel <riel@redhat.com> >>>> >>>> You should consider using guest_enter() and guest_exit() >>>> instead. These are context tracking APIs too but specifically >>>> for guest. > > What do you mean instead? KVM already uses those. > > I just wanted to avoid duplicating the code... > >> I mean you can call rcu_user APIs directly from >> guest_enter/exit. You don't really need to call the >> context_tracking_user functions since guest_enter/guest_exit >> already handle the vtime accounting. I would still have to modify exception_enter and exception_exit, and with them context_tracking_user_enter and context_tracking_user_exit. We have to re-enable RCU when an exception happens. I suspect exceptions in a guest just trigger VMEXIT, and we figure later why the exception happened. However, if we were to get an exception during the code where we transition into or out of guest mode, we would still need exception_enter and exception_exit... - -- All rights reversed -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU1Q1MAAoJEM553pKExN6DB90H/iVCnfrooAB15E5Qioa3Ty+X hNMaIMX6zjYg++IFR5BhYLp9hp36o/98sv8RLTjZQix2q1ljivobmbABvx2MBNhx NiPfU9DyBkhz3gwI4oTkggb383Wrcyt+RgvclI/96AbwkhrdzxmT1nnUc0kA98xC 6NTW2+imkYX31sY/2SFmYWnJMVZOjOIep3LCVh/hrWnQARd6mdyzzFr+v6Z/vyFe 8P2rbqlkN0nf1pGYz3VF6zqF8wVmOi1mx4mo0qy80Sax7jsZv9+gGfbF1HkHJnjg FLsj/q/mcrH1GBK54a3s3P6ghpcFXfwibjhkGmrmA07XNHqLiNgKgmgPtArhU+s= =9Ln1 -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 06, 2015 at 01:51:56PM -0500, Rik van Riel wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 02/06/2015 01:23 PM, Frederic Weisbecker wrote: > > On Fri, Feb 06, 2015 at 01:20:21PM -0500, Rik van Riel wrote: On > > 02/06/2015 12:22 PM, Frederic Weisbecker wrote: > >>>> On Thu, Feb 05, 2015 at 03:23:48PM -0500, riel@redhat.com > >>>> wrote: > >>>>> From: Rik van Riel <riel@redhat.com> > >>>>> > >>>>> Add the expected ctx_state as a parameter to > >>>>> context_tracking_user_enter and > >>>>> context_tracking_user_exit, allowing the same functions to > >>>>> not just track kernel <> user space switching, but also > >>>>> kernel <> guest transitions. > >>>>> > >>>>> Signed-off-by: Rik van Riel <riel@redhat.com> > >>>> > >>>> You should consider using guest_enter() and guest_exit() > >>>> instead. These are context tracking APIs too but specifically > >>>> for guest. > > > > What do you mean instead? KVM already uses those. > > > > I just wanted to avoid duplicating the code... > > > >> I mean you can call rcu_user APIs directly from > >> guest_enter/exit. You don't really need to call the > >> context_tracking_user functions since guest_enter/guest_exit > >> already handle the vtime accounting. > > I would still have to modify exception_enter and exception_exit, > and with them context_tracking_user_enter and > context_tracking_user_exit. > > We have to re-enable RCU when an exception happens. > > I suspect exceptions in a guest just trigger VMEXIT, and we > figure later why the exception happened. However, if we were > to get an exception during the code where we transition into > or out of guest mode, we would still need exception_enter > and exception_exit... Ah that's a fair point. I didn't think about that. Ok then a real IN_GUEST mode makes sense. And context_tracking_user_enter/exit() can be reused as is indeed. Just a few things then: 1) In this case rename context_tracking_user_enter/exit() to context_tracking_enter() and context_tracking_exit(), since it's not anymore about user only but about any generic context. 2) We have the "WARN_ON_ONCE(!current->mm);" condition that is a debug check specific to userspace transitions because kernel threads aren't expected to resume to userspace. Can we also expect that we never switch to/from guest from a kernel thread? AFAICS this happens from an ioctl (thus user task) in x86 for kvm. But I only know this case. 3) You might want to update a few comments that assume we only deal with userspace transitions. 4) trace_user_enter/exit() should stay user-transitions specific. Thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/06/2015 06:15 PM, Frederic Weisbecker wrote: > Just a few things then: > > 1) In this case rename context_tracking_user_enter/exit() to > context_tracking_enter() and context_tracking_exit(), since it's > not anymore about user only but about any generic context. > > 2) We have the "WARN_ON_ONCE(!current->mm);" condition that is a > debug check specific to userspace transitions because kernel > threads aren't expected to resume to userspace. Can we also expect > that we never switch to/from guest from a kernel thread? AFAICS > this happens from an ioctl (thus user task) in x86 for kvm. But I > only know this case. > > 3) You might want to update a few comments that assume we only deal > with userspace transitions. > > 4) trace_user_enter/exit() should stay user-transitions specific. Paul, would you like me to send follow-up patches with the cleanups suggested by Frederic, or would you prefer me to send a new series with the cleanups integrated? Frederic, I will also add the cleanup you suggested for patch 4/5. - -- All rights reversed -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU1Yw+AAoJEM553pKExN6DhycH/ifPeaRaFcj/BBKaDf7BmKAJ cGMplf/vMtJA5DCFfZTmRp5Yb/9f3XBk8MU4Z+oWZFPB/msA8WkibhZtRGXpXXl9 7XgDXaXUuo++Axhb3SYHXEDhkPkhmfdjlctyr5ZUu3gHqkeWl6utv0t4anIBfo3Z NdWG8yEhgKU6OyFppf3CH0Cm46xPN+pUyAFMgK9HbSfDkR3a9rMZ32aQq8fyV15e LV4qE+/lPi7lyoLqbHmmU+pqp6iBqyQ9uFIDCRAoBBXF5jh0jxEynRubBn2D1HZJ FBi+dBWGhAjRN05tuurvkwbJtcmTpnsHyNrmzNlAeop0Upc/5Vta43zN/nu1AFA= =Z9mE -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 06, 2015 at 10:53:34PM -0500, Rik van Riel wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 02/06/2015 06:15 PM, Frederic Weisbecker wrote: > > > Just a few things then: > > > > 1) In this case rename context_tracking_user_enter/exit() to > > context_tracking_enter() and context_tracking_exit(), since it's > > not anymore about user only but about any generic context. > > > > 2) We have the "WARN_ON_ONCE(!current->mm);" condition that is a > > debug check specific to userspace transitions because kernel > > threads aren't expected to resume to userspace. Can we also expect > > that we never switch to/from guest from a kernel thread? AFAICS > > this happens from an ioctl (thus user task) in x86 for kvm. But I > > only know this case. > > > > 3) You might want to update a few comments that assume we only deal > > with userspace transitions. > > > > 4) trace_user_enter/exit() should stay user-transitions specific. > > Paul, would you like me to send follow-up patches with the cleanups > suggested by Frederic, or would you prefer me to send a new series > with the cleanups integrated? I would prefer a new series, in order to prevent possible future confusion. Thanx, Paul > Frederic, I will also add the cleanup you suggested for patch 4/5. > > - -- > All rights reversed > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v1 > > iQEcBAEBAgAGBQJU1Yw+AAoJEM553pKExN6DhycH/ifPeaRaFcj/BBKaDf7BmKAJ > cGMplf/vMtJA5DCFfZTmRp5Yb/9f3XBk8MU4Z+oWZFPB/msA8WkibhZtRGXpXXl9 > 7XgDXaXUuo++Axhb3SYHXEDhkPkhmfdjlctyr5ZUu3gHqkeWl6utv0t4anIBfo3Z > NdWG8yEhgKU6OyFppf3CH0Cm46xPN+pUyAFMgK9HbSfDkR3a9rMZ32aQq8fyV15e > LV4qE+/lPi7lyoLqbHmmU+pqp6iBqyQ9uFIDCRAoBBXF5jh0jxEynRubBn2D1HZJ > FBi+dBWGhAjRN05tuurvkwbJtcmTpnsHyNrmzNlAeop0Upc/5Vta43zN/nu1AFA= > =Z9mE > -----END PGP SIGNATURE----- > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 06, 2015 at 10:34:21PM -0800, Paul E. McKenney wrote: > On Fri, Feb 06, 2015 at 10:53:34PM -0500, Rik van Riel wrote: > > -----BEGIN PGP SIGNED MESSAGE----- > > Hash: SHA1 > > > > On 02/06/2015 06:15 PM, Frederic Weisbecker wrote: > > > > > Just a few things then: > > > > > > 1) In this case rename context_tracking_user_enter/exit() to > > > context_tracking_enter() and context_tracking_exit(), since it's > > > not anymore about user only but about any generic context. > > > > > > 2) We have the "WARN_ON_ONCE(!current->mm);" condition that is a > > > debug check specific to userspace transitions because kernel > > > threads aren't expected to resume to userspace. Can we also expect > > > that we never switch to/from guest from a kernel thread? AFAICS > > > this happens from an ioctl (thus user task) in x86 for kvm. But I > > > only know this case. > > > > > > 3) You might want to update a few comments that assume we only deal > > > with userspace transitions. > > > > > > 4) trace_user_enter/exit() should stay user-transitions specific. > > > > Paul, would you like me to send follow-up patches with the cleanups > > suggested by Frederic, or would you prefer me to send a new series > > with the cleanups integrated? > > I would prefer a new series, in order to prevent possible future > confusion. Of course, if Frederic would rather push them himself, I am fine with that. And in that case, you should ask him for his preferences, which just might differ from mine. ;-) Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Feb 06, 2015 at 11:14:53PM -0800, Paul E. McKenney wrote: > On Fri, Feb 06, 2015 at 10:34:21PM -0800, Paul E. McKenney wrote: > > On Fri, Feb 06, 2015 at 10:53:34PM -0500, Rik van Riel wrote: > > > -----BEGIN PGP SIGNED MESSAGE----- > > > Hash: SHA1 > > > > > > On 02/06/2015 06:15 PM, Frederic Weisbecker wrote: > > > > > > > Just a few things then: > > > > > > > > 1) In this case rename context_tracking_user_enter/exit() to > > > > context_tracking_enter() and context_tracking_exit(), since it's > > > > not anymore about user only but about any generic context. > > > > > > > > 2) We have the "WARN_ON_ONCE(!current->mm);" condition that is a > > > > debug check specific to userspace transitions because kernel > > > > threads aren't expected to resume to userspace. Can we also expect > > > > that we never switch to/from guest from a kernel thread? AFAICS > > > > this happens from an ioctl (thus user task) in x86 for kvm. But I > > > > only know this case. > > > > > > > > 3) You might want to update a few comments that assume we only deal > > > > with userspace transitions. > > > > > > > > 4) trace_user_enter/exit() should stay user-transitions specific. > > > > > > Paul, would you like me to send follow-up patches with the cleanups > > > suggested by Frederic, or would you prefer me to send a new series > > > with the cleanups integrated? > > > > I would prefer a new series, in order to prevent possible future > > confusion. > > Of course, if Frederic would rather push them himself, I am fine with > that. And in that case, you should ask him for his preferences, which > just might differ from mine. ;-) I prefer a new series too. Now whether you or me take the patches, I don't mind either way :-) Also I wonder how this feature is going to be enabled. Will it be enabled on full dynticks or should it be a seperate feature depending on full dynticks? Or even just CONFIG_RCU_USER_EQS? Because I'm still unclear about how and what this is used, if it involves full dynticks or only RCU extended quiescent states. Thanks. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/07/2015 03:30 AM, Frederic Weisbecker wrote: > I prefer a new series too. Now whether you or me take the patches, > I don't mind either way :-) I'll make it, no problem. > Also I wonder how this feature is going to be enabled. Will it be > enabled on full dynticks or should it be a seperate feature > depending on full dynticks? Or even just CONFIG_RCU_USER_EQS? > Because I'm still unclear about how and what this is used, if it > involves full dynticks or only RCU extended quiescent states. It involves full dynticks and CONFIG_RCU_USER_EQS. - -- All rights reversed -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU1fcsAAoJEM553pKExN6D7NEH/RZM/gqZ7CCACr4T3/Esd8GL IeYmZui+GDyrzj63/xX7ZgU+aqPbkfbEJ3ueQkabjtzIHhkurBM19XZ8CwWb42S9 5kAi51MjLrNnLPdvYCcu2q15TKSygU+V5wvxVohxHC9fi+tE/1+FOrVATky68uO4 6izXTm8EXbDLRg0tB5Mq/sRqBXGHfDw19vVQqMkQ47vzIw4oNHpLBSTv7GXHhN7u GH0QMzcDUUZ8IcyOSxLhRPOUX3XrV7C4U8ilP0ZJQ287sqtsMpQWtNZK6jmJN1tv niCrHQAOH++MuuF3x2fulpO3fSTbgwW3bGeMKh2ITHk0ODG6iIh1htmg4EFA2Bg= =AyIN -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Feb 07, 2015 at 09:30:41AM +0100, Frederic Weisbecker wrote: > On Fri, Feb 06, 2015 at 11:14:53PM -0800, Paul E. McKenney wrote: > > On Fri, Feb 06, 2015 at 10:34:21PM -0800, Paul E. McKenney wrote: > > > On Fri, Feb 06, 2015 at 10:53:34PM -0500, Rik van Riel wrote: > > > > -----BEGIN PGP SIGNED MESSAGE----- > > > > Hash: SHA1 > > > > > > > > On 02/06/2015 06:15 PM, Frederic Weisbecker wrote: > > > > > > > > > Just a few things then: > > > > > > > > > > 1) In this case rename context_tracking_user_enter/exit() to > > > > > context_tracking_enter() and context_tracking_exit(), since it's > > > > > not anymore about user only but about any generic context. > > > > > > > > > > 2) We have the "WARN_ON_ONCE(!current->mm);" condition that is a > > > > > debug check specific to userspace transitions because kernel > > > > > threads aren't expected to resume to userspace. Can we also expect > > > > > that we never switch to/from guest from a kernel thread? AFAICS > > > > > this happens from an ioctl (thus user task) in x86 for kvm. But I > > > > > only know this case. > > > > > > > > > > 3) You might want to update a few comments that assume we only deal > > > > > with userspace transitions. > > > > > > > > > > 4) trace_user_enter/exit() should stay user-transitions specific. > > > > > > > > Paul, would you like me to send follow-up patches with the cleanups > > > > suggested by Frederic, or would you prefer me to send a new series > > > > with the cleanups integrated? > > > > > > I would prefer a new series, in order to prevent possible future > > > confusion. > > > > Of course, if Frederic would rather push them himself, I am fine with > > that. And in that case, you should ask him for his preferences, which > > just might differ from mine. ;-) > > I prefer a new series too. Now whether you or me take the patches, I don't mind > either way :-) > > Also I wonder how this feature is going to be enabled. Will it be enabled on > full dynticks or should it be a seperate feature depending on full dynticks? > Or even just CONFIG_RCU_USER_EQS? Because I'm still unclear about how and what > this is used, if it involves full dynticks or only RCU extended quiescent states. Well, we certainly need it documented. And validation considerations would push for keeping the number of possible combinations low, while paranoia about added feature would push for having it be separately enabled. And if distros are going to enable this at build time, we either need -serious- validation or a way to disable at boot time. On the desired/required combinations of features, let's see... If I understand this completely, which I probably don't, we have the following considerations: o NO_HZ_FULL: Needed to get rid of the scheduling-clock interrupt during guest execution, though I am not sure whether we really have that completely wired up with this patch set. Regardless, Rik, for your use case, do you care about whether or not the guest gets interrupted by the host's scheduling-clock interrupts? (Based on discussion in this thread, my guess is "yes".) o RCU_NOCB_CPUS: Implied by NO_HZ_FULL, but only on CPUs actually enabled for NO_HZ_FULL operation, either by NO_HZ_FULL_ALL at build time or by nohz_full= at boot time. Needed to avoid interrupting the guest with host RCU callback invocation. Rik, does your use case care about guests being interrupted by RCU callback invocation? (Based on discussion in this thread, my guess is "yes".) o RCU_USER_EQS: Implied by NO_HZ_FULL, and I would have to go look to see what relation this has to nohz_full=. Needed for RCU to be able to recognize userspace-execution quiescent states on a given CPU without disturbing that CPU. Unless I am missing something subtle, you have to have this for this patch series to make sense. If my guesses are correct, the best approach would be to have this new mode of operation implied by NO_HZ_FULL. The patches seem simple enough that killer validation should be practical, which would avoid further complication of the Kconfig combinatorial space. So, are my guesses correct? Thanx, Paul -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 02/07/2015 03:06 PM, Paul E. McKenney wrote: > On Sat, Feb 07, 2015 at 09:30:41AM +0100, Frederic Weisbecker > wrote: >> On Fri, Feb 06, 2015 at 11:14:53PM -0800, Paul E. McKenney >> wrote: >>> On Fri, Feb 06, 2015 at 10:34:21PM -0800, Paul E. McKenney >>> wrote: >>>> On Fri, Feb 06, 2015 at 10:53:34PM -0500, Rik van Riel >>>> wrote: >>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 >>>>> >>>>> On 02/06/2015 06:15 PM, Frederic Weisbecker wrote: >>>>> >>>>>> Just a few things then: >>>>>> >>>>>> 1) In this case rename context_tracking_user_enter/exit() >>>>>> to context_tracking_enter() and context_tracking_exit(), >>>>>> since it's not anymore about user only but about any >>>>>> generic context. >>>>>> >>>>>> 2) We have the "WARN_ON_ONCE(!current->mm);" condition >>>>>> that is a debug check specific to userspace transitions >>>>>> because kernel threads aren't expected to resume to >>>>>> userspace. Can we also expect that we never switch >>>>>> to/from guest from a kernel thread? AFAICS this happens >>>>>> from an ioctl (thus user task) in x86 for kvm. But I only >>>>>> know this case. >>>>>> >>>>>> 3) You might want to update a few comments that assume we >>>>>> only deal with userspace transitions. >>>>>> >>>>>> 4) trace_user_enter/exit() should stay user-transitions >>>>>> specific. >>>>> >>>>> Paul, would you like me to send follow-up patches with the >>>>> cleanups suggested by Frederic, or would you prefer me to >>>>> send a new series with the cleanups integrated? >>>> >>>> I would prefer a new series, in order to prevent possible >>>> future confusion. >>> >>> Of course, if Frederic would rather push them himself, I am >>> fine with that. And in that case, you should ask him for his >>> preferences, which just might differ from mine. ;-) >> >> I prefer a new series too. Now whether you or me take the >> patches, I don't mind either way :-) >> >> Also I wonder how this feature is going to be enabled. Will it be >> enabled on full dynticks or should it be a seperate feature >> depending on full dynticks? Or even just CONFIG_RCU_USER_EQS? >> Because I'm still unclear about how and what this is used, if it >> involves full dynticks or only RCU extended quiescent states. > > Well, we certainly need it documented. And validation > considerations would push for keeping the number of possible > combinations low, while paranoia about added feature would push for > having it be separately enabled. And if distros are going to > enable this at build time, we either need -serious- validation or a > way to disable at boot time. > > On the desired/required combinations of features, let's see... > > If I understand this completely, which I probably don't, we have > the following considerations: > > o NO_HZ_FULL: Needed to get rid of the scheduling-clock interrupt > during guest execution, though I am not sure whether we really have > that completely wired up with this patch set. Regardless, Rik, for > your use case, do you care about whether or not the guest gets > interrupted by the host's scheduling-clock interrupts? (Based on > discussion in this thread, my guess is "yes".) > > o RCU_NOCB_CPUS: Implied by NO_HZ_FULL, but only on CPUs actually > enabled for NO_HZ_FULL operation, either by NO_HZ_FULL_ALL at build > time or by nohz_full= at boot time. Needed to avoid interrupting > the guest with host RCU callback invocation. Rik, does your use > case care about guests being interrupted by RCU callback > invocation? (Based on discussion in this thread, my guess is > "yes".) > > o RCU_USER_EQS: Implied by NO_HZ_FULL, and I would have to go look > to see what relation this has to nohz_full=. Needed for RCU to be > able to recognize userspace-execution quiescent states on a given > CPU without disturbing that CPU. Unless I am missing something > subtle, you have to have this for this patch series to make sense. > > If my guesses are correct, the best approach would be to have this > new mode of operation implied by NO_HZ_FULL. I agree. It makes sense to have all three, and all three are enabled in the configuration we use. I cannot think of a case where someone would significantly benefit from just one or two of the above, except maybe for debugging reasons. Having NO_HZ_FULL enable all the above, either through a boot time commandline option, or just by default, would make sense. - -- All rights reversed -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJU2NVpAAoJEM553pKExN6DxxUH/RwpZI6dRYvIQbtY2y93ax5/ Lba4QbmZ6n6AnGXrtlpwEQMSMvLawKqT9ZFSwzKeSarX6Uu4aRCdi8td34ruu9rg hfhv8hD1z15deYc0UPKUCbZrYrIi9uaG/FpioafDmPH+P4T2bFdvn7d/bKIoiaBM T1QA+LNddRxOhtayrIEDH1BnPKgXw9V8f7/mGQPmRf+oRz+Hgn6DPpEm0kTbqn+L RkhHNPemJ8bMaIwntAwzEklgnhkON9zOBe/XFof0lC+SdhtlAVkXPvX+cXiZMQZt 1rEqxK1+S9beeKVX65mLtxZg2omz46qz7SQRUGf3If2wHZXQtIRnvtlyCsDu/AI= =gj2E -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h index 37b81bd51ec0..bd9f000fc98d 100644 --- a/include/linux/context_tracking.h +++ b/include/linux/context_tracking.h @@ -10,21 +10,21 @@ #ifdef CONFIG_CONTEXT_TRACKING extern void context_tracking_cpu_set(int cpu); -extern void context_tracking_user_enter(void); -extern void context_tracking_user_exit(void); +extern void context_tracking_user_enter(enum ctx_state state); +extern void context_tracking_user_exit(enum ctx_state state); extern void __context_tracking_task_switch(struct task_struct *prev, struct task_struct *next); static inline void user_enter(void) { if (context_tracking_is_enabled()) - context_tracking_user_enter(); + context_tracking_user_enter(IN_USER); } static inline void user_exit(void) { if (context_tracking_is_enabled()) - context_tracking_user_exit(); + context_tracking_user_exit(IN_USER); } static inline enum ctx_state exception_enter(void) @@ -35,7 +35,7 @@ static inline enum ctx_state exception_enter(void) return 0; prev_ctx = this_cpu_read(context_tracking.state); - context_tracking_user_exit(); + context_tracking_user_exit(prev_ctx); return prev_ctx; } @@ -44,7 +44,7 @@ static inline void exception_exit(enum ctx_state prev_ctx) { if (context_tracking_is_enabled()) { if (prev_ctx == IN_USER) - context_tracking_user_enter(); + context_tracking_user_enter(prev_ctx); } } diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c index 937ecdfdf258..4c010787c9ec 100644 --- a/kernel/context_tracking.c +++ b/kernel/context_tracking.c @@ -47,7 +47,7 @@ void context_tracking_cpu_set(int cpu) * to execute won't use any RCU read side critical section because this * function sets RCU in extended quiescent state. */ -void context_tracking_user_enter(void) +void context_tracking_user_enter(enum ctx_state state) { unsigned long flags; @@ -75,7 +75,7 @@ void context_tracking_user_enter(void) WARN_ON_ONCE(!current->mm); local_irq_save(flags); - if ( __this_cpu_read(context_tracking.state) != IN_USER) { + if ( __this_cpu_read(context_tracking.state) != state) { if (__this_cpu_read(context_tracking.active)) { trace_user_enter(0); /* @@ -101,7 +101,7 @@ void context_tracking_user_enter(void) * OTOH we can spare the calls to vtime and RCU when context_tracking.active * is false because we know that CPU is not tickless. */ - __this_cpu_write(context_tracking.state, IN_USER); + __this_cpu_write(context_tracking.state, state); } local_irq_restore(flags); } @@ -118,7 +118,7 @@ NOKPROBE_SYMBOL(context_tracking_user_enter); * This call supports re-entrancy. This way it can be called from any exception * handler without needing to know if we came from userspace or not. */ -void context_tracking_user_exit(void) +void context_tracking_user_exit(enum ctx_state state) { unsigned long flags; @@ -129,7 +129,7 @@ void context_tracking_user_exit(void) return; local_irq_save(flags); - if (__this_cpu_read(context_tracking.state) == IN_USER) { + if (__this_cpu_read(context_tracking.state) == state) { if (__this_cpu_read(context_tracking.active)) { /* * We are going to run code that may use RCU. Inform