Messages in this thread | | | From | Vincent Guittot <> | Date | Wed, 26 Oct 2016 14:31:01 +0200 | Subject | Re: [PATCH 4/6 v5] sched: propagate load during synchronous attach/detach |
| |
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);
| |