On 6 December 2012 11:27, Liviu Dudau <liviu.du...@arm.com> wrote: > On Thu, Dec 06, 2012 at 09:32:11AM +0000, Viresh Kumar wrote: >> Hi Vincent, >> >> On 6 December 2012 14:50, Vincent Guittot <vincent.guit...@linaro.org> wrote: >> > It's look like you have disabled packing small task in your v13 with >> > 4a29297 >> >> It looks like you are referring to an older version of this branch as >> the commit id's >> don't match. Can you please check that again? >> >> > In addition, I don't fully understand why you absolutely want to >> > revert 39b0e77. In the worst case, it will not fully solve what it >> > should have solved but it doesn't add any regression. >> >> I don't want to, but Liviu and Morten do :) >> @Liviu/Morten: Your comments > > Hi Vincent, > > I was just a messenger here, but this is my understanding of where things are: > > - Morten has reviewed your patch and had provided comments, mainly pointing > towards the fact that the guarantees that the code is trying to provide are > not met. > - You have replied/promised to review and update the patch (is that correct?) > - Nothing has happened since in this area (is that true?)
both are true > > To us, looking at the code (mainly the while loop in the is_buddy_busy() > function), is seem that you are trying to guarantee that the runnable_avg_sum > and the runnable_avg_period that you read from the runqueue have not been > updated before deciding if that cpu is busy. But there is no reason* why > another thread could not update the runnable_avg_sum and then get preempted > before updating the runnable_avg_period, which means you will still use the > wrong values when doing the "greater than" check in the return statement. > No, I don't want to ensure that values have not been updated before I use them but that both values are coherent > The other point is that even if your code is "broken" (i.e. you read stale > values that still get you out of the while loop) the code works but > your estimate of a CPU being busy is off by .... 10% ? In other words, > having that while loop in is_buddy_busy() does not change the behaviour of > the rest of the code. So the objection is on having the while loop and > trying to claim guarantees that are not met. Why not removing the loop? So if it doesn't change the behavior why removing it ? It make the maintenance of patches series more complex. > > Hope I'm making sense. > > * my understanding of the scheduler code is rather incomplete, so I'm > expecting to be proven wrong here. > > > Best regards, > Liviu > > >> >> > According to my test, sched-pack-small-tasks-v1-arm is enough if you >> > want to use packing small task on tc2 >> >> I didn't get this one. This is what i have included in my tree. >> >> -- >> viresh >> > > -- > ==================== > | I would like to | > | fix the world, | > | but they're not | > | giving me the | > \ source code! / > --------------- > ¯\_(ツ)_/¯ > _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
