On Thu, Jun 16, 2016 at 01:38:13PM +0800, Huang Rui wrote: > On Wed, Jun 15, 2016 at 09:13:59PM -0400, Vince Weaver wrote: > > > > 2. Unless I'm misunderstanding things, the code seems to be accumulating > > Power. (see chunk below) Power is an instantaneous measurement, it > > makes no sense to add values. If you use 5W for 1ms and 10W for > > 1ms, the average power across the 2ms interval is not 15W. > > > > You can add energy, but not power. > > > > > + delta *= cpu_pwr_sample_ratio * 1000; > > > + tdelta = new_ptsc - prev_ptsc; > > > + > > > + do_div(delta, tdelta); > > > + local64_add(delta, &event->count); > > > > You're right. Nice catch! The average power is per compute unit. We > cannot add the power simplely for each processor/package. > > So here, the average power per package should be (delta1 + delta2 + ... + > deltaN)/(tdelta_avg). > I will work out a fix. Thanks to point out. >
After considering carefully, the original method should be OK. AMD nomenclature for CMT systems: [node 0] -> [Compute Unit 0] -> [Compute Unit Core 0] -> Linux CPU 0 -> [Compute Unit Core 1] -> Linux CPU 1 -> [Compute Unit 1] -> [Compute Unit Core 0] -> Linux CPU 2 -> [Compute Unit Core 1] -> Linux CPU 3 The deltaN is power per compute unit. Current one package has two CUs. In the *same* interval, CU0's power is 10W, CU1's power is 15W. The package (CU0 + CU1) power should be 15W, right? Because the interval is the same. Q = Q1 + Q2. P = Q/t = (Q1 + Q2)/t = Q1/t + Q2/t = P1 + P2. Is that clear? Thanks, Rui