Peter Williams wrote: > Peter Williams wrote: > >> Andrew Morton wrote: >> >>> Peter Williams wrote: >>> >>>> I don't think either of these issues warrant abandoning smpnice. >>>> The first is highly unlikely to occur on real systems and the second >>>> is just an example of the patch doing its job (maybe too >>>> officiously). I don't think users would notice either on real systems. >>>> >>>> Even if you pull it from 2.6.16 rather than upgrading it with my >>>> patch can you please leave both in -mm? >>> >>> >>> >>> >>> Yes, I have done that. I currently have: >>> >>> sched-restore-smpnice.patch >>> sched-modified-nice-support-for-smp-load-balancing.patch >>> sched-cleanup_task_activated.patch >>> sched-alter_uninterruptible_sleep_interactivity.patch >>> sched-make_task_noninteractive_use_sleep_type.patch >>> sched-dont_decrease_idle_sleep_avg.patch >>> sched-include_noninteractive_sleep_in_idle_detect.patch >>> sched-new-sched-domain-for-representing-multi-core.patch >>> sched-fix-group-power-for-allnodes_domains.patch >> >> >> >> OK. Having slept on these problems I am now of the opinion that the >> problems are caused by the use of NICE_TO_BIAS_PRIO(0) to set >> *imbalance inside the (*imbalance < SCHED_LOAD_SCALE) if statement in >> find_busiest_group(). What is happening here is that even though the >> imbalance is less than one (average) task sometimes the decision is >> made to move a task anyway but with the current version this decision >> can be subverted in two ways: 1) if the task to be moved has a nice >> value less than zero the value of *imbalance that is set will be too >> small for move_tasks() to move it; and 2) if there are a number of >> tasks with nice values greater than zero on the "busiest" more than >> one of them may be moved as the value of *imbalance that is set may be >> big enough to include more than one of these tasks. >> >> The fix for this problem is to replace NICE_TO_BIAS_PRIO(0) with the >> "average bias prio per runnable task" on "busiest". This will >> (generally) result in a larger value for *imbalance in case 1. above >> and a smaller one in case 2. and alleviate both problems. A patch to >> apply this fix is attached. >> >> Signed-off-by: Peter Williams >> >> Could you please add this patch to -mm so that it can be tested? >> >> [old patch removed] > > > Can we pull this one, please? I've mistakenly assumed that busiest was > a run queue when it's actually a sched group. :-( Here's a fixed version that takes into account the fact that busiest is a group not a run queue. This patch also includes "improvements" to the logic at the end of find_busiest_queue() to take account of the fact that the loads are weighted. Hopefully, the comments in the code explain these changes. Signed-off-by: Peter Williams Because this is a more substantial change it may be advisable to wait for comments from Ingo and Nick before including this in -mm for testing. Peter -- Peter Williams pwil3058@bigpond.net.au "Learning, n. The kind of ignorance distinguishing the studious." -- Ambrose Bierce