lkml.org 
[lkml]   [2019]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC PATCH v2 16/17] sched: Wake up sibling if it has something to run
On Tue, Apr 23, 2019 at 04:18:21PM +0000, Vineeth Remanan Pillai wrote:

(you lost From: Julien)

> During core scheduling, it can happen that the current rq selects a
> non-tagged process while the sibling might be idling even though it
> had something to run (because the sibling selected idle to match the
> tagged process in previous tag matching iteration). We need to wake up
> the sibling if such a situation arise.
>
> Signed-off-by: Vineeth Remanan Pillai <vpillai@digitalocean.com>
> Signed-off-by: Julien Desfossez <jdesfossez@digitalocean.com>
> ---
> kernel/sched/core.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index e8f5ec641d0a..0e3c51a1b54a 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -3775,6 +3775,21 @@ pick_next_task(struct rq *rq, struct task_struct *prev, struct rq_flags *rf)
> */
> if (i == cpu && !rq->core->core_cookie && !p->core_cookie) {
> next = p;
> + rq->core_pick = NULL;
> +
> + /*
> + * If the sibling is idling, we might want to wake it
> + * so that it can check for any runnable tasks that did
> + * not get a chance to run due to previous task matching.
> + */
> + for_each_cpu(j, smt_mask) {
> + struct rq *rq_j = cpu_rq(j);
> + rq_j->core_pick = NULL;
> + if (j != cpu &&
> + is_idle_task(rq_j->curr) && rq_j->nr_running) {
> + resched_curr(rq_j);
> + }
> + }
> goto done;
> }

Anyway, as written here:

https://lkml.kernel.org/r/20190410150116.GI2490@worktop.programming.kicks-ass.net

I think this isn't quite right. Does the below patch (which actually
removes lines) also work?

As written before; the intent was to not allow that optimization if the
last pick had a cookie; thereby doing a (last) core wide selection when
we go to a 0-cookie, and this then includes kicking forced-idle cores.

---
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3574,18 +3574,7 @@ static struct task_struct *
pick_task(struct rq *rq, const struct sched_class *class, struct task_struct *max)
{
struct task_struct *class_pick, *cookie_pick;
- unsigned long cookie = 0UL;
-
- /*
- * We must not rely on rq->core->core_cookie here, because we fail to reset
- * rq->core->core_cookie on new picks, such that we can detect if we need
- * to do single vs multi rq task selection.
- */
-
- if (max && max->core_cookie) {
- WARN_ON_ONCE(rq->core->core_cookie != max->core_cookie);
- cookie = max->core_cookie;
- }
+ unsigned long cookie = rq->core->core_cookie;

class_pick = class->pick_task(rq);
if (!cookie)
@@ -3612,6 +3601,7 @@ pick_next_task(struct rq *rq, struct tas
struct task_struct *next, *max = NULL;
const struct sched_class *class;
const struct cpumask *smt_mask;
+ unsigned long prev_cookie;
int i, j, cpu;

if (!sched_core_enabled(rq))
@@ -3653,7 +3643,10 @@ pick_next_task(struct rq *rq, struct tas
*/
rq->core->core_task_seq++;

+ prev_cookie = rq->core->core_cookie;
+
/* reset state */
+ rq->core->core_cookie = 0UL;
for_each_cpu(i, smt_mask) {
struct rq *rq_i = cpu_rq(i);

@@ -3688,7 +3681,7 @@ pick_next_task(struct rq *rq, struct tas
* If there weren't no cookies; we don't need
* to bother with the other siblings.
*/
- if (i == cpu && !rq->core->core_cookie)
+ if (i == cpu && !prev_cookie)
goto next_class;

continue;
@@ -3698,7 +3691,7 @@ pick_next_task(struct rq *rq, struct tas
* Optimize the 'normal' case where there aren't any
* cookies and we don't need to sync up.
*/
- if (i == cpu && !rq->core->core_cookie && !p->core_cookie) {
+ if (i == cpu && !prev_cookie && !p->core_cookie) {
next = p;
goto done;
}
\
 
 \ /
  Last update: 2019-04-26 17:04    [W:0.393 / U:5.464 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site