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

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

[Michael Wang]

From: Michael Wang <wangyun@linux.vnet.ibm.com>

We can avoid some useless operation in some special condition.

For example:
If we have "cfs_rq->next" and it can be use, we just return it directly.

Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
 kernel/sched/fair.c |   28 +++++++++++++++++-----------
 1 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 84adb2d..9fc2c3c 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -1295,6 +1295,8 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
 static int
 wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se);
 
+#define ENTITY_PREEMPT_ALLOWED(prev,next)	(wakeup_preempt_entity(prev, next) < 1)
+
 /*
  * Pick the next process, keeping these things in mind, in this order:
  * 1) keep things fair between processes/task groups
@@ -1308,29 +1310,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 && ENTITY_PREEMPT_ALLOWED(cfs_rq->next, left)) {
+		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 && ENTITY_PREEMPT_ALLOWED(cfs_rq->last, left)) {
 		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 && ENTITY_PREEMPT_ALLOWED(second, left))
+			se = second;
+	}
 
+out:
 	clear_buddies(cfs_rq, se);
-
 	return se;
 }
 
-- 
1.7.4.1

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

[Michael Wang]

On 01/16/2012 05:37 PM, Michael Wang wrote:

> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> 
> We can avoid some useless operation in some special condition.
> 
> For example:
> If we have "cfs_rq->next" and it can be use, we just return it directly.
> 


Please tell me if I got wrong understanding on these code, I think the change can 
maintain the old logic and suppose to be a little quick in some condition.
 

> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  kernel/sched/fair.c |   28 +++++++++++++++++-----------
>  1 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 84adb2d..9fc2c3c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1295,6 +1295,8 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  static int
>  wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se);
>  
> +#define ENTITY_PREEMPT_ALLOWED(prev,next)	(wakeup_preempt_entity(prev, next) < 1)


I want to use a name which can make it more easy to be understand, please tell me 
if it is a bad idea...

Regards,
Michael Wang

> +
>  /*
>   * Pick the next process, keeping these things in mind, in this order:
>   * 1) keep things fair between processes/task groups
> @@ -1308,29 +1310,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 && ENTITY_PREEMPT_ALLOWED(cfs_rq->next, left)) {
> +		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 && ENTITY_PREEMPT_ALLOWED(cfs_rq->last, left)) {
>  		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 && ENTITY_PREEMPT_ALLOWED(second, left))
> +			se = second;
> +	}
>  
> +out:
>  	clear_buddies(cfs_rq, se);
> -
>  	return se;
>  }
>  

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

[Peter Zijlstra]

On Mon, 2012-01-16 at 17:37 +0800, Michael Wang wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> 
> We can avoid some useless operation in some special condition.

This is a pretty empty statement.

> For example:
> If we have "cfs_rq->next" and it can be use, we just return it directly.

What it doesn't state is what it actually does, if it affects the common
case and performance numbers (or a good reason for the lack thereof).

> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  kernel/sched/fair.c |   28 +++++++++++++++++-----------
>  1 files changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
> index 84adb2d..9fc2c3c 100644
> --- a/kernel/sched/fair.c
> +++ b/kernel/sched/fair.c
> @@ -1295,6 +1295,8 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
>  static int
>  wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se);
>  
> +#define ENTITY_PREEMPT_ALLOWED(prev,next)	(wakeup_preempt_entity(prev, next) < 1)

This is just uglification imo, its shouting and it doesn't actually win
you much space.

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

[Michael Wang]

Hi, peter

Thanks so much for your reply :)

On 01/16/2012 05:51 PM, Peter Zijlstra wrote:

> On Mon, 2012-01-16 at 17:37 +0800, Michael Wang wrote:
>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>
>> We can avoid some useless operation in some special condition.
> 
> This is a pretty empty statement.
> 
>> For example:
>> If we have "cfs_rq->next" and it can be use, we just return it directly.
> 
> What it doesn't state is what it actually does, if it affects the common
> case and performance numbers (or a good reason for the lack thereof).
> 


Please help me to understand the logic, I think in the original code,
even if we have cfs_rq->next and wakeup_preempt_entity check passed, we
still need to do a lot of work (check cfs_rq->last for example) which
have no influence on result, will it be better if we skip them and just
do what really needed?

>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>> ---
>>  kernel/sched/fair.c |   28 +++++++++++++++++-----------
>>  1 files changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>> index 84adb2d..9fc2c3c 100644
>> --- a/kernel/sched/fair.c
>> +++ b/kernel/sched/fair.c
>> @@ -1295,6 +1295,8 @@ set_next_entity(struct cfs_rq *cfs_rq, struct sched_entity *se)
>>  static int
>>  wakeup_preempt_entity(struct sched_entity *curr, struct sched_entity *se);
>>  
>> +#define ENTITY_PREEMPT_ALLOWED(prev,next)	(wakeup_preempt_entity(prev, next) < 1)
> 

> This is just uglification imo, its shouting and it doesn't actually win
> you much space.
> 


I see, sorry for the bad idea.

Best regards,
Michael Wang

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

[Michael Wang]

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;
 }

-- 
1.7.4.1

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

[Michael Wang]

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;
>  }
> 

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

[Xiaotian Feng]

On Tue, Jan 17, 2012 at 10:36 AM, Michael Wang
<wangyun@linux.vnet.ibm.com> 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.

Then what if condition 1 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;
>  }
>
> --
> 1.7.4.1
>
> --
> 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/

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

[Michael Wang]

On 01/17/2012 10:58 AM, Xiaotian Feng wrote:

> On Tue, Jan 17, 2012 at 10:36 AM, Michael Wang
> <wangyun@linux.vnet.ibm.com> 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.
> 
> Then what if condition 1 is true now?


Hi, Xiaotian

Thanks for your reply.

We can see in original code, even condition 1 is true, we
still will use value3 if condition3 is true, like this:

original:

condition1	condition3	result
true		true		value3
true		false		value1

That means if condition3 is true, we don't care whether
condition1 is true or not because we will finally use value3.

Regards,
Michael Wang

> 
>>
>> 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;
>>  }
>>
>> --
>> 1.7.4.1
>>
>> --
>> 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/
> 

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

[Peter Zijlstra]

On Tue, 2012-01-17 at 11:04 +0800, Michael Wang wrote:
> 
> > Then what if condition 1 is true now?
> 
> We can see in original code, even condition 1 is true, we
> still will use value3 if condition3 is true, like this:
> 
> original:
> 
> condition1      condition3      result
> true            true            value3
> true            false           value1
> 
> That means if condition3 is true, we don't care whether
> condition1 is true or not because we will finally use value3.

Right, so from the original 8 possible states we used to evaluate 3*8 =
24 conditionals. The new code will reduce this to 1*4 + 2*2 + 2*3 = 14.

Now I guess the question is if it matters for the modal or average
state.

I've applied the patch since it can't be worse, but I've no idea if it
matters or not in practice.

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

[Ingo Molnar]

* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:

> On Tue, 2012-01-17 at 11:04 +0800, Michael Wang wrote:
> > 
> > > Then what if condition 1 is true now?
> > 
> > We can see in original code, even condition 1 is true, we
> > still will use value3 if condition3 is true, like this:
> > 
> > original:
> > 
> > condition1      condition3      result
> > true            true            value3
> > true            false           value1
> > 
> > That means if condition3 is true, we don't care whether
> > condition1 is true or not because we will finally use value3.
> 
> Right, so from the original 8 possible states we used to evaluate 3*8 =
> 24 conditionals. The new code will reduce this to 1*4 + 2*2 + 2*3 = 14.
> 
> Now I guess the question is if it matters for the modal or average
> state.
> 
> I've applied the patch since it can't be worse, but I've no idea if it
> matters or not in practice.

A before/after kernel/sched.o size comparison on 64-bit 
defconfig typically gives a pretty good indication whether it's 
a step forward or not.

Thanks,

	Ingo

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

[Michael Wang]

On 01/26/2012 06:04 PM, Ingo Molnar wrote:

> 
> * Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> 
>> On Tue, 2012-01-17 at 11:04 +0800, Michael Wang wrote:
>>>
>>>> Then what if condition 1 is true now?
>>>
>>> We can see in original code, even condition 1 is true, we
>>> still will use value3 if condition3 is true, like this:
>>>
>>> original:
>>>
>>> condition1      condition3      result
>>> true            true            value3
>>> true            false           value1
>>>
>>> That means if condition3 is true, we don't care whether
>>> condition1 is true or not because we will finally use value3.
>>
>> Right, so from the original 8 possible states we used to evaluate 3*8 =
>> 24 conditionals. The new code will reduce this to 1*4 + 2*2 + 2*3 = 14.
>>
>> Now I guess the question is if it matters for the modal or average
>> state.
>>
>> I've applied the patch since it can't be worse, but I've no idea if it
>> matters or not in practice.
> 
> A before/after kernel/sched.o size comparison on 64-bit 
> defconfig typically gives a pretty good indication whether it's 
> a step forward or not.
> 
> Thanks,
> 
> 	Ingo
> 


Hi, Ingo

Thanks for your reply.

I have try use "ls -l" to see the size of sched.o, but after applied the
patch, it is still 1636.

I have not use this method before, may be I use the wrong command...

But I think the new code should be similar to the old one after compile,
because we still have 3 condition check here.

I suppose the new sched.o will be a little bigger, because one jump
command and a label need to be added.

Regards,
Michael Wang

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

[Cong Wang]

On 01/27/2012 09:22 AM, Michael Wang wrote:
> Hi, Ingo
>
> Thanks for your reply.
>
> I have try use "ls -l" to see the size of sched.o, but after applied the
> patch, it is still 1636.
>
> I have not use this method before, may be I use the wrong command...
>
> But I think the new code should be similar to the old one after compile,
> because we still have 3 condition check here.
>
> I suppose the new sched.o will be a little bigger, because one jump
> command and a label need to be added.
>

Try to see if `size` helps.

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

[Michael Wang]

On 01/27/2012 12:42 PM, Cong Wang wrote:

> On 01/27/2012 09:22 AM, Michael Wang wrote:
>> Hi, Ingo
>>
>> Thanks for your reply.
>>
>> I have try use "ls -l" to see the size of sched.o, but after applied the
>> patch, it is still 1636.
>>
>> I have not use this method before, may be I use the wrong command...
>>
>> But I think the new code should be similar to the old one after compile,
>> because we still have 3 condition check here.
>>
>> I suppose the new sched.o will be a little bigger, because one jump
>> command and a label need to be added.
>>
> 
> Try to see if `size` helps.

Hi, Cong

Thanks for your advise, but still, the size not changed.

And also I don't know whether the size can be some kind of proof to
confirm the performance improvement in this case...

Thanks,
Michael Wang

> 

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

[Ingo Molnar]

* Michael Wang <wangyun@linux.vnet.ibm.com> wrote:

> On 01/27/2012 12:42 PM, Cong Wang wrote:
> 
> > On 01/27/2012 09:22 AM, Michael Wang wrote:
> >> Hi, Ingo
> >>
> >> Thanks for your reply.
> >>
> >> I have try use "ls -l" to see the size of sched.o, but after applied the
> >> patch, it is still 1636.
> >>
> >> I have not use this method before, may be I use the wrong command...
> >>
> >> But I think the new code should be similar to the old one after compile,
> >> because we still have 3 condition check here.
> >>
> >> I suppose the new sched.o will be a little bigger, because one jump
> >> command and a label need to be added.
> >>
> > 
> > Try to see if `size` helps.
> 
> Hi, Cong
> 
> Thanks for your advise, but still, the size not changed.
> 
> And also I don't know whether the size can be some kind of 
> proof to confirm the performance improvement in this case...

You could disassemble the .o file via objdump -d and run diff on 
it - is there any change in the code generated by GCC?

Thanks,

	Ingo

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

[Michael Wang]

On 01/30/2012 12:33 AM, Ingo Molnar wrote:

> 
> * Michael Wang <wangyun@linux.vnet.ibm.com> wrote:
> 
>> On 01/27/2012 12:42 PM, Cong Wang wrote:
>>
>>> On 01/27/2012 09:22 AM, Michael Wang wrote:
>>>> Hi, Ingo
>>>>
>>>> Thanks for your reply.
>>>>
>>>> I have try use "ls -l" to see the size of sched.o, but after applied the
>>>> patch, it is still 1636.
>>>>
>>>> I have not use this method before, may be I use the wrong command...
>>>>
>>>> But I think the new code should be similar to the old one after compile,
>>>> because we still have 3 condition check here.
>>>>
>>>> I suppose the new sched.o will be a little bigger, because one jump
>>>> command and a label need to be added.
>>>>
>>>
>>> Try to see if `size` helps.
>>
>> Hi, Cong
>>
>> Thanks for your advise, but still, the size not changed.
>>
>> And also I don't know whether the size can be some kind of 
>> proof to confirm the performance improvement in this case...
> 
> You could disassemble the .o file via objdump -d and run diff on 
> it - is there any change in the code generated by GCC?
> 
> Thanks,
> 
> 	Ingo
> 

This tool is great :)
But the sched.o under ./arch/x86/kernel/cpu/ still not change...
I think I may checking the wrong file, because this patch is for fair.c.

And the fair.o changed after apply the patch, the size is a little
bigger, and the gcc generated code changed.

But I still don't know what can we get from this result? Bigger size is
caused by additional code, but these additional code will help to step
over some unnecessary code under special condition, looks like some
balance between size and performance...

Regards,
Michael Wang

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

[Cong Wang]

On 01/30/2012 11:18 AM, Michael Wang wrote:
> But the sched.o under ./arch/x86/kernel/cpu/ still not change...
> I think I may checking the wrong file, because this patch is for fair.c.
>
> And the fair.o changed after apply the patch, the size is a little
> bigger, and the gcc generated code changed.
>
> But I still don't know what can we get from this result? Bigger size is
> caused by additional code, but these additional code will help to step
> over some unnecessary code under special condition, looks like some
> balance between size and performance...

As your patch reduces the conditionals from 24 to 14, so it is possible 
that it also reduces the size of the code too. This is Ingo's point.

You need to check the diff to see why gcc actually generates bigger code.

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

[Michael Wang]

On 01/30/2012 11:25 AM, Cong Wang wrote:

> On 01/30/2012 11:18 AM, Michael Wang wrote:
>> But the sched.o under ./arch/x86/kernel/cpu/ still not change...
>> I think I may checking the wrong file, because this patch is for fair.c.
>>
>> And the fair.o changed after apply the patch, the size is a little
>> bigger, and the gcc generated code changed.
>>
>> But I still don't know what can we get from this result? Bigger size is
>> caused by additional code, but these additional code will help to step
>> over some unnecessary code under special condition, looks like some
>> balance between size and performance...
> 
> As your patch reduces the conditionals from 24 to 14, so it is possible
> that it also reduces the size of the code too. This is Ingo's point.
> 


I think the number reduce because we can ignore some condition in
special case after applied the patch, but all the old code are still
needed, the compiled code should like:

old:

check condition 1
	process condition 1
check condition 2
	process condition 2
check condition 3
	process condition 3
return

new:

check condition 3
	process condition 3
	jump to label		//we can step over some code here
check condition 2
	process condition 2
	jump to label
check condition 1
	process condition 1
label:return

We can do this change because the priority is 3 > 2 > 1.

So while the code is running and the condition 3 matched, the cpu don't
need to run the code "check condition 2" and "check condition 1".

But those code is still needed, and we can see the "jump to label" will
be the additional code which is the reason of bigger size.

Regards,
Michael Wang

> You need to check the diff to see why gcc actually generates bigger code.
> -- 
> 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/
> 

[PATCH] sched: remove useless code in yield_to

[Michael Wang]

From: Michael Wang <wangyun@linux.vnet.ibm.com>

it's impossible to enter else branch if we have set skip_clock_update
in task_yield_fair(), as yield_to_task_fair() will directly return
true after invoke task_yield_fair().

Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
---
 kernel/sched/core.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 9bb7d28..77c14aa 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4737,13 +4737,6 @@ again:
 		 */
 		if (preempt && rq != p_rq)
 			resched_task(p_rq->curr);
-	} else {
-		/*
-		 * We might have set it in task_yield_fair(), but are
-		 * not going to schedule(), so don't want to skip
-		 * the next update.
-		 */
-		rq->skip_clock_update = 0;
 	}
 
 out:
-- 
1.7.4.1

Re: [PATCH] sched: remove useless code in yield_to

[Michael Wang]

On 07/03/2012 02:34 PM, Michael Wang wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> 
> it's impossible to enter else branch if we have set skip_clock_update
> in task_yield_fair(), as yield_to_task_fair() will directly return
> true after invoke task_yield_fair().
> 
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  kernel/sched/core.c |    7 -------
>  1 files changed, 0 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9bb7d28..77c14aa 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4737,13 +4737,6 @@ again:
>  		 */
>  		if (preempt && rq != p_rq)
>  			resched_task(p_rq->curr);
> -	} else {
> -		/*
> -		 * We might have set it in task_yield_fair(), but are
> -		 * not going to schedule(), so don't want to skip
> -		 * the next update.
> -		 */
> -		rq->skip_clock_update = 0;
>  	}

Can I get some comments on this patch?

Regards,
Michael Wang

> 
>  out:
> 

Re: [PATCH] sched: remove useless code in yield_to

[Peter Zijlstra]

On Tue, 2012-07-03 at 14:34 +0800, Michael Wang wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> 
> it's impossible to enter else branch if we have set skip_clock_update
> in task_yield_fair(), as yield_to_task_fair() will directly return
> true after invoke task_yield_fair().

It helps if you CC the guy who wrote the code.. I think you're right,
although getting that skip_clock_update crap wrong is annoying.

Mike?


> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  kernel/sched/core.c |    7 -------
>  1 files changed, 0 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9bb7d28..77c14aa 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4737,13 +4737,6 @@ again:
>  		 */
>  		if (preempt && rq != p_rq)
>  			resched_task(p_rq->curr);
> -	} else {
> -		/*
> -		 * We might have set it in task_yield_fair(), but are
> -		 * not going to schedule(), so don't want to skip
> -		 * the next update.
> -		 */
> -		rq->skip_clock_update = 0;
>  	}
>  
>  out:

Re: [PATCH] sched: remove useless code in yield_to

[Mike Galbraith]

On Thu, 2012-07-12 at 16:07 +0200, Peter Zijlstra wrote: 
> On Tue, 2012-07-03 at 14:34 +0800, Michael Wang wrote:
> > From: Michael Wang <wangyun@linux.vnet.ibm.com>
> > 
> > it's impossible to enter else branch if we have set skip_clock_update
> > in task_yield_fair(), as yield_to_task_fair() will directly return
> > true after invoke task_yield_fair().
> 
> It helps if you CC the guy who wrote the code.. I think you're right,
> although getting that skip_clock_update crap wrong is annoying.

Getting that crap wrong is ease, so Michael is probably right.

> Mike?

If it looks right to you too, it likely is.  When I have time to look
for nanoseconds again, I can squeak if/when I find some.  In any case,
for normal scheduler citizens, it's chump change, so nuke it.
> > Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> > ---
> >  kernel/sched/core.c |    7 -------
> >  1 files changed, 0 insertions(+), 7 deletions(-)
> > 
> > diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> > index 9bb7d28..77c14aa 100644
> > --- a/kernel/sched/core.c
> > +++ b/kernel/sched/core.c
> > @@ -4737,13 +4737,6 @@ again:
> >  		 */
> >  		if (preempt && rq != p_rq)
> >  			resched_task(p_rq->curr);
> > -	} else {
> > -		/*
> > -		 * We might have set it in task_yield_fair(), but are
> > -		 * not going to schedule(), so don't want to skip
> > -		 * the next update.
> > -		 */
> > -		rq->skip_clock_update = 0;
> >  	}
> >  
> >  out:
> 
> --
> 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/

Re: [PATCH] sched: remove useless code in yield_to

[Michael Wang]

On 07/12/2012 10:07 PM, Peter Zijlstra wrote:
> On Tue, 2012-07-03 at 14:34 +0800, Michael Wang wrote:
>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>
>> it's impossible to enter else branch if we have set skip_clock_update
>> in task_yield_fair(), as yield_to_task_fair() will directly return
>> true after invoke task_yield_fair().
> 
> It helps if you CC the guy who wrote the code.. 

Oh, my fault..., will take care next time.

I think you're right,
> although getting that skip_clock_update crap wrong is annoying.
> 
> Mike?
> 
> 
>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>> ---
>>  kernel/sched/core.c |    7 -------
>>  1 files changed, 0 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 9bb7d28..77c14aa 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4737,13 +4737,6 @@ again:
>>  		 */
>>  		if (preempt && rq != p_rq)
>>  			resched_task(p_rq->curr);
>> -	} else {
>> -		/*
>> -		 * We might have set it in task_yield_fair(), but are
>> -		 * not going to schedule(), so don't want to skip
>> -		 * the next update.
>> -		 */
>> -		rq->skip_clock_update = 0;
>>  	}
>>  
>>  out:
> 
> --
> 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/
> 

Re: [PATCH] sched: remove useless code in yield_to

[Michael Wang]

On 07/12/2012 10:07 PM, Peter Zijlstra wrote:
> On Tue, 2012-07-03 at 14:34 +0800, Michael Wang wrote:
>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>
>> it's impossible to enter else branch if we have set skip_clock_update
>> in task_yield_fair(), as yield_to_task_fair() will directly return
>> true after invoke task_yield_fair().
> 
> It helps if you CC the guy who wrote the code.. I think you're right,
> although getting that skip_clock_update crap wrong is annoying.
> 
> Mike?

Hi, Peter

I think Mike also agreed to remove that piece of code, should we action now?

Regards,
Michael Wang

> 
> 
>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>> ---
>>  kernel/sched/core.c |    7 -------
>>  1 files changed, 0 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 9bb7d28..77c14aa 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4737,13 +4737,6 @@ again:
>>  		 */
>>  		if (preempt && rq != p_rq)
>>  			resched_task(p_rq->curr);
>> -	} else {
>> -		/*
>> -		 * We might have set it in task_yield_fair(), but are
>> -		 * not going to schedule(), so don't want to skip
>> -		 * the next update.
>> -		 */
>> -		rq->skip_clock_update = 0;
>>  	}
>>  
>>  out:
> 
> --
> 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/
> 

Re: [PATCH] sched: remove useless code in yield_to

[Peter Zijlstra]

On Fri, 2012-08-17 at 14:56 +0800, Michael Wang wrote:
> 
> I think Mike also agreed to remove that piece of code, should we
> action now?

Right you are, applied.

Re: [PATCH] sched: remove useless code in yield_to

[Michael Wang]

On 07/03/2012 02:34 PM, Michael Wang wrote:
> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> 
> it's impossible to enter else branch if we have set skip_clock_update
> in task_yield_fair(), as yield_to_task_fair() will directly return
> true after invoke task_yield_fair().

Could I get some conclusion on this patch? Should we clean up that peace
of code or leave it there?

Regards,
Michael Wang

> 
> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> ---
>  kernel/sched/core.c |    7 -------
>  1 files changed, 0 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 9bb7d28..77c14aa 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4737,13 +4737,6 @@ again:
>  		 */
>  		if (preempt && rq != p_rq)
>  			resched_task(p_rq->curr);
> -	} else {
> -		/*
> -		 * We might have set it in task_yield_fair(), but are
> -		 * not going to schedule(), so don't want to skip
> -		 * the next update.
> -		 */
> -		rq->skip_clock_update = 0;
>  	}
> 
>  out:
> 

Re: [PATCH] sched: remove useless code in yield_to

[Michael Wang]

On 08/10/2012 11:05 AM, Michael Wang wrote:
> On 07/03/2012 02:34 PM, Michael Wang wrote:
>> From: Michael Wang <wangyun@linux.vnet.ibm.com>
>>
>> it's impossible to enter else branch if we have set skip_clock_update
>> in task_yield_fair(), as yield_to_task_fair() will directly return
>> true after invoke task_yield_fair().
> 
> Could I get some conclusion on this patch? Should we clean up that peace
> of code or leave it there?
s /peace/piece and cc mike...
> 
> Regards,
> Michael Wang
> 
>>
>> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
>> ---
>>  kernel/sched/core.c |    7 -------
>>  1 files changed, 0 insertions(+), 7 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 9bb7d28..77c14aa 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4737,13 +4737,6 @@ again:
>>  		 */
>>  		if (preempt && rq != p_rq)
>>  			resched_task(p_rq->curr);
>> -	} else {
>> -		/*
>> -		 * We might have set it in task_yield_fair(), but are
>> -		 * not going to schedule(), so don't want to skip
>> -		 * the next update.
>> -		 */
>> -		rq->skip_clock_update = 0;
>>  	}
>>
>>  out:
>>
> 
> --
> 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/
> 

Re: [PATCH] sched: remove useless code in yield_to

[Mike Galbraith]

On Fri, 2012-08-10 at 11:10 +0800, Michael Wang wrote: 
> On 08/10/2012 11:05 AM, Michael Wang wrote:
> > On 07/03/2012 02:34 PM, Michael Wang wrote:
> >> From: Michael Wang <wangyun@linux.vnet.ibm.com>
> >>
> >> it's impossible to enter else branch if we have set skip_clock_update
> >> in task_yield_fair(), as yield_to_task_fair() will directly return
> >> true after invoke task_yield_fair().
> > 
> > Could I get some conclusion on this patch? Should we clean up that peace
> > of code or leave it there?
> s /peace/piece and cc mike...

If some other class grows yield_to_task (ick), it'll have to be looked
at again, but hopefully for all eternity that branch is dead.  Bury it.

> > 
> > Regards,
> > Michael Wang
> > 
> >>
> >> Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
> >> ---
> >>  kernel/sched/core.c |    7 -------
> >>  1 files changed, 0 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> >> index 9bb7d28..77c14aa 100644
> >> --- a/kernel/sched/core.c
> >> +++ b/kernel/sched/core.c
> >> @@ -4737,13 +4737,6 @@ again:
> >>  		 */
> >>  		if (preempt && rq != p_rq)
> >>  			resched_task(p_rq->curr);
> >> -	} else {
> >> -		/*
> >> -		 * We might have set it in task_yield_fair(), but are
> >> -		 * not going to schedule(), so don't want to skip
> >> -		 * the next update.
> >> -		 */
> >> -		rq->skip_clock_update = 0;
> >>  	}
> >>
> >>  out:
> >>
> > 
> > --
> > 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/
> > 
> 

[tip:sched/core] sched: Remove useless code in yield_to()

[tip-bot for Michael Wang]

Commit-ID:  38b8dd6f87398524d02c21ff614c507ba8c9d295
Gitweb:     http://git.kernel.org/tip/38b8dd6f87398524d02c21ff614c507ba8c9d295
Author:     Michael Wang <wangyun@linux.vnet.ibm.com>
AuthorDate: Tue, 3 Jul 2012 14:34:02 +0800
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Tue, 4 Sep 2012 14:31:42 +0200

sched: Remove useless code in yield_to()

It's impossible to enter the else branch if we have set
skip_clock_update in task_yield_fair(), as yield_to_task_fair()
 will directly return true after invoke task_yield_fair().

Signed-off-by: Michael Wang <wangyun@linux.vnet.ibm.com>
Acked-by: Mike Galbraith <efault@gmx.de>
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/4FF2925A.9060005@linux.vnet.ibm.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/sched/core.c |    7 -------
 1 files changed, 0 insertions(+), 7 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index ec0f2b8..c46a011 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4348,13 +4348,6 @@ again:
 		 */
 		if (preempt && rq != p_rq)
 			resched_task(p_rq->curr);
-	} else {
-		/*
-		 * We might have set it in task_yield_fair(), but are
-		 * not going to schedule(), so don't want to skip
-		 * the next update.
-		 */
-		rq->skip_clock_update = 0;
 	}
 
 out:

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

[Michael Wang]

On 01/25/2012 11:55 PM, Peter Zijlstra wrote:

> On Tue, 2012-01-17 at 11:04 +0800, Michael Wang wrote:
>>
>>> Then what if condition 1 is true now?
>>
>> We can see in original code, even condition 1 is true, we
>> still will use value3 if condition3 is true, like this:
>>
>> original:
>>
>> condition1      condition3      result
>> true            true            value3
>> true            false           value1
>>
>> That means if condition3 is true, we don't care whether
>> condition1 is true or not because we will finally use value3.
> 

> Right, so from the original 8 possible states we used to evaluate 3*8 =
> 24 conditionals. The new code will reduce this to 1*4 + 2*2 + 2*3 = 14.
> 
> Now I guess the question is if it matters for the modal or average
> state.
> 


Hi, Peter

I think how much help this patch can do on the performance depends on
how often the condition2 and 3 occur.

> I've applied the patch since it can't be worse, but I've no idea if it
> matters or not in practice.


It's hardly to say how useful this patch is, but it will do some help
under special condition.
Thanks for your review.

Regards,
Michael Wang

>