| Message ID | 1423579310-24555-2-git-send-email-riel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 10, 2015 at 09:41:45AM -0500, riel@redhat.com wrote: > From: Rik van Riel <riel@redhat.com> > > These wrapper functions allow architecture code (eg. ARM) to keep > calling context_tracking_user_enter & context_tracking_user_exit > the same way it always has, without error prone tricks like duplicate > defines of argument values in assembly code. > > Signed-off-by: Rik van Riel <riel@redhat.com> This patch alone doesn't make much sense. The changelog says it's about keeping context_tracking_user_*() functions as wrappers but fails to explain to what they wrap, why and what are the new context_tracking_enter/exit functions for. Perhaps patches 1 and 2 should be merged together into something like: context_tracking: Generalize context tracking APIs to support user and guest Do that because we'll also track guest....etc And keep the old user context tracking APIs for now to avoid painful enum parameter support in ARM assembly.... > --- > include/linux/context_tracking.h | 2 ++ > kernel/context_tracking.c | 37 +++++++++++++++++++++++++------------ > 2 files changed, 27 insertions(+), 12 deletions(-) > > diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h > index 37b81bd51ec0..03b9c733eae7 100644 > --- a/include/linux/context_tracking.h > +++ b/include/linux/context_tracking.h > @@ -10,6 +10,8 @@ > #ifdef CONFIG_CONTEXT_TRACKING > extern void context_tracking_cpu_set(int cpu); > > +extern void context_tracking_enter(void); > +extern void context_tracking_exit(void); > extern void context_tracking_user_enter(void); > extern void context_tracking_user_exit(void); > extern void __context_tracking_task_switch(struct task_struct *prev, > diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c > index 937ecdfdf258..bbdc423936e6 100644 > --- a/kernel/context_tracking.c > +++ b/kernel/context_tracking.c > @@ -39,15 +39,15 @@ void context_tracking_cpu_set(int cpu) > } > > /** > - * context_tracking_user_enter - Inform the context tracking that the CPU is going to > - * enter userspace mode. > + * context_tracking_enter - Inform the context tracking that the CPU is going > + * enter user or guest space mode. > * > * This function must be called right before we switch from the kernel > - * to userspace, when it's guaranteed the remaining kernel instructions > - * to execute won't use any RCU read side critical section because this > - * function sets RCU in extended quiescent state. > + * to user or guest space, when it's guaranteed the remaining kernel > + * instructions 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_enter(void) > { > unsigned long flags; > > @@ -105,20 +105,27 @@ void context_tracking_user_enter(void) > } > local_irq_restore(flags); > } > +NOKPROBE_SYMBOL(context_tracking_enter); > + > +void context_tracking_user_enter(void) > +{ > + context_tracking_enter(); > +} > NOKPROBE_SYMBOL(context_tracking_user_enter); > > /** > - * context_tracking_user_exit - Inform the context tracking that the CPU is > - * exiting userspace mode and entering the kernel. > + * context_tracking_exit - Inform the context tracking that the CPU is > + * exiting user or guest mode and entering the kernel. > * > - * This function must be called after we entered the kernel from userspace > - * before any use of RCU read side critical section. This potentially include > - * any high level kernel code like syscalls, exceptions, signal handling, etc... > + * This function must be called after we entered the kernel from user or > + * guest space before any use of RCU read side critical section. This > + * potentially include any high level kernel code like syscalls, exceptions, > + * signal handling, etc... > * > * 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_exit(void) Comment changes are good, thanks! > { > unsigned long flags; > > @@ -143,6 +150,12 @@ void context_tracking_user_exit(void) > } > local_irq_restore(flags); > } > +NOKPROBE_SYMBOL(context_tracking_exit); > + > +void context_tracking_user_exit(void) > +{ > + context_tracking_exit(); > +} > NOKPROBE_SYMBOL(context_tracking_user_exit); > > /** > -- > 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 02/10/2015 10:28 AM, Frederic Weisbecker wrote: > On Tue, Feb 10, 2015 at 09:41:45AM -0500, riel@redhat.com wrote: >> From: Rik van Riel <riel@redhat.com> >> >> These wrapper functions allow architecture code (eg. ARM) to keep >> calling context_tracking_user_enter & context_tracking_user_exit >> the same way it always has, without error prone tricks like duplicate >> defines of argument values in assembly code. >> >> Signed-off-by: Rik van Riel <riel@redhat.com> > > This patch alone doesn't make much sense. Agreed, my goal was to keep things super easy to review, to reduce the chance of introducing bugs. > The changelog says it's about keeping > context_tracking_user_*() functions as wrappers but fails to explain to what they > wrap, why and what are the new context_tracking_enter/exit functions for. > > Perhaps patches 1 and 2 should be merged together into something like: > > context_tracking: Generalize context tracking APIs to support user and guest > > Do that because we'll also track guest....etc And keep the old user context tracking APIs > for now to avoid painful enum parameter support in ARM assembly.... Can do... Paul, would you like me to resend the whole series, or just a merged patch that replaces patches 1 & 2? That is assuming Paul prefers having the patches merged into one :) -- 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 Tue, Feb 10, 2015 at 11:48:37AM -0500, Rik van Riel wrote: > On 02/10/2015 10:28 AM, Frederic Weisbecker wrote: > > On Tue, Feb 10, 2015 at 09:41:45AM -0500, riel@redhat.com wrote: > >> From: Rik van Riel <riel@redhat.com> > >> > >> These wrapper functions allow architecture code (eg. ARM) to keep > >> calling context_tracking_user_enter & context_tracking_user_exit > >> the same way it always has, without error prone tricks like duplicate > >> defines of argument values in assembly code. > >> > >> Signed-off-by: Rik van Riel <riel@redhat.com> > > > > This patch alone doesn't make much sense. > > Agreed, my goal was to keep things super easy to review, > to reduce the chance of introducing bugs. > > > The changelog says it's about keeping > > context_tracking_user_*() functions as wrappers but fails to explain to what they > > wrap, why and what are the new context_tracking_enter/exit functions for. > > > > Perhaps patches 1 and 2 should be merged together into something like: > > > > context_tracking: Generalize context tracking APIs to support user and guest > > > > Do that because we'll also track guest....etc And keep the old user context tracking APIs > > for now to avoid painful enum parameter support in ARM assembly.... > > Can do... > > Paul, would you like me to resend the whole series, or just > a merged patch that replaces patches 1 & 2? I prefer the whole series, as it reduces my opportunity to introduce human error when applying them. ;-) > That is assuming Paul prefers having the patches merged into > one :) I am OK with that. I will merge the next set with the first two patches merged, people have had sufficient opportunity to review. 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 Tue, Feb 10, 2015 at 09:25:26AM -0800, Paul E. McKenney wrote: > On Tue, Feb 10, 2015 at 11:48:37AM -0500, Rik van Riel wrote: > > On 02/10/2015 10:28 AM, Frederic Weisbecker wrote: > > > On Tue, Feb 10, 2015 at 09:41:45AM -0500, riel@redhat.com wrote: > > >> From: Rik van Riel <riel@redhat.com> > > >> > > >> These wrapper functions allow architecture code (eg. ARM) to keep > > >> calling context_tracking_user_enter & context_tracking_user_exit > > >> the same way it always has, without error prone tricks like duplicate > > >> defines of argument values in assembly code. > > >> > > >> Signed-off-by: Rik van Riel <riel@redhat.com> > > > > > > This patch alone doesn't make much sense. > > > > Agreed, my goal was to keep things super easy to review, > > to reduce the chance of introducing bugs. > > > > > The changelog says it's about keeping > > > context_tracking_user_*() functions as wrappers but fails to explain to what they > > > wrap, why and what are the new context_tracking_enter/exit functions for. > > > > > > Perhaps patches 1 and 2 should be merged together into something like: > > > > > > context_tracking: Generalize context tracking APIs to support user and guest > > > > > > Do that because we'll also track guest....etc And keep the old user context tracking APIs > > > for now to avoid painful enum parameter support in ARM assembly.... > > > > Can do... > > > > Paul, would you like me to resend the whole series, or just > > a merged patch that replaces patches 1 & 2? > > I prefer the whole series, as it reduces my opportunity to introduce > human error when applying them. ;-) > > > That is assuming Paul prefers having the patches merged into > > one :) > > I am OK with that. I will merge the next set with the first two patches > merged, people have had sufficient opportunity to review. BTW, I have a few patches to make on the next cycle to fix a few context tracking related things. And since it's too late to push this series for the current merge window, now I wonder it may be easier if I take these patches. Otherwise you might experience unpleasant rebase conflicts. Is that ok for you? 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
On Tue, Feb 10, 2015 at 06:36:47PM +0100, Frederic Weisbecker wrote: > On Tue, Feb 10, 2015 at 09:25:26AM -0800, Paul E. McKenney wrote: > > On Tue, Feb 10, 2015 at 11:48:37AM -0500, Rik van Riel wrote: > > > On 02/10/2015 10:28 AM, Frederic Weisbecker wrote: > > > > On Tue, Feb 10, 2015 at 09:41:45AM -0500, riel@redhat.com wrote: > > > >> From: Rik van Riel <riel@redhat.com> > > > >> > > > >> These wrapper functions allow architecture code (eg. ARM) to keep > > > >> calling context_tracking_user_enter & context_tracking_user_exit > > > >> the same way it always has, without error prone tricks like duplicate > > > >> defines of argument values in assembly code. > > > >> > > > >> Signed-off-by: Rik van Riel <riel@redhat.com> > > > > > > > > This patch alone doesn't make much sense. > > > > > > Agreed, my goal was to keep things super easy to review, > > > to reduce the chance of introducing bugs. > > > > > > > The changelog says it's about keeping > > > > context_tracking_user_*() functions as wrappers but fails to explain to what they > > > > wrap, why and what are the new context_tracking_enter/exit functions for. > > > > > > > > Perhaps patches 1 and 2 should be merged together into something like: > > > > > > > > context_tracking: Generalize context tracking APIs to support user and guest > > > > > > > > Do that because we'll also track guest....etc And keep the old user context tracking APIs > > > > for now to avoid painful enum parameter support in ARM assembly.... > > > > > > Can do... > > > > > > Paul, would you like me to resend the whole series, or just > > > a merged patch that replaces patches 1 & 2? > > > > I prefer the whole series, as it reduces my opportunity to introduce > > human error when applying them. ;-) > > > > > That is assuming Paul prefers having the patches merged into > > > one :) > > > > I am OK with that. I will merge the next set with the first two patches > > merged, people have had sufficient opportunity to review. > > BTW, I have a few patches to make on the next cycle to fix a few context tracking > related things. And since it's too late to push this series for the current merge window, > now I wonder it may be easier if I take these patches. Otherwise you might experience > unpleasant rebase conflicts. Is that ok for you? Works for me! ;-) 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
diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h index 37b81bd51ec0..03b9c733eae7 100644 --- a/include/linux/context_tracking.h +++ b/include/linux/context_tracking.h @@ -10,6 +10,8 @@ #ifdef CONFIG_CONTEXT_TRACKING extern void context_tracking_cpu_set(int cpu); +extern void context_tracking_enter(void); +extern void context_tracking_exit(void); extern void context_tracking_user_enter(void); extern void context_tracking_user_exit(void); extern void __context_tracking_task_switch(struct task_struct *prev, diff --git a/kernel/context_tracking.c b/kernel/context_tracking.c index 937ecdfdf258..bbdc423936e6 100644 --- a/kernel/context_tracking.c +++ b/kernel/context_tracking.c @@ -39,15 +39,15 @@ void context_tracking_cpu_set(int cpu) } /** - * context_tracking_user_enter - Inform the context tracking that the CPU is going to - * enter userspace mode. + * context_tracking_enter - Inform the context tracking that the CPU is going + * enter user or guest space mode. * * This function must be called right before we switch from the kernel - * to userspace, when it's guaranteed the remaining kernel instructions - * to execute won't use any RCU read side critical section because this - * function sets RCU in extended quiescent state. + * to user or guest space, when it's guaranteed the remaining kernel + * instructions 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_enter(void) { unsigned long flags; @@ -105,20 +105,27 @@ void context_tracking_user_enter(void) } local_irq_restore(flags); } +NOKPROBE_SYMBOL(context_tracking_enter); + +void context_tracking_user_enter(void) +{ + context_tracking_enter(); +} NOKPROBE_SYMBOL(context_tracking_user_enter); /** - * context_tracking_user_exit - Inform the context tracking that the CPU is - * exiting userspace mode and entering the kernel. + * context_tracking_exit - Inform the context tracking that the CPU is + * exiting user or guest mode and entering the kernel. * - * This function must be called after we entered the kernel from userspace - * before any use of RCU read side critical section. This potentially include - * any high level kernel code like syscalls, exceptions, signal handling, etc... + * This function must be called after we entered the kernel from user or + * guest space before any use of RCU read side critical section. This + * potentially include any high level kernel code like syscalls, exceptions, + * signal handling, etc... * * 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_exit(void) { unsigned long flags; @@ -143,6 +150,12 @@ void context_tracking_user_exit(void) } local_irq_restore(flags); } +NOKPROBE_SYMBOL(context_tracking_exit); + +void context_tracking_user_exit(void) +{ + context_tracking_exit(); +} NOKPROBE_SYMBOL(context_tracking_user_exit); /**