[PATCH] sched-rt: Reduce excessive task push rate by not pushing tasks with equal priority as the current task

[PATCH] sched-rt: Reduce excessive task push rate by not pushing tasks with equal priority as the current task

[Tim Chen]

Commit 3be209a8 tries to migrate task of equal priority as the running
one to other cpus to balance load and eliminate any idle cpus.  However,
for system that is fully busy and running workload of a few priorities,
we found this change to cause tasks getting pushed around without
improving cpu utilization. On a fully loaded system running a well known
OLTP benchmark, it causes 70% more run queue locking in the push task
path without improving cpu utilization and make throughput degrade by
1.5%. We observe much higher rq lock contention due to excessive lockings
of target run queues on task wakeup.

A previous patch we submitted that added a check only to 
acquire lock on rq with lower priority tasks helped, 
otherwise the regression will be 2.0%. 
Our suspicion is there are higher priority tasks that wake up and run
for a short time, and balancing these tasks too much could hurt. 

This patch reverts the change and we got 1.5% improvement to the well
known OLTP database benchmark.  If reverting commit 3be209a8 is not an option,
I would appreciate suggestions on other ways to fix this regression.
Or perhaps provide an option not to push equal priority tasks on wake up?

Thanks.

Tim

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
---
 kernel/sched/rt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
index 0e4382e..7cadc92 100644
--- a/kernel/sched/rt.c
+++ b/kernel/sched/rt.c
@@ -1334,7 +1334,7 @@ select_task_rq_rt(struct task_struct *p, int cpu, int sd_flag, int flags)
 	 */
 	if (curr && unlikely(rt_task(curr)) &&
 	    (curr->nr_cpus_allowed < 2 ||
-	     curr->prio <= p->prio)) {
+	     curr->prio < p->prio)) {
 		int target = find_lowest_rq(p);
 
 		if (target != -1 &&
@@ -1867,7 +1867,7 @@ static void task_woken_rt(struct rq *rq, struct task_struct *p)
 	    p->nr_cpus_allowed > 1 &&
 	    (dl_task(rq->curr) || rt_task(rq->curr)) &&
 	    (rq->curr->nr_cpus_allowed < 2 ||
-	     rq->curr->prio <= p->prio))
+	     rq->curr->prio < p->prio))
 		push_rt_tasks(rq);
 }
 
-- 
1.8.3.1

Re: [PATCH] sched-rt: Reduce excessive task push rate by not pushing tasks with equal priority as the current task

[Mike Galbraith]

On Thu, 2015-01-22 at 10:53 -0800, Tim Chen wrote: 
> Commit 3be209a8 tries to migrate task of equal priority as the running
> one to other cpus to balance load and eliminate any idle cpus.  However,
> for system that is fully busy and running workload of a few priorities,
> we found this change to cause tasks getting pushed around without
> improving cpu utilization. On a fully loaded system running a well known
> OLTP benchmark, it causes 70% more run queue locking in the push task
> path without improving cpu utilization and make throughput degrade by
> 1.5%. We observe much higher rq lock contention due to excessive lockings
> of target run queues on task wakeup.

Pushing tasks of equal priority is about getting rt tasks to a CPU they
can utilize NOW.  Trying to improve throughput and whatnot is all well
and good, but sacrificing the most sacred rt cow on the planet to
improve some benchmark number is a very bad idea :)

-Mike

Re: [PATCH] sched-rt: Reduce excessive task push rate by not pushing tasks with equal priority as the current task

[Steven Rostedt]

On Fri, 23 Jan 2015 05:29:30 +0100
Mike Galbraith <umgwanakikbuti@gmail.com> wrote:

> On Thu, 2015-01-22 at 10:53 -0800, Tim Chen wrote: 
> > Commit 3be209a8 tries to migrate task of equal priority as the
> > running one to other cpus to balance load and eliminate any idle
> > cpus.  However, for system that is fully busy and running workload
> > of a few priorities, we found this change to cause tasks getting
> > pushed around without improving cpu utilization. On a fully loaded
> > system running a well known OLTP benchmark, it causes 70% more run
> > queue locking in the push task path without improving cpu
> > utilization and make throughput degrade by 1.5%. We observe much
> > higher rq lock contention due to excessive lockings of target run
> > queues on task wakeup.
> 
> Pushing tasks of equal priority is about getting rt tasks to a CPU
> they can utilize NOW.  Trying to improve throughput and whatnot is
> all well and good, but sacrificing the most sacred rt cow on the
> planet to improve some benchmark number is a very bad idea :)
> 

What Mike said.

With RT tasks the #1 importance is running as soon as they should run.
Throughput is #2. We never sacrifice #1 to improve #2.

Non RT tasks have much better throughput algorithms than RT tasks. But
they are much less likely to react to an event consistently as an RT
task will.

Never give a task an RT priority if your main objective is to have it
"run faster". If you do, you have no clue about what RT is used for.

-- Steve