Re: [PATCH -tip 12/32] sched: Simplify the core pick loop for optimized case

[Peter Zijlstra]

On Tue, Nov 17, 2020 at 06:19:42PM -0500, Joel Fernandes (Google) wrote:
> +     /*
> +      * Optimize for common case where this CPU has no cookies
> +      * and there are no cookied tasks running on siblings.
> +      */
> +     if (!need_sync) {
> +             for_each_class(class) {
> +                     next = class->pick_task(rq);
> +                     if (next)
> +                             break;
> +             }
> +
> +             if (!next->core_cookie) {
> +                     rq->core_pick = NULL;
> +                     goto done;
> +             }
>               need_sync = true;
>       }

This isn't what I send you here:

  
https://lkml.kernel.org/r/20201026093131.gf2...@hirez.programming.kicks-ass.net

Specifically, you've lost the whole cfs-cgroup optimization.

What was wrong/not working with the below?

--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5225,8 +5227,6 @@ pick_next_task(struct rq *rq, struct tas
                return next;
        }
 
-       put_prev_task_balance(rq, prev, rf);
-
        smt_mask = cpu_smt_mask(cpu);
        need_sync = !!rq->core->core_cookie;
 
@@ -5255,17 +5255,14 @@ pick_next_task(struct rq *rq, struct tas
         * and there are no cookied tasks running on siblings.
         */
        if (!need_sync) {
-               for_each_class(class) {
-                       next = class->pick_task(rq);
-                       if (next)
-                               break;
-               }
-
+               next = __pick_next_task(rq, prev, rf);
                if (!next->core_cookie) {
                        rq->core_pick = NULL;
-                       goto done;
+                       return next;
                }
-               need_sync = true;
+               put_prev_task(next);
+       } else {
+               put_prev_task_balance(rq, prev, rf);
        }
 
        for_each_cpu(i, smt_mask) {