linux-kernel - Re: [PATCH RESEND] sched: prefer an idle cpu vs an idle sibling for BALANCE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <556DE3FB.9020400@fb.com>
Date:	Tue, 2 Jun 2015 13:12:27 -0400
From:	Josef Bacik <jbacik@...com>
To:	Peter Zijlstra <peterz@...radead.org>
CC:	<riel@...hat.com>, <mingo@...hat.com>,
	<linux-kernel@...r.kernel.org>, <umgwanakikbuti@...il.com>,
	<morten.rasmussen@....com>, kernel-team <Kernel-team@...com>
Subject: Re: [PATCH RESEND] sched: prefer an idle cpu vs an idle sibling for
 BALANCE_WAKE

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.
>

Ok I took Rik's idea and came out with this


diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index b71eb2b..75073d3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -4761,13 +4761,14 @@ 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))
+		else if (want_affine && affine_sd) {
+			sd = affine_sd;
  			break;
+		}
  	}

  	if (affine_sd && cpu != prev_cpu && wake_affine(affine_sd, p, sync)) {
  		prev_cpu = cpu;
-		sd = NULL; /* WAKE_AFFINE trumps BALANCE_WAKE */
  	}

  	if (sd_flag & SD_BALANCE_WAKE) {

But now that I re-read your response I think this is even more what you 
were worried about than less.

Basically it comes down to if sd isn't set then we get shit performance. 
  I realize that this magic to find an idle cpu when sd is set is pretty 
heavy handed, but it's obviously helpful in our case.

So let me ask this question.  When do we want to do the heavy handed 
search and when do we not?  With WAKE_AFFINE what is our ultimate goal 
vs the other SD's?  If we don't have an sd that matches our sd_flags 
what should we be doing, should we just go with whatever cpu we're on 
and carry on?  Thanks,

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