Message-ID: <1374083871.6458.166.camel@gandalf.local.home>
Subject: Re: [PATCH 05/18] vtime: Update a few comments
From: Steven Rostedt <rostedt@goodmis.org>
To: Frederic Weisbecker <fweisbec@gmail.com>
Cc: LKML <linux-kernel@vger.kernel.org>,
        "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
        Ingo Molnar <mingo@kernel.org>, Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>, Borislav Petkov <bp@alien8.de>,
        Li Zhong <zhong@linux.vnet.ibm.com>, Mike Galbraith <efault@gmx.de>,
        Kevin Hilman <khilman@linaro.org>
Date: Wed, 17 Jul 2013 13:57:51 -0400
In-Reply-To: <1374079471-3129-6-git-send-email-fweisbec@gmail.com>
References: <1374079471-3129-1-git-send-email-fweisbec@gmail.com>
	 <1374079471-3129-6-git-send-email-fweisbec@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2907
Lines: 85

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@gmail.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: Li Zhong <zhong@linux.vnet.ibm.com>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Kevin Hilman <khilman@linaro.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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/