Re: [PATCH 09/11] sched_ext: Replace consume_local_task() with move_local_task_to_local_dsq()

David Vernet
2024-09-01T01:55:57Z

[-- Attachment #1: Type: text/plain, Size: 4281 bytes --]

On Fri, Aug 30, 2024 at 01:03:53AM -1000, Tejun Heo wrote:
> - Rename move_task_to_local_dsq() to move_remote_task_to_local_dsq().
>
> - Rename consume_local_task() to move_local_task_to_local_dsq() and remove
> task_unlink_from_dsq() and source DSQ unlocking from it.
>
> This is to make the migration code easier to reuse.
>
> No functional changes intended.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>

Acked-by: David Vernet <void@manifault.com>

> ---
> kernel/sched/ext.c | 42 ++++++++++++++++++++++++++----------------
> 1 file changed, 26 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 51d141602a11..df33524d68f3 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -2103,23 +2103,30 @@ static bool yield_to_task_scx(struct rq *rq, struct task_struct *to)
> return false;
> }
>
> -static void consume_local_task(struct task_struct *p,
> - struct scx_dispatch_q *dsq, struct rq *rq)
> +static void move_local_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
> + struct scx_dispatch_q *src_dsq,
> + struct rq *dst_rq)
> {
> - lockdep_assert_held(&dsq->lock); /* released on return */
> + struct scx_dispatch_q *dst_dsq = &dst_rq->scx.local_dsq;
> +
> + /* @dsq is locked and @p is on @dst_rq */
> + lockdep_assert_held(&src_dsq->lock);
> + lockdep_assert_rq_held(dst_rq);
>
> - /* @dsq is locked and @p is on this rq */
> WARN_ON_ONCE(p->scx.holding_cpu >= 0);
> - task_unlink_from_dsq(p, dsq);
> - list_add_tail(&p->scx.dsq_list.node, &rq->scx.local_dsq.list);
> - dsq_mod_nr(&rq->scx.local_dsq, 1);
> - p->scx.dsq = &rq->scx.local_dsq;
> - raw_spin_unlock(&dsq->lock);
> +
> + if (enq_flags & (SCX_ENQ_HEAD | SCX_ENQ_PREEMPT))
> + list_add(&p->scx.dsq_list.node, &dst_dsq->list);
> + else
> + list_add_tail(&p->scx.dsq_list.node, &dst_dsq->list);
> +
> + dsq_mod_nr(dst_dsq, 1);
> + p->scx.dsq = dst_dsq;
> }
>
> #ifdef CONFIG_SMP
> /**
> - * move_task_to_local_dsq - Move a task from a different rq to a local DSQ
> + * move_remote_task_to_local_dsq - Move a task from a foreign rq to a local DSQ
> * @p: task to move
> * @enq_flags: %SCX_ENQ_*
> * @src_rq: rq to move the task from, locked on entry, released on return
> @@ -2127,8 +2134,8 @@ static void consume_local_task(struct task_struct *p,
> *
> * Move @p which is currently on @src_rq to @dst_rq's local DSQ.
> */
> -static void move_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
> - struct rq *src_rq, struct rq *dst_rq)
> +static void move_remote_task_to_local_dsq(struct task_struct *p, u64 enq_flags,
> + struct rq *src_rq, struct rq *dst_rq)
> {
> lockdep_assert_rq_held(src_rq);
>
> @@ -2251,7 +2258,7 @@ static bool consume_remote_task(struct rq *this_rq, struct task_struct *p,
> raw_spin_rq_unlock(this_rq);
>
> if (unlink_dsq_and_lock_task_rq(p, dsq, task_rq)) {
> - move_task_to_local_dsq(p, 0, task_rq, this_rq);
> + move_remote_task_to_local_dsq(p, 0, task_rq, this_rq);
> return true;
> } else {
> raw_spin_rq_unlock(task_rq);
> @@ -2282,7 +2289,9 @@ static bool consume_dispatch_q(struct rq *rq, struct scx_dispatch_q *dsq)
> struct rq *task_rq = task_rq(p);
>
> if (rq == task_rq) {
> - consume_local_task(p, dsq, rq);
> + task_unlink_from_dsq(p, dsq);
> + move_local_task_to_local_dsq(p, 0, dsq, rq);
> + raw_spin_unlock(&dsq->lock);
> return true;
> }
>
> @@ -2362,13 +2371,14 @@ static void dispatch_to_local_dsq(struct rq *rq, struct scx_dispatch_q *dst_dsq,
> /*
> * If @p is staying on the same rq, there's no need to go
> * through the full deactivate/activate cycle. Optimize by
> - * abbreviating the operations in move_task_to_local_dsq().
> + * abbreviating move_remote_task_to_local_dsq().
> */
> if (src_rq == dst_rq) {
> p->scx.holding_cpu = -1;
> dispatch_enqueue(&dst_rq->scx.local_dsq, p, enq_flags);
> } else {
> - move_task_to_local_dsq(p, enq_flags, src_rq, dst_rq);
> + move_remote_task_to_local_dsq(p, enq_flags,
> + src_rq, dst_rq);
> }
>
> /* if the destination CPU is idle, wake it up */
> --
> 2.46.0
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]