On Fri, Oct 15, 2010 at 7:29 PM, Peter Zijlstra <pet...@infradead.org> wrote: > On Fri, 2010-10-15 at 16:54 +0200, Stephane Eranian wrote: >> The group_sched_in() function uses a transactional approach to schedule >> a group of events. In a group, either all events can be scheduled or >> none are. To schedule each event in, the function calls event_sched_in(). >> In case of error, event_sched_out() is called on each event in the group. >> >> The problem is that event_sched_out() does not completely cancel the >> effects of event_sched_in(). Furthermore event_sched_out() changes the >> state of the event as if it had run which is not true is this particular >> case. >> >> Those inconsistencies impact time tracking fields and may lead to events >> in a group not all reporting the same time_enabled and time_running values. >> This is demonstrated with the example below: >> >> $ task -eunhalted_core_cycles,baclears,baclears -e >> unhalted_core_cycles,baclears,baclears sleep 5 >> 1946101 unhalted_core_cycles (32.85% scaling, ena=829181, run=556827) >> 11423 baclears (32.85% scaling, ena=829181, run=556827) >> 7671 baclears (0.00% scaling, ena=556827, run=556827) >> >> 2250443 unhalted_core_cycles (57.83% scaling, ena=962822, run=405995) >> 11705 baclears (57.83% scaling, ena=962822, run=405995) >> 11705 baclears (57.83% scaling, ena=962822, run=405995) >> >> Notice that in the first group, the last baclears event does not >> report the same timings as its siblings. >> >> This issue comes from the fact that tstamp_stopped is updated >> by event_sched_out() as if the event had actually run. >> >> To solve the issue, we must ensure that, in case of error, there is >> no change in the event state whatsoever. That means timings must >> remain as they were when entering group_sched_in(). >> >> To do this we defer updating tstamp_running until we know the >> transaction succeeded. Therefore, we have split event_sched_in() >> in two parts separating the update to tstamp_running. >> >> Similarly, in case of error, we do not want to update tstamp_stopped. >> Therefore, we have split event_sched_out() in two parts separating >> the update to tstamp_stopped. >> >> With this patch, we now get the following output: >> >> $ task -eunhalted_core_cycles,baclears,baclears -e >> unhalted_core_cycles,baclears,baclears sleep 5 >> 2492050 unhalted_core_cycles (71.75% scaling, ena=1093330, run=308841) >> 11243 baclears (71.75% scaling, ena=1093330, run=308841) >> 11243 baclears (71.75% scaling, ena=1093330, run=308841) >> >> 1852746 unhalted_core_cycles (0.00% scaling, ena=784489, run=784489) >> 9253 baclears (0.00% scaling, ena=784489, run=784489) >> 9253 baclears (0.00% scaling, ena=784489, run=784489) >> >> Note that the uneven timing between groups is a side effect of >> the process spending most of its time sleeping, i.e., not enough >> event rotations (but that's a separate issue). >> >> Signed-off-by: Stephane Eranian <eran...@google.com> > > Yes, makes sense.. I'm a bit hesitant to slap a -stable tag on it due to > its size,.. Ingo, Paulus? > I was worried about the size too but I could not figure out another smaller way of doing this. An alternative would be to pass an extra argument to the function and call it twice.
------------------------------------------------------------------------------ Download new Adobe(R) Flash(R) Builder(TM) 4 The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly Flex(R) Builder(TM)) enable the development of rich applications that run across multiple browsers and platforms. Download your free trials today! http://p.sf.net/sfu/adobe-dev2dev _______________________________________________ perfmon2-devel mailing list perfmon2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/perfmon2-devel
