[PATCH] sched: Removed redundant set_current_state in "yield"

[PATCH] sched: Removed redundant set_current_state in "yield"

[Sasikantha babu]

Removed redundant setting current task state to TASK_RUNNING.

During yield process will always remains in TASK_RUNNING state. 

Process state of  yielded process will not change from TASK_RUNNING, so it does not 
make sense setting the process state again to TASK_RUNNING (TASK_RUNNING -> TASK_RUNNING).

Signed-off-by: Sasikantha babu <sasikanth.v19@gmail.com>
---
 kernel/sched/core.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 4603b9d..c2a357d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4624,7 +4624,6 @@ EXPORT_SYMBOL(__cond_resched_softirq);
  */
 void __sched yield(void)
 {
-	set_current_state(TASK_RUNNING);
 	sys_sched_yield();
 }
 EXPORT_SYMBOL(yield);
-- 
1.7.3.4

Re: [PATCH] sched: Removed redundant set_current_state in "yield"

[Peter Zijlstra]

On Tue, 2012-04-10 at 21:15 +0530, Sasikantha babu wrote:
> Removed redundant setting current task state to TASK_RUNNING.
> 
> During yield process will always remains in TASK_RUNNING state. 

Uhm, no, if you call yield() with !TASK_RUNNING you'll block and need a
proper wakeup. That is, you can in fact abuse yield() as schedule().

> Process state of  yielded process will not change from TASK_RUNNING, so it does not 
> make sense setting the process state again to TASK_RUNNING (TASK_RUNNING -> TASK_RUNNING).

No real objections, but have you tested/verified that all users are
indeed unaffected by this change?

> Signed-off-by: Sasikantha babu <sasikanth.v19@gmail.com>
> ---
>  kernel/sched/core.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 4603b9d..c2a357d 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -4624,7 +4624,6 @@ EXPORT_SYMBOL(__cond_resched_softirq);
>   */
>  void __sched yield(void)
>  {
> -	set_current_state(TASK_RUNNING);
>  	sys_sched_yield();
>  }
>  EXPORT_SYMBOL(yield);

Re: [PATCH] sched: Removed redundant set_current_state in "yield"

[Sasikanth babu]

On Mon, May 7, 2012 at 6:43 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Tue, 2012-04-10 at 21:15 +0530, Sasikantha babu wrote:
>> Removed redundant setting current task state to TASK_RUNNING.
>>
>> During yield process will always remains in TASK_RUNNING state.
>
> Uhm, no, if you call yield() with !TASK_RUNNING you'll block and need a
> proper wakeup. That is, you can in fact abuse yield() as schedule().
With the current code even if we call yield with !TASK_RUNNING, we are
setting it
back to TASK_RUNNING in yield. is this the expected behavior?.

void __sched yield(void)
 {
        set_current_state(TASK_RUNNING);
        sys_sched_yield();
 }

In the code I could not find yield was called with  !TASK_RUNNING.
>
>> Process state of  yielded process will not change from TASK_RUNNING, so it does not
>> make sense setting the process state again to TASK_RUNNING (TASK_RUNNING -> TASK_RUNNING).
>
> No real objections, but have you tested/verified that all users are
> indeed unaffected by this change?
 I did minimal testing on changes (Could not able to test all
scenarios, suggest me if there is anyway to test all cases) and
 verified none of the code path is going affected by this change.
>
>> Signed-off-by: Sasikantha babu <sasikanth.v19@gmail.com>
>> ---
>>  kernel/sched/core.c |    1 -
>>  1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 4603b9d..c2a357d 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -4624,7 +4624,6 @@ EXPORT_SYMBOL(__cond_resched_softirq);
>>   */
>>  void __sched yield(void)
>>  {
>> -     set_current_state(TASK_RUNNING);
>>       sys_sched_yield();
>>  }
>>  EXPORT_SYMBOL(yield);
>
>
>