Re: Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()
On Thu, 2021-03-18 at 10:14 +0800, 王擎 wrote: > >> > >> * Mike Galbraith 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
Re: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()
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 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: [PATCH] sched: swait: use wake_up_process() instead of wake_up_state()
On Wed, 2021-03-17 at 10:46 +0100, Ingo Molnar wrote: > * Mike Galbraith 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()
On Wed, Mar 17, 2021 at 10:46:18AM +0100, Ingo Molnar wrote: > > * Mike Galbraith 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 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()
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
> ---
> 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()) {
> curr = list_first_entry(, typeof(*curr), task_list);
>
> - wake_up_state(curr->task, TASK_NORMAL);
> + wake_up_process(curr->task);
> list_del_init(>task_list);
>
> if (list_empty())
