[PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Wang Qing]

Why not just use wake_up_process().

Signed-off-by: Wang Qing <wangqing@vivo.com>
---
 kernel/sched/swait.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
index e1c655f..7a24925
--- a/kernel/sched/swait.c
+++ b/kernel/sched/swait.c
@@ -69,7 +69,7 @@ void swake_up_all(struct swait_queue_head *q)
 	while (!list_empty(&tmp)) {
 		curr = list_first_entry(&tmp, typeof(*curr), task_list);
 
-		wake_up_state(curr->task, TASK_NORMAL);
+		wake_up_process(curr->task);
 		list_del_init(&curr->task_list);
 
 		if (list_empty(&tmp))
-- 
2.7.4

Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Mike Galbraith]

On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
> Why not just use wake_up_process().

IMO this is not an improvement.  There are other places where explicit
TASK_NORMAL is used as well, and they're all perfectly clear as is.

> Signed-off-by: Wang Qing <wangqing@vivo.com>
> ---
>  kernel/sched/swait.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
> index e1c655f..7a24925
> --- a/kernel/sched/swait.c
> +++ b/kernel/sched/swait.c
> @@ -69,7 +69,7 @@ void swake_up_all(struct swait_queue_head *q)
>  	while (!list_empty(&tmp)) {
>  		curr = list_first_entry(&tmp, typeof(*curr), task_list);
>
> -		wake_up_state(curr->task, TASK_NORMAL);
> +		wake_up_process(curr->task);
>  		list_del_init(&curr->task_list);
>
>  		if (list_empty(&tmp))

Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Ingo Molnar]

* Mike Galbraith <efault@gmx.de> wrote:

> On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
> > Why not just use wake_up_process().
> 
> IMO this is not an improvement.  There are other places where explicit
> TASK_NORMAL is used as well, and they're all perfectly clear as is.

Arguably those could all be converted to wake_up_process() as well. 
It's a very small kernel code size optimization. There's about 3 such 
places, could be converted in a single patch.

Thanks,

	Ingo

Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Peter Zijlstra]

On Wed, Mar 17, 2021 at 10:46:18AM +0100, Ingo Molnar wrote:
> 
> * Mike Galbraith <efault@gmx.de> wrote:
> 
> > On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
> > > Why not just use wake_up_process().
> > 
> > IMO this is not an improvement.  There are other places where explicit
> > TASK_NORMAL is used as well, and they're all perfectly clear as is.
> 
> Arguably those could all be converted to wake_up_process() as well. 
> It's a very small kernel code size optimization. There's about 3 such 
> places, could be converted in a single patch.

It's still pointless churn IMO.

Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Mike Galbraith]

On Wed, 2021-03-17 at 10:46 +0100, Ingo Molnar wrote:
> * Mike Galbraith <efault@gmx.de> wrote:
>
> > On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
> > > Why not just use wake_up_process().
> >
> > IMO this is not an improvement.  There are other places where explicit
> > TASK_NORMAL is used as well, and they're all perfectly clear as is.
>
> Arguably those could all be converted to wake_up_process() as well.
> It's a very small kernel code size optimization. There's about 3 such
> places, could be converted in a single patch.

I still prefer the way it sits, but that's certainlyly a heck of a lot
better change justification than "why not" :)

	-Mike

Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Thomas Gleixner]

On Wed, Mar 17 2021 at 11:41, Mike Galbraith wrote:
> On Wed, 2021-03-17 at 10:46 +0100, Ingo Molnar wrote:
>> * Mike Galbraith <efault@gmx.de> wrote:
>>
>> > On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
>> > > Why not just use wake_up_process().
>> >
>> > IMO this is not an improvement.  There are other places where explicit
>> > TASK_NORMAL is used as well, and they're all perfectly clear as is.
>>
>> Arguably those could all be converted to wake_up_process() as well.
>> It's a very small kernel code size optimization. There's about 3 such
>> places, could be converted in a single patch.
>
> I still prefer the way it sits, but that's certainlyly a heck of a lot
> better change justification than "why not" :)

Which begs the reply "Why should we?" just for 10 bytes less of kernel
text :)

Re:Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[王擎]

>> 
>> * Mike Galbraith <efault@gmx.de> wrote:
>> 
>> > On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
>> > > Why not just use wake_up_process().
>> > 
>> > IMO this is not an improvement.  There are other places where explicit
>> > TASK_NORMAL is used as well, and they're all perfectly clear as is.
>> 
>> Arguably those could all be converted to wake_up_process() as well. 
>> It's a very small kernel code size optimization. There's about 3 such 
>> places, could be converted in a single patch.
>
>It's still pointless churn IMO.

Using wake_up_process() is more simpler and friendly for beginners, 
and it is more convenient for analysis and statistics.

Re: Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()

[Mike Galbraith]

On Thu, 2021-03-18 at 10:14 +0800, 王擎 wrote:
> >>
> >> * Mike Galbraith <efault@gmx.de> wrote:
> >>
> >> > On Tue, 2021-03-16 at 19:20 +0800, Wang Qing wrote:
> >> > > Why not just use wake_up_process().
> >> >
> >> > IMO this is not an improvement.  There are other places where explicit
> >> > TASK_NORMAL is used as well, and they're all perfectly clear as is.
> >>
> >> Arguably those could all be converted to wake_up_process() as well.
> >> It's a very small kernel code size optimization. There's about 3 such
> >> places, could be converted in a single patch.
> >
> >It's still pointless churn IMO.
>
> Using wake_up_process() is more simpler and friendly for beginners,
> and it is more convenient for analysis and statistics.

If that's your argument, that should have been in the change log. That
said, it's IMO still pretty darn weak. When presenting a patch, do what
Ingo did, show the technical merit, that's what will determine whether
it flies or dies.

	-Mike