On Thu, Mar 12, 2015 at 2:04 PM, Tuukka Tikkanen idlestat@tic0.net wrote:

On Thu, 12 Mar 2015, Amit Kucheria wrote:

...

Tuukka,

While testing out the various outstanding patches, I found that the comparison mode was broken.

Specifically, the following command failed with an assertion in merged_pstates()

./idlestat --import -f changedstate -b mytrace -r comparison -p

Data files can be obtained from pastebin: mytrace = http://paste.ubuntu.com/10582474/ changedstate = http://paste.ubuntu.com/10582483/

During a quick investigation, it seemed like merged_pstates() isn't doing what it says in the comments. The following patch seems to fix it and make comparison mode work again. Can you confirm this is the intended behaviour?

Hi,

I think it is broken, too. Consider input

A: 100 200 300 B: 100 200

Then the loop will terminate after two iterations because (idx < percpu_b->max) is not true when idx == 2.

Since the number of frequencies is lowish, the simple and bulletproof solution would be to replace the loop with two loops:

for (idx = 0 ; idx < a->max ; ++idx) alloc_pstate(percpu_b, percpu_a[idx].freq); for (idx = 0 ; idx < b->max ; ++idx) alloc_pstate(percpu_a, percpu_b[idx].freq);

Alloc_pstate will refuse to add duplicates and everything should be fine. A patch implementing this change is attached.

No testing done, but it compiles and is simple, should work. *knocks wood*

Tuukka

I get identical "comparison mode" output with your patch and mine. But yours is conceptually easier to understand so I'll apply it.

Thanks.