Re: [PATCH RESEND] sched: prefer an idle cpu vs an idle sibling for BALANCE_WAKE

From: Josef Bacik
Date: Mon Jun 01 2015 - 17:05:00 EST


On 06/01/2015 04:42 PM, Peter Zijlstra wrote:
On Mon, 2015-06-01 at 15:38 -0400, Josef Bacik wrote:

Ok I got this patch to give me the same performance as all our other
crap, just need to apply this incremental


diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b71eb2b..e11cfec 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4761,13 +4761,10 @@ select_task_rq_fair(struct task_struct *p, int
prev_cpu, int sd_flag, int wake_f

if (tmp->flags & sd_flag)
sd = tmp;
- else if (!want_affine || (want_affine && affine_sd))
- break;
}

That bit worries me a bit, because that causes us to have a weird
definition for what sd is.

Without WAKE_AFFINE, sd is the biggest domain with BALANCE_WAKE (or any
other sd_flag) set.

But with WAKE_AFFINE, its the first domain that satisfies the wake
affine constraint of covering both the previous and waking cpu. It
basically reduces sd to affine_sd.


I'm just giving you what made the performance good for us, I'm relying on you to make it sane ;). So with that bit we break out if !wake_affine as soon as we don't find an sd that matches sd_flag, that seems less than helpful. I was going to change it to

else if (want_affine && affine_sd)

but the thing is we don't do anything with affine_sd unless the bit below matches

if (affine_sd && cpu != prev_cpu && wake_affine(affine_sd, p, sync))

so it seems like we're leaving ourselves without sd set in a few cases where we'd actually want it set, so I just nuked the whole thing and carried on. I'm all ears for other ideas, I'm just letting you know what my results are since you are the guys who actually understand this stuff. Thanks,

Josef

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/