linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Meyer, Mike" <Mike.Meyer@Teradata.com>
To: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Cc: "mingo@redhat.com" <mingo@redhat.com>,
	"peterz@infradead.org" <peterz@infradead.org>
Subject: [PATCH] sched: fix task and run queue run_delay inconsistencies
Date: Wed, 23 Sep 2015 00:37:18 +0000	[thread overview]
Message-ID: <72105DE171429F468B83CE64C279BFFA1760DC12@SUSHDC8002.TD.TERADATA.COM> (raw)

During evaluation of some performance data, it was discovered thread
and run queue run_delay accounting data was inconsistent with the other
accounting data that was collected.  Further investigation found under
certain circumstances execution time was leaking into the task and
run queue accounting of run_delay.

Consider the following sequence:

    a. thread is running.
    b. thread moves beween cgroups, changes scheduling class or priority.
    c. thread sleeps OR
    d. thread involuntarily gives up cpu.

a. implies:

    thread->sched_info.last_queued = 0

a. and b. results in the following:

    1. dequeue_task(rq, thread)

           sched_info_dequeued(rq, thread)
               delta = 0

               sched_info_reset_dequeued(thread)
                   thread->sched_info.last_queued = 0

               thread->sched_info.run_delay += delta

    2. enqueue_task(rq, thread)

           sched_info_queued(rq, thread)

               /* thread is still on cpu at this point. */
               thread->sched_info.last_queued = task_rq(thread)->clock;

c. results in:

    dequeue_task(rq, thread)

        sched_info_dequeued(rq, thread)

            /* delta is execution time not run_delay. */
            delta = task_rq(thread)->clock - thread->sched_info.last_queued

        sched_info_reset_dequeued(thread)
            thread->sched_info.last_queued = 0

        thread->sched_info.run_delay += delta

    Since thread was running between enqueue_task(rq, thread) and
    dequeue_task(rq, thread), the delta above is really execution
    time and not run_delay.

d. results in:

    __sched_info_switch(thread, next_thread)

        sched_info_depart(rq, thread)

            sched_info_queued(rq, thread)

                /* last_queued not updated due to being non-zero */
                return

    Since thread was running between enqueue_task(rq, thread) and
    __sched_info_switch(thread, next_thread), the execution time
    between enqueue_task(rq, thread) and
    __sched_info_switch(thread, next_thread) now will become
    associated with run_delay due to when last_queued was last updated.

The proposed patch addresses the issue by calling
sched_info_reset_dequeued(thread) following the call to
enqueue_task(rq, thread) for running threads in situations in which
thread->sched_info.last_queued should remain 0.

Signed-off-by: Mike Meyer <mike.meyer@teradata.com>
---
 kernel/sched/core.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2f9c928..88bfe43 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1187,8 +1187,12 @@ void do_set_cpus_allowed(struct task_struct *p, const struct cpumask *new_mask)
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (queued)
+	if (queued) {
 		enqueue_task(rq, p, 0);
+
+		if (running)
+			sched_info_reset_dequeued(p);
+	}
 }
 
 /*
@@ -3378,9 +3382,13 @@ void rt_mutex_setprio(struct task_struct *p, int prio)
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (queued)
+	if (queued) {
 		enqueue_task(rq, p, enqueue_flag);
 
+		if (running)
+			sched_info_reset_dequeued(p);
+	}
+
 	check_class_changed(rq, p, prev_class, oldprio);
 out_unlock:
 	preempt_disable(); /* avoid rq from going away on us */
@@ -3393,7 +3401,7 @@ out_unlock:
 
 void set_user_nice(struct task_struct *p, long nice)
 {
-	int old_prio, delta, queued;
+	int old_prio, delta, queued, running;
 	unsigned long flags;
 	struct rq *rq;
 
@@ -3415,6 +3423,7 @@ void set_user_nice(struct task_struct *p, long nice)
 		goto out_unlock;
 	}
 	queued = task_on_rq_queued(p);
+	running = task_current(rq, p);
 	if (queued)
 		dequeue_task(rq, p, 0);
 
@@ -3426,11 +3435,15 @@ void set_user_nice(struct task_struct *p, long nice)
 
 	if (queued) {
 		enqueue_task(rq, p, 0);
+
+		if (running)
+			sched_info_reset_dequeued(p);
+
 		/*
 		 * If the task increased its priority or is running and
 		 * lowered its priority, then reschedule its CPU:
 		 */
-		if (delta < 0 || (delta > 0 && task_running(rq, p)))
+		if (delta < 0 || (delta > 0 && running))
 			resched_curr(rq);
 	}
 out_unlock:
@@ -3945,6 +3958,9 @@ change:
 		 * increased (user space view).
 		 */
 		enqueue_task(rq, p, oldprio <= p->prio ? ENQUEUE_HEAD : 0);
+
+		if (running)
+			sched_info_reset_dequeued(p);
 	}
 
 	check_class_changed(rq, p, prev_class, oldprio);
@@ -5093,8 +5109,12 @@ void sched_setnuma(struct task_struct *p, int nid)
 
 	if (running)
 		p->sched_class->set_curr_task(rq);
-	if (queued)
+	if (queued) {
 		enqueue_task(rq, p, 0);
+
+		if (running)
+			sched_info_reset_dequeued(p);
+	}
 	task_rq_unlock(rq, p, &flags);
 }
 #endif /* CONFIG_NUMA_BALANCING */
@@ -7735,9 +7755,13 @@ void sched_move_task(struct task_struct *tsk)
 
 	if (unlikely(running))
 		tsk->sched_class->set_curr_task(rq);
-	if (queued)
+	if (queued) {
 		enqueue_task(rq, tsk, 0);
 
+		if (unlikely(running))
+			sched_info_reset_dequeued(tsk);
+	}
+
 	task_rq_unlock(rq, tsk, &flags);
 }
 #endif /* CONFIG_CGROUP_SCHED */
-- 
2.1.4




             reply	other threads:[~2015-09-23  1:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-23  0:37 Meyer, Mike [this message]
2015-09-30 15:44 ` [PATCH] sched: fix task and run queue run_delay inconsistencies Peter Zijlstra
2015-09-30 20:28   ` Meyer, Mike
2015-10-01  6:37     ` Peter Zijlstra
2015-10-01  9:43       ` Peter Zijlstra
2015-10-01 16:06         ` Meyer, Mike
2015-10-06 16:17   ` [tip:sched/core] sched/core: Fix task and run queue sched_info:: " tip-bot for Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=72105DE171429F468B83CE64C279BFFA1760DC12@SUSHDC8002.TD.TERADATA.COM \
    --to=mike.meyer@teradata.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).