On Wed, Oct 19, 2016 at 08:38:12AM +0200, Vincent Guittot wrote: > > It might make sense to have helper functions to evaluate those > > The main issue is the number of parameters used in these conditions > that makes helper function not really more readable.
Fair enough I suppose.. > > conditions, because currently there's two instances of each, once in the > > branch selection and then again (but inverted, we miss the == case fwiw) > > not sure to catch the comment about inverted and miss the == case Oh, you're right. Which proves my point on this not being entirely readable :/ Initially I thought the things were of the form: else if (ineq) { } ... if (... || !ineq || ...) Which would get you things like: a<b, !a<b := a>b, and leave a==b undefined. But if I put them along side one another like: + } else if (min_runnable_load > (runnable_load + imbalance)) { + (min_runnable_load > (this_runnable_load + imbalance)) || + } else if ((runnable_load < (min_runnable_load + imbalance)) && + (100*min_avg_load > sd->imbalance_pct*avg_load)) { + ((this_runnable_load < (min_runnable_load + imbalance)) && + (100*min_avg_load > sd->imbalance_pct*this_avg_load))) We can see this is not in fact the case. Blergh, I also cannot see a pretty way to increase readability here, because while they have the same general shape, there's this small variation with this_*. A well..