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

[Steven Rostedt]

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@xxxxxxxxx>
> Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
> Cc: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Borislav Petkov <bp@xxxxxxxxx>
> Cc: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>
> Cc: Mike Galbraith <efault@xxxxxx>
> Cc: Kevin Hilman <khilman@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/