On 26 October 2016 at 16:28, Peter Zijlstra peterz@infradead.org wrote:
On Wed, Oct 26, 2016 at 02:31:01PM +0200, Vincent Guittot wrote:
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
Doesn't matter, util_avg is unsigned, this means MSB set is a valid and non-negative number, while the above will truncate it to 0.
So you really do need an alternative method of underflow. And yes, delta being signed makes it slightly more complicated.
How about something like the below, that will, if val is negative and we thus end up doing a subtraction (assumes 2s complement, which is fine, we do all over anyway), check the result isn't larger than we started out with.
#define add_positive(_ptr, _val) do { \ typeof(_ptr) ptr = (_ptr); \ typeof(_val) val = (_val); \ typeof(*ptr) res, var = READ_ONCE(*ptr); \ \ res = var + val; \ \ if (val < 0 && res > var) \ res = 0; \ \ WRITE_ONCE(*ptr, res); \ } while (0)
Indeed, looks better like that