Re: [PATCH 1/2] Add scaled time to taskstats based process accounting

[Balbir Singh]

Hi, Michael,

Thanks for doing this, this is really useful.

Michael Neuling wrote:
> This adds two items to the taststats struct to account for user and
> system time based on scaling the CPU frequency and instruction issue
> rates.
> 
> Adds account_(user|system)_time_scaled callbacks which architectures
> can use to account for time using this mechanism.
> 
> Signed-off-by: Michael Neuling <mikey@xxxxxxxxxxx>
> 
> ---
> 
>  include/linux/kernel_stat.h |    2 ++
>  include/linux/sched.h       |    2 +-
>  include/linux/taskstats.h   |    6 +++++-
>  kernel/fork.c               |    2 ++
>  kernel/sched.c              |   21 +++++++++++++++++++++
>  kernel/timer.c              |    7 +++++--
>  kernel/tsacct.c             |    4 ++++
>  7 files changed, 40 insertions(+), 4 deletions(-)
> 
> Index: linux-2.6-ozlabs/include/linux/kernel_stat.h
> ===================================================================
> --- linux-2.6-ozlabs.orig/include/linux/kernel_stat.h
> +++ linux-2.6-ozlabs/include/linux/kernel_stat.h
> @@ -52,7 +52,9 @@ static inline int kstat_irqs(int irq)
>  }
> 
>  extern void account_user_time(struct task_struct *, cputime_t);
> +extern void account_user_time_scaled(struct task_struct *, cputime_t);
>  extern void account_system_time(struct task_struct *, int, cputime_t);
> +extern void account_system_time_scaled(struct task_struct *, cputime_t);
>  extern void account_steal_time(struct task_struct *, cputime_t);
> 
>  #endif /* _LINUX_KERNEL_STAT_H */
> Index: linux-2.6-ozlabs/include/linux/sched.h
> ===================================================================
> --- linux-2.6-ozlabs.orig/include/linux/sched.h
> +++ linux-2.6-ozlabs/include/linux/sched.h
> @@ -1020,7 +1020,7 @@ struct task_struct {
>  	int __user *clear_child_tid;		/* CLONE_CHILD_CLEARTID */
> 
>  	unsigned int rt_priority;
> -	cputime_t utime, stime;
> +	cputime_t utime, stime, utimescaled, stimescaled;
>  	unsigned long nvcsw, nivcsw; /* context switch counts */
>  	struct timespec start_time; 		/* monotonic time */
>  	struct timespec real_start_time;	/* boot based time */
> Index: linux-2.6-ozlabs/include/linux/taskstats.h
> ===================================================================
> --- linux-2.6-ozlabs.orig/include/linux/taskstats.h
> +++ linux-2.6-ozlabs/include/linux/taskstats.h
> @@ -31,7 +31,7 @@
>   */
> 
> 
> -#define TASKSTATS_VERSION	5
> +#define TASKSTATS_VERSION	6
>  #define TS_COMM_LEN		32	/* should be >= TASK_COMM_LEN
>  					 * in linux/sched.h */
> 
> @@ -142,6 +142,10 @@ struct taskstats {
>  	__u64	write_char;		/* bytes written */
>  	__u64	read_syscalls;		/* read syscalls */
>  	__u64	write_syscalls;		/* write syscalls */
> +
> +	/* time accounting for SMT machines */
> +	__u64	ac_utimescaled;		/* utime scaled on frequency etc */
> +	__u64	ac_stimescaled;		/* stime scaled on frequency etc */
>  	/* Extended accounting fields end */
> 

I'd also request for you to add a cpu_scaled_run_real_total for use
by delay accounting. cpu_scaled_run_real_total should be similar in
functionality to cpu_run_real_total.

>  #define TASKSTATS_HAS_IO_ACCOUNTING
> Index: linux-2.6-ozlabs/kernel/fork.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/kernel/fork.c
> +++ linux-2.6-ozlabs/kernel/fork.c
> @@ -1045,6 +1045,8 @@ static struct task_struct *copy_process(
> 
>  	p->utime = cputime_zero;
>  	p->stime = cputime_zero;
> +	p->utimescaled = cputime_zero;
> +	p->stimescaled = cputime_zero;
> 
>  #ifdef CONFIG_TASK_XACCT
>  	p->rchar = 0;		/* I/O counter: bytes read */
> Index: linux-2.6-ozlabs/kernel/sched.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/kernel/sched.c
> +++ linux-2.6-ozlabs/kernel/sched.c
> @@ -3249,6 +3249,16 @@ void account_user_time(struct task_struc
>  }
> 
>  /*
> + * Account scaled user cpu time to a process.
> + * @p: the process that the cpu time gets accounted to
> + * @cputime: the cpu time spent in user space since the last update
> + */
> +void account_user_time_scaled(struct task_struct *p, cputime_t cputime)
> +{
> +	p->utimescaled = cputime_add(p->utimescaled, cputime);
> +}
> +
> +/*
>   * Account system cpu time to a process.
>   * @p: the process that the cpu time gets accounted to
>   * @hardirq_offset: the offset to subtract from hardirq_count()
> @@ -3280,6 +3290,17 @@ void account_system_time(struct task_str
>  }
> 
>  /*
> + * Account scaled system cpu time to a process.
> + * @p: the process that the cpu time gets accounted to
> + * @hardirq_offset: the offset to subtract from hardirq_count()
> + * @cputime: the cpu time spent in kernel space since the last update
> + */
> +void account_system_time_scaled(struct task_struct *p, cputime_t cputime)
> +{
> +	p->stimescaled = cputime_add(p->stimescaled, cputime);
> +}
> +
> +/*
>   * Account for involuntary wait time.
>   * @p: the process from which the cpu time has been stolen
>   * @steal: the cpu time spent in involuntary wait
> Index: linux-2.6-ozlabs/kernel/timer.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/kernel/timer.c
> +++ linux-2.6-ozlabs/kernel/timer.c
> @@ -826,10 +826,13 @@ void update_process_times(int user_tick)
>  	int cpu = smp_processor_id();
> 
>  	/* Note: this timer irq context must be accounted for as well. */
> -	if (user_tick)
> +	if (user_tick) {
>  		account_user_time(p, jiffies_to_cputime(1));
> -	else
> +		account_user_time_scaled(p, jiffies_to_cputime(1));
> +	} else {
>  		account_system_time(p, HARDIRQ_OFFSET, jiffies_to_cputime(1));
> +		account_system_time_scaled(p, jiffies_to_cputime(1));
> +	}

I am a little confused here, scaled accounting and regular accounting
go hand in hand?


>  	run_local_timers();
>  	if (rcu_pending(cpu))
>  		rcu_check_callbacks(cpu, user_tick);
> Index: linux-2.6-ozlabs/kernel/tsacct.c
> ===================================================================
> --- linux-2.6-ozlabs.orig/kernel/tsacct.c
> +++ linux-2.6-ozlabs/kernel/tsacct.c
> @@ -62,6 +62,10 @@ void bacct_add_tsk(struct taskstats *sta
>  	rcu_read_unlock();
>  	stats->ac_utime	 = cputime_to_msecs(tsk->utime) * USEC_PER_MSEC;
>  	stats->ac_stime	 = cputime_to_msecs(tsk->stime) * USEC_PER_MSEC;
> +	stats->ac_utimescaled =
> +		cputime_to_msecs(tsk->utimescaled) * USEC_PER_MSEC;
> +	stats->ac_stimescaled =
> +		cputime_to_msecs(tsk->stimescaled) * USEC_PER_MSEC;
>  	stats->ac_minflt = tsk->min_flt;
>  	stats->ac_majflt = tsk->maj_flt;
> 


-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
-
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/