[PATCH] PPC64 collect and export low-level cpu usage statistics

Stephen Rothwell sfr at canb.auug.org.au
Tue Feb 14 18:32:59 EST 2006 

Hi Manish,

Paul has asked me to have a look at this patch and to also consider what
has been done for similar work in s390.   I will compare this to s390
tomorrow, but for now here are some preliminary comments:

> Index: linux-2.6.15-rc6/arch/powerpc/kernel/asm-offsets.c
> ===================================================================
> --- linux-2.6.15-rc6.orig/arch/powerpc/kernel/asm-offsets.c	2005-12-18 16:36:54.000000000 -0800
> +++ linux-2.6.15-rc6/arch/powerpc/kernel/asm-offsets.c	2006-01-17 15:39:03.000000000 -0800
> @@ -144,6 +144,10 @@
>  	DEFINE(LPPACASRR1, offsetof(struct lppaca, saved_srr1));
>  	DEFINE(LPPACAANYINT, offsetof(struct lppaca, int_dword.any_int));
>  	DEFINE(LPPACADECRINT, offsetof(struct lppaca, int_dword.fields.decr_int));
> +	DEFINE(PACA_STARTB, offsetof(struct paca_struct, start_tb));
> +	DEFINE(PACA_CDFLAG, offsetof(struct paca_struct, cdflag));
> +	DEFINE(PACA_DELTATB, offsetof(struct paca_struct, delta_tb));

Why not PACA_START_TB and PACA_DELTA_TB?  Also, start_tb and delta_tb don't really
store time base values, but PURR values.

> Index: linux-2.6.15-rc6/arch/powerpc/kernel/entry_64.S
> ===================================================================
> --- linux-2.6.15-rc6.orig/arch/powerpc/kernel/entry_64.S	2005-12-18 16:36:54.000000000 -0800
> +++ linux-2.6.15-rc6/arch/powerpc/kernel/entry_64.S	2006-01-17 15:39:03.000000000 -0800
> @@ -520,7 +520,19 @@
>  	 * r13 is our per cpu area, only restore it if we are returning to
>  	 * userspace
>  	 */
> +
>  	beq	1f
> +BEGIN_FTR_SECTION
> +	li	r10,0
> +	stb	r10,PACA_CDFLAG(r13)

cdflag get set here but not set or used anywhere else.

> Index: linux-2.6.15-rc6/arch/powerpc/kernel/process.c
> ===================================================================
> --- linux-2.6.15-rc6.orig/arch/powerpc/kernel/process.c	2005-12-18 16:36:54.000000000 -0800
> +++ linux-2.6.15-rc6/arch/powerpc/kernel/process.c	2006-01-17 21:20:25.000000000 -0800
> @@ -243,6 +243,7 @@
>  	struct thread_struct *new_thread, *old_thread;
>  	unsigned long flags;
>  	struct task_struct *last;
> +	struct paca_struct *lpaca;

This could have been declared below (near pd)

>  
>  #ifdef CONFIG_SMP
>  	/* avoid complexity of lazy save/restore of fpu
> @@ -313,19 +314,34 @@
>  	new_thread = &new->thread;
>  	old_thread = &current->thread;
>  
> -#ifdef CONFIG_PPC64
> -	/*
> -	 * Collect processor utilization data per process
> -	 */
> -	if (firmware_has_feature(FW_FEATURE_SPLPAR)) {
> -		struct cpu_usage *cu = &__get_cpu_var(cpu_usage_array);
> -		long unsigned start_tb, current_tb;
> -		start_tb = old_thread->start_tb;
> -		cu->current_tb = current_tb = mfspr(SPRN_PURR);
> -		old_thread->accum_tb += (current_tb - start_tb);
> -		new_thread->start_tb = current_tb;
> +
> +/* Collect cpu_util utilization data per process and per processor wise */
> +	if (cpu_has_feature(CPU_FTR_PURR)) {
> +		struct cpu_usage *pd = &__get_cpu_var(cpu_usage_array);

Was there some good reason to change this variable name from cu to pd?

> +		long unsigned start_cpu_util, current_cpu_util;
> +
> +		if ( old_thread->start_cpu_util )
> +			pd->current_cpu_util = current_cpu_util = mfspr(SPRN_PURR);
> +		else
> +		   	old_thread->start_cpu_util = pd->current_cpu_util = current_cpu_util = mfspr(SPRN_PURR);

Probably better would be:
	pd->current_cpu_util = current_cpu_util = mfspr(SPRN_PURR);
	if (old_thread->start_cpu_util == 0)
		old_thread->start_cpu_util = current_cpu_util;

> +
> +		/* store delta_tb & mftb into cpu_util data array for    *
> +		 * later easy access otherwise you have to do run_on_cpu *
> +		 * which is expensive             			 */

Comment style should be:

	/* store delta_tb & mftb into cpu_util data array for
	 * later easy access otherwise you have to do run_on_cpu
	 * which is expensive
	 */

> +
> +		lpaca = get_paca();
> +		pd->collected_krntb = lpaca->delta_tb;
> +		pd->collected_timebase = mftb();
> +
> +		start_cpu_util = old_thread->start_cpu_util;
> +		old_thread->total_dp += (current_cpu_util - start_cpu_util);
> +
> +		/* collect time from entry into kernel to now and account it *
> +		 * in process kernel time 				     */

Comment style again.

> +
> +		old_thread->proc_stime += (current_cpu_util - lpaca->start_tb);
> +		new_thread->start_cpu_util = current_cpu_util;
>  	}
> -#endif
>  
>  	local_irq_save(flags);
>  	last = _switch(old_thread, new_thread);
> Index: linux-2.6.15-rc6/arch/powerpc/kernel/setup_64.c
> ===================================================================
> --- linux-2.6.15-rc6.orig/arch/powerpc/kernel/setup_64.c	2005-12-18 16:36:54.000000000 -0800
> +++ linux-2.6.15-rc6/arch/powerpc/kernel/setup_64.c	2006-02-10 11:51:28.197401840 -0800
> @@ -851,3 +851,153 @@

> +static void collect_cpu_deltas(int cpu)

> +static void post_cpu_deltas(int cpu)

Should those two be #ifdef CONFIG_HOTPLUG_CPU ?

> +		/* Initialize the global variables to zero */
> +		offline_cpu_total_tb = 0;
> +		offline_cpu_total_cpu_util = 0;
> +		offline_cpu_total_krncycles = 0;
> +		offline_cpu_total_idle = 0;

You don't need to set these to zero explicitly.

> Index: linux-2.6.15-rc6/arch/powerpc/kernel/sysfs.c
> ===================================================================
> --- linux-2.6.15-rc6.orig/arch/powerpc/kernel/sysfs.c	2005-12-18 16:36:54.000000000 -0800
> +++ linux-2.6.15-rc6/arch/powerpc/kernel/sysfs.c	2006-02-10 12:36:02.375372096 -0800
> @@ -232,8 +240,11 @@
>  	if (cur_cpu_spec->num_pmcs >= 8)
>  		sysdev_create_file(s, &attr_pmc8);
>  
> -	if (cpu_has_feature(CPU_FTR_SMT))
> +	if (cpu_has_feature(CPU_FTR_PURR)) {
>  		sysdev_create_file(s, &attr_purr);

This will mean that the "purr" file doesn't exist in some cases where it
used to (even if it was useless).  Not sure if that is a problem for any
user mode utilities.

> Index: linux-2.6.15-rc6/include/asm-powerpc/processor.h
> ===================================================================
> --- linux-2.6.15-rc6.orig/include/asm-powerpc/processor.h	2005-12-18 16:36:54.000000000 -0800
> +++ linux-2.6.15-rc6/include/asm-powerpc/processor.h	2006-01-17 21:31:17.000000000 -0800
> @@ -177,6 +177,9 @@
>  #ifdef CONFIG_PPC64
>  	unsigned long	start_tb;	/* Start purr when proc switched in */
>  	unsigned long	accum_tb;	/* Total accumilated purr for process */
> +	unsigned long   start_cpu_util;	/* Start cpu_util when proc switch in */
> +	unsigned long   total_dp ;	/* Total delta cpu_util accum for proc */
> +	unsigned long   proc_stime;	/* Was pad,Now process cpu_util stime */

total_dp and proc_stime are not used anywhere and start_tb accum_tb are no longer used.

-- 
Cheers,
Stephen Rothwell                    sfr at canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

More information about the Linuxppc64-dev mailing list