Re: [PATCH] sched: smart wake-affine

[Michael Wang <wangyun@linux.vnet.ibm.com>]

On 07/02/2013 05:35 PM, Michael Wang wrote:
[snip]
>> I've seen there's some discussion as to this function name.. good :-) It
>> really wants to change. How about something like:
>>
>> int wake_affine()
>> {
>>   ...
>>
>>   /*
>>    * If we wake multiple tasks be careful to not bounce
>>    * ourselves around too much.
>>    */
>>   if (wake_wide(p))
>>   	return 0;
> 
> Do you mean wake_wipe() here?

Oh, wake_wide() means don't pull tasks together, I got it ;-)

Regards,
Michael Wang

> 
>>
>>
>>> +{
>>> +	int factor = cpumask_weight(cpu_online_mask);
>>
>> We have num_cpus_online() for this.. however both are rather expensive.
>> Having to walk and count a 4096 bitmap for every wakeup if going to get
>> tiresome real quick.
>>
>> I suppose the question is; to what level do we really want to scale?
>>
>> One fair answer would be node size I suppose; do you really want to go
>> bigger than that?
> 
> Agree, it sounds more reasonable, let me do some testing on it.
> 
>>
>> Also; you compare a size against a switching frequency, that's not
>> really and apples to apples comparison.
>>
>>> +
>>> +	/*
>>> +	 * Yeah, it's the switching-frequency, could means many wakee or
>>> +	 * rapidly switch, use factor here will just help to automatically
>>> +	 * adjust the loose-degree, so more cpu will lead to more pull.
>>> +	 */
>>> +	if (p->nr_wakee_switch > factor) {
>>> +		/*
>>> +		 * wakee is somewhat hot, it needs certain amount of cpu
>>> +		 * resource, so if waker is far more hot, prefer to leave
>>> +		 * it alone.
>>> +		 */
>>> +		if (current->nr_wakee_switch > (factor * p->nr_wakee_switch))
>>> +			return 1;
>>
>> Ah ok, this makes more sense; the first is simply a filter to avoid
>> doing the second dereference I suppose.
> 
> Yeah, the first one is some kind of vague filter, the second one is the
> core filter ;-)
> 
>>
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>>  static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync)
>>>  {
>>>  	s64 this_load, load;
>>> @@ -3118,6 +3157,9 @@ static int wake_affine(struct sched_domain *sd, struct task_struct *p, int sync)
>>>  	unsigned long weight;
>>>  	int balanced;
>>>  
>>> +	if (nasty_pull(p))
>>> +		return 0;
>>> +
>>>  	idx	  = sd->wake_idx;
>>>  	this_cpu  = smp_processor_id();
>>>  	prev_cpu  = task_cpu(p);
>>> @@ -3410,6 +3452,9 @@ select_task_rq_fair(struct task_struct *p, int sd_flag, int wake_flags)
>>>  		/* while loop will break here if sd == NULL */
>>>  	}
>>>  unlock:
>>> +	if (sd_flag & SD_BALANCE_WAKE)
>>> +		record_wakee(p);
>>
>> if we put this in task_waking_fair() we can avoid an entire conditional!
> 
> Nice, will do it in next version :)
> 
> Regards,
> Michael Wang
> 
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> Please read the FAQ at  http://www.tux.org/lkml/
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>