Li, Aubrey wrote: [snip] >>> Your answer didn't address the problem above. When stop the cstate >>> dtrace probe, the total event number is not valid any longer, the >>> subsequent report is broken. >>> >>> And, for what reason we need to lower the side effects? When we are >>> monitoring the system, the impact was expected. >> >> On a dual socket, quad core Nehalem box: >> >> >> ~3k xcalls generated by PowerTOP. But if you turn the c-state >> report off: > >> We no longer have the ~3k xcalls, and the number of wakeups is down by >> that same amount. Is this what you're referring to ? > > I understand this and I think it's expected. my concern is, events report > calculation > depends on the C-state transition number, when you turn c-state dtrace probe > off, this number becomes obsolete for the next loop. And I don't think it's > fixed > in your updated webrev.
That dependency is gone once the idle report is stopped. g_total_events is set to zero at each iteration of the main loop, pt_display_wakeups() will then set it to the number of events traced in the last interval. From then on, we use the total # of events instead of the # of idle state transitions. Rafael >>> Actually, I really like to separate this patch into two, and commit >>> 7951 first. As for 7953, I think it's a big change for powertop, it >>> would be better to me to putback after the new version release. >> Ok, maybe we should ask someone else's opinion on this one, since we >> can't seem to agree on it :) It will take me more time to split the >> patch in two than to test and fix whatever bugs we find. > > This depends on the quality of 7953, and we already seem to have the different > opinion, :) > >>> We have several critical bug-fixes in ptop repo. How's the progress >>> to putback into ON? I think we should prioritize the putback. >> It's in progress, we just have to stop finding bugs in the fixes. >> Patches to both p and c-state reports should go into the >> current build, >> I'm not waiting for this patch. >> > > Thanks, > -Aubrey
