Re: [PATCH v2] sched: Accelerate "pick_next_entity" under special condition

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

Hi, Peter

I sent this v2 patch with more description, suppose to explain
what's in my mind clearly.

Please tell me if my understanding on the logic is wrong, I really
need your help to realize the issue.

Best regards
Michael Wang

On 01/17/2012 10:36 AM, Michael Wang wrote:

> From: wangyun <wangyun@linux.vnet.ibm.com>
> 
> In original code, we get the next entity in this way:
> 
> 	if(condition1)
> 		result=value1;
> 	if(condition2)
> 		result=value2;
> 	if(condition3)
> 		result=value3;
> 	return result;
> 
> So if condition3 is true, we will get value3, but still
> need to check condition1 and condition2, this will waste
> our time.
> 
> This patch will change the way like:
> 
> 	if(condition3) {
> 		result=value3;
> 		goto out;
> 	}
> 	if(condition2) {
> 		result=value2;
> 		goto out;
> 	}
> 	if(condition1) {
> 		result=value1;
> 		goto out;
> 	}
> 
> 	out:
> 	return result;
> 
> So we can avoid check condition2 and condition1 when
> condition3 is true now.
> 
> v2:
> 	1. do not use ugly macro any more.
> 	2. add more description.
> 
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  kernel/sched/fair.c |   26 +++++++++++++++-----------
>  1 files changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 84adb2d..e8a72b2 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1308,29 +1308,33 @@ static struct sched_entity
> *pick_next_entity(struct cfs_rq *cfs_rq)
>  	struct sched_entity *left = se;
> 
>  	/*
> -	 * Avoid running the skip buddy, if running something else can
> -	 * be done without getting too unfair.
> +	 * Someone really wants this to run. If it's not unfair, run it.
>  	 */
> -	if (cfs_rq->skip == se) {
> -		struct sched_entity *second = __pick_next_entity(se);
> -		if (second && wakeup_preempt_entity(second, left) < 1)
> -			se = second;
> +	if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1) {
> +		se = cfs_rq->next;
> +		goto out;
>  	}
> 
>  	/*
>  	 * Prefer last buddy, try to return the CPU to a preempted task.
>  	 */
> -	if (cfs_rq->last && wakeup_preempt_entity(cfs_rq->last, left) < 1)
> +	if (cfs_rq->last && wakeup_preempt_entity(cfs_rq->last, left) < 1) {
>  		se = cfs_rq->last;
> +		goto out;
> +	}
> 
>  	/*
> -	 * Someone really wants this to run. If it's not unfair, run it.
> +	 * Avoid running the skip buddy, if running something else can
> +	 * be done without getting too unfair.
>  	 */
> -	if (cfs_rq->next && wakeup_preempt_entity(cfs_rq->next, left) < 1)
> -		se = cfs_rq->next;
> +	if (cfs_rq->skip == se) {
> +		struct sched_entity *second = __pick_next_entity(se);
> +		if (second && wakeup_preempt_entity(second, left) < 1)
> +			se = second;
> +	}
> 
> +out:
>  	clear_buddies(cfs_rq, se);
> -
>  	return se;
>  }
>