Bug #25211
Updated by Kefu Chai over 5 years ago
when we call PerfCounters::inc() and read_avg() at the same time, maybe the result is not what we want.
show the code here
<pre><code class="cpp">
void PerfCounters::inc(int idx, uint64_t amt)
{
if (!m_cct->_conf->perf)
return;
assert(idx > m_lower_bound);
assert(idx < m_upper_bound);
perf_counter_data_any_d& data(m_data[idx - m_lower_bound - 1]);
if (!(data.type & PERFCOUNTER_U64))
return;
if (data.type & PERFCOUNTER_LONGRUNAVG) {
data.avgcount.inc();
data.u64.add(amt);
data.avgcount2.inc();
} else {
data.u64.add(amt);
}
}
</code></pre>
<pre><code class="cpp">
/// read <sum, count> safely
pair<uint64_t,uint64_t> read_avg() const {
uint64_t sum, count;
do {
count = avgcount.read();
sum = u64.read();
} while (avgcount2.read() != count);
return make_pair(sum, count);
}
</code></pre>
ok, here is the problem, if we have two thread:
assume data.u64 = s; data.avgcount = data.avgcount2
<pre>
thread1 : read | thread2: inc(v)
|
count = (avgcount=n) (avgcount2=n) |
| avgcount avgcount2 = (n+1)
| u64 = (s+v)
sum = (s+v) |
count == avgcount2 == n |
| avgcount2 = (n+1)
\/
</pre>
we get (s+v, n), but correct result is (s+v, n+1)
same problem:
<pre>
thread1 : read_avg() thread2: inc(v)
| avgcount avgcount2 = (n+1)
|
count = (avgcount=(n+1)) (avgcount2=(n+1)) |
sum = (data.u64 = s) |
| data.u64 = (s+v)
| avgcount2 = (n+1)
count == avgcount2 == n+1 |
\/
</pre>
we get (s, n+1), but correct result is (s+v, n+1)
we should swap the position of avgcount and avgcount2:
<pre><code class="cpp">
/// read <sum, count> safely
pair<uint64_t,uint64_t> read_avg() const {
uint64_t sum, count;
do {
count = avgcount2.read(); // read avgcount2 first instead of avgcount
sum = u64.read();
} while (avgcount.read() != count);
return make_pair(sum, count);
}
</code></pre>