[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.10.1604251359040.18257@vshiva-Udesk>
Date: Mon, 25 Apr 2016 14:12:09 -0700 (PDT)
From: Vikas Shivappa <vikas.shivappa@...el.com>
To: Peter Zijlstra <peterz@...radead.org>
cc: Vikas Shivappa <vikas.shivappa@...el.com>,
Vikas Shivappa <vikas.shivappa@...ux.intel.com>,
tony.luck@...el.com, ravi.v.shankar@...el.com,
fenghua.yu@...el.com, x86@...nel.org, linux-kernel@...r.kernel.org,
hpa@...or.com, tglx@...utronix.de, mingo@...nel.org,
h.peter.anvin@...el.com
Subject: Re: [PATCH 2/4] perf/x86/mbm: Store bytes counted for mbm during
recycle
On Mon, 25 Apr 2016, Peter Zijlstra wrote:
> On Mon, Apr 25, 2016 at 11:04:38AM -0700, Vikas Shivappa wrote:
>>> This is a 'creative' solution; why don't you do the normal thing, which
>>> is:
>>>
>>> start:
>>> prev_count = read_hw_counter();
>>>
>>> read:
>>> do {
>>> prev = prev_count;
>>> cur_val = read_hw_counter();
>>> delta = cur_val - prev;
>>> } while (local_cmpxchg(&prev_count, prev, cur_val) != prev);
>>> count += delta;
>>
>>
>> I may need to update the comment.
>>
>> rc_count stores the total bytes for RMIDs that were used for this event
>> except for the count of current RMID.
>
> Yeah, I got that, eventually.
>
>> Say an event used RMID(1) .. RMID(k) from init to read and it had RMID(k)
>> when read was called, the rc_count stores the values read from RMID1 ..
>> RMID(k-1).
>>
>> For MBM the patch is trying to do:
>> count
>> = total_bytes of RMID(1) + ... +total_bytes of RMID(k-1) + total_bytes of
>> RMID(k))
>> = rc_count + total_bytes of RMID(k).
>
> How is the regular counting scheme as outlined above not dealing with
> this properly?
>
>
By regular if you mean the current upstream code
local64_set(&event->count, atomic64_read(&rr.value));
then note that the rr.value is just the current RMIDs total_bytes, then we loose
the old values. So if RMID(1) counted 100MB , then RMID(2) counted 10MB and
there was a read(which is actually count call for cqm) call after RMID(2) then
it returns 10MB and not 100MB which is the real total_bytes..
if you mean the below -
>
> start:
> prev_count = read_hw_counter();
I am assuming this means we keep the prev_count when event is initialized. This
is done in the mbm_init which calls update_sample with first parameter set to
true..
>
> read:
> do {
> prev = prev_count;
> cur_val = read_hw_counter();
> delta = cur_val - prev;
> } while (local_cmpxchg(&prev_count, prev, cur_val) != prev);
> count += delta;
the update_sample does the work to compute the delta and add the delta to
total_bytes.. it has all the code except for the while loop.
So we miss the counter values of RMIDs which may
be used by somebody else now.. they are stored during recycling just
before we loose the RMID.
If you are tyring to count multiple RMIDs(that were used by the event) with the
while loop(?) thats something we do but its done in the xchng when we loose the
RMID.. as those counts are probably not there in those respective RMIDs anymore.
Powered by blists - more mailing lists