On 26 October 2016 at 12:54, Peter Zijlstra peterz@infradead.org wrote:
On Mon, Oct 17, 2016 at 11:14:11AM +0200, Vincent Guittot wrote:
/*
- Signed add and clamp on underflow.
- Explicitly do a load-store to ensure the intermediate value never hits
- memory. This allows lockless observations without ever seeing the negative
- values.
- */
+#define add_positive(_ptr, _val) do { \
typeof(_ptr) ptr = (_ptr); \
typeof(_val) res, val = (_val); \
typeof(*ptr) var = READ_ONCE(*ptr); \
res = var + val; \
if (res < 0) \
res = 0; \
I think this is broken, and inconsistent with sub_positive().
I agree that the behavior is different from sub_positive which deals with unsigned value, but i was not able to come with a short name that highlight this signed/unsigned difference
The thing is, util_avg, on which you use this, is an unsigned type.
The delta that is added to util_avg, is a signed value
Checking for unsigned underflow can be done by comparing against either one of the terms.
WRITE_ONCE(*ptr, res); \
+} while (0)
add_positive(&cfs_rq->avg.util_avg, delta);