linux-kernel - Re: [PATCH 05/18] vtime: Update a few comments

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]

Date:	Wed, 17 Jul 2013 13:57:51 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Ingo Molnar <mingo@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>,
	Borislav Petkov <bp@...en8.de>,
	Li Zhong <zhong@...ux.vnet.ibm.com>,
	Mike Galbraith <efault@....de>,
	Kevin Hilman <khilman@...aro.org>
Subject: Re: [PATCH 05/18] vtime: Update a few comments

On Wed, 2013-07-17 at 18:44 +0200, Frederic Weisbecker wrote:
> Update a stale comment from the old vtime era and document some
> locking that might be non obvious.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Li Zhong <zhong@...ux.vnet.ibm.com>
> Cc: Mike Galbraith <efault@....de>
> Cc: Kevin Hilman <khilman@...aro.org>
> ---
>  include/linux/context_tracking.h |    8 ++------
>  kernel/sched/cputime.c           |    7 +++++++
>  2 files changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/context_tracking.h b/include/linux/context_tracking.h
> index 5984f25..12045ce 100644
> --- a/include/linux/context_tracking.h
> +++ b/include/linux/context_tracking.h
> @@ -72,8 +72,8 @@ extern void guest_exit(void);
>  static inline void guest_enter(void)
>  {
>  	/*
> -	 * This is running in ioctl context so we can avoid
> -	 * the call to vtime_account() with its unnecessary idle check.
> +	 * This is running in ioctl context so its safe
> +	 * to assume the pending cputime to flush is stime.

The above is worded funny. What about:

"This is running in ioctl context so its safe to assume that its the
stime pending cputime to flush"

I don't know. But "is stime" is what made me have to read that three
times to figure out what you meant.


>  	 */
>  	vtime_account_system(current);
>  	current->flags |= PF_VCPU;
> @@ -81,10 +81,6 @@ static inline void guest_enter(void)
>  
>  static inline void guest_exit(void)
>  {
> -	/*
> -	 * This is running in ioctl context so we can avoid
> -	 * the call to vtime_account() with its unnecessary idle check.
> -	 */

Should we copy the comment here too?

-- Steve

>  	vtime_account_system(current);
>  	current->flags &= ~PF_VCPU;
>  }
> diff --git a/kernel/sched/cputime.c b/kernel/sched/cputime.c
> index a7959e0..223a35e 100644
> --- a/kernel/sched/cputime.c
> +++ b/kernel/sched/cputime.c
> @@ -712,6 +712,13 @@ void vtime_user_enter(struct task_struct *tsk)
>  
>  void vtime_guest_enter(struct task_struct *tsk)
>  {
> +	/*
> +	 * The flags must be updated under the lock with
> +	 * the vtime_snap flush and update.
> +	 * That enforces a right ordering and update sequence
> +	 * synchronization against the reader (task_gtime())
> +	 * that can thus safely catch up with a tickless delta.
> +	 */
>  	write_seqlock(&tsk->vtime_seqlock);
>  	__vtime_account_system(tsk);
>  	current->flags |= PF_VCPU;


--
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/