* Kirill Tkhai <[email protected]> wrote:
>
> Implement task_queued() and use it everywhere instead of on_rq check.
> No functional changes.
>
> The only exception is we do not use the wrapper in check_for_tasks(),
> because it requires to export task_queued() in global header files.
> Next patch in series would return it back, so it doesn't matter.
>
> Signed-off-by: Kirill Tkhai <[email protected]>
> ---
> kernel/sched/core.c | 82
> +++++++++++++++++++++++-----------------------
> kernel/sched/deadline.c | 14 ++++----
> kernel/sched/fair.c | 22 ++++++------
> kernel/sched/rt.c | 16 ++++-----
> kernel/sched/sched.h | 7 ++++
> kernel/sched/stop_task.c | 2 +
> 6 files changed, 75 insertions(+), 68 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 1211575..67e8d1e 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1043,7 +1043,7 @@ void check_preempt_curr(struct rq *rq, struct
> task_struct *p, int flags)
> * A queue event has occurred, and we're going to schedule. In
> * this case, we can save a useless back to back clock update.
> */
> - if (rq->curr->on_rq && test_tsk_need_resched(rq->curr))
> + if (task_queued(rq->curr) && test_tsk_need_resched(rq->curr))
> rq->skip_clock_update = 1;
> - p->on_rq = 1;
> + p->on_rq = ONRQ_QUEUED;
> --- a/kernel/sched/sched.h
> +++ b/kernel/sched/sched.h
> @@ -15,6 +15,9 @@
>
> struct rq;
>
> +/* task_struct::on_rq states: */
> +#define ONRQ_QUEUED 1
> +
> extern __read_mostly int scheduler_running;
>
> extern unsigned long calc_load_update;
> @@ -942,6 +945,10 @@ static inline int task_running(struct rq *rq, struct
> task_struct *p)
> #endif
> }
>
> +static inline int task_queued(struct task_struct *p)
> +{
> + return p->on_rq == ONRQ_QUEUED;
> +}
So I agree with splitting p->on_rq into more states, but the
new naming used looks pretty random, we can and should do
better.
For example 'task_queued()' gives very little clue that it's
all about the p->on_rq state. The 'ONRQ_QUEUED' name does not
signal that this is a task's scheduler internal state, etc.
So I'd suggest a more structured naming scheme, something along
the lines of:
TASK_ON_RQ_QUEUED
TASK_ON_RQ_MIGRATING
task_on_rq_queued()
task_on_rq_migrating()
etc.
It's a bit longer, but also more logical and thus easier to
read and maintain.
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
