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.