linux-kernel - Re: [PATCH 1/6] rcu,nohz: add context_tracking_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20150210152832.GA21917@lerouge>
Date:	Tue, 10 Feb 2015 16:28:35 +0100
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	riel@...hat.com
Cc:	paulmck@...ux.vnet.ibm.com, will.deacon@....com,
	linux-kernel@...r.kernel.org, Catalin.Marinas@....com,
	kvm@...r.kernel.org, mtosatti@...hat.com, borntraeger@...ibm.com,
	mingo@...nel.org, oleg@...hat.com, lcapitulino@...hat.com,
	pbonzini@...hat.com
Subject: Re: [PATCH 1/6] rcu,nohz: add context_tracking_user_enter/exit
 wrapper functions

On Tue, Feb 10, 2015 at 09:41:45AM -0500, riel@...hat.com wrote:
> From: Rik van Riel <riel@...hat.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@...hat.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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/