Messages in this thread | | | Date | Thu, 10 Mar 2016 14:46:36 -0800 (PST) | From | Vikas Shivappa <> | Subject | Re: [PATCH 4/6] x86/mbm: Memory bandwidth monitoring event management |
| |
On Tue, 8 Mar 2016, Peter Zijlstra wrote:
> On Mon, Mar 07, 2016 at 11:27:26PM +0000, Luck, Tony wrote: >>>> + bytes = mbm_current->interval_bytes * MSEC_PER_SEC; >>>> + do_div(bytes, diff_time); >>>> + mbm_current->bandwidth = bytes; >>>> + mbm_current->interval_bytes = 0; >>>> + mbm_current->interval_start = cur_time; >>>> + } >>>>> + >>>> + return mbm_current; >>>> +} >>> >>> How does the above time tracking deal with the event not actually having >>> been scheduled the whole time? >> >> That's been the topic of a few philosophical debates ... what exactly are >> we trying to say when we report that a process has a "memory bandwidth" >> of, say, 1523 MBytes/s? We need to know both the amount of data moved >> and to pick an interval to measure and divide by. Does it make a difference >> whether the process voluntarily gave up the cpu for some part of the interval >> (by blocking on I/O)? Or did the scheduler time-slice it out to run other jobs? >> >> The above code gives the average bandwidth across the last interval >> (with a minimum interval size of 100ms to avoid craziness with rounding >> errors on exceptionally tiny intervals). Some folks apparently want to get >> a "rate" directly from perf. I think many folks will find the "bytes" counters >> more helpful (where they control the sample interval with '-I" flag to perf >> utility). > > So why didn't any of that make it into the Changelog? This is very much > different from any other perf driver, at the very least this debate > should have been mentioned and the choice defended. > > Also, why are we doing the time tracking and divisions at all? Can't we > simply report the number of bytes transferred and let userspace sort out > the rest? > > Userspace is provided the time the event was enabled, the time the event > was running and it can fairly trivially obtain walltime if it so desires > and then it can compute whatever definition of bandwidth it wants to > use.
We had discussions on removing the bw event. Discussed this with Tony and will update the patch by removing the bw events.. So this code will be removed.
thanks, Vikas
> > >
| |