Re: [PATCH v5] sched/deadline: fix earliest_dl.next logic

[Wanpeng Li]

2015-12-02 22:08 GMT+08:00 Luca Abeni <luca.abeni@xxxxxxxx>:
> Hi,
>
> On 12/02/2015 02:33 PM, Wanpeng Li wrote:
> [...]
>>>
>>> We updated leftmost above, can't we simply use that path for this thing
>>> below?
>>
>>
>> Do you mean something like below?
>>
>> @@ -195,6 +195,9 @@ static void dequeue_pushable_dl_task(struct rq *rq,
>> struct task_struct *p)
>>
>>                  next_node = rb_next(&p->pushable_dl_tasks);
>>                  dl_rq->pushable_dl_tasks_leftmost = next_node;
>> +               if (has_pushable_dl_tasks(rq))
>
> I do not know the rb trees code, but... Are you sre you can call
> has_pushable_tasks() here?
> (I suspect pushable_dl_tasks_root is not updated yet, so maybe
> has_pushable_dl_tasks() risks
> to return a wrong value?)

Right.

>
>> +                       dl_rq->earliest_dl.next =
>> rb_entry(rq->dl.pushable_dl_tasks_leftmost,
>> +                               struct task_struct,
>> pushable_dl_task)->dl.deadline;
>
> I am not sure if this is what Juri meant, but maybe something like this?
>
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 087d090..26d3279 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -185,11 +185,6 @@ static void enqueue_pushable_dl_task(struct rq *rq,
> struct task_struct *p)
>         rb_insert_color(&p->pushable_dl_tasks,
> &dl_rq->pushable_dl_tasks_root);
>  }
>
> -static inline int has_pushable_dl_tasks(struct rq *rq)
> -{
> -       return !RB_EMPTY_ROOT(&rq->dl.pushable_dl_tasks_root);
> -}
> -
>  static void dequeue_pushable_dl_task(struct rq *rq, struct task_struct *p)
>  {
>         struct dl_rq *dl_rq = &rq->dl;
> @@ -202,16 +197,18 @@ static void dequeue_pushable_dl_task(struct rq *rq,
> struct task_struct *p)
>
>                 next_node = rb_next(&p->pushable_dl_tasks);
>                 dl_rq->pushable_dl_tasks_leftmost = next_node;
> +               if (next_node)
> +                       dl_rq->earliest_dl.next = rb_entry(next_node,
> +                            struct task_struct,
> pushable_dl_tasks)->dl.deadline;

Juri mentioned "updated leftmost", I'm not sure if it means that:

@@ -195,6 +195,9 @@ static void dequeue_pushable_dl_task(struct rq
*rq, struct task_struct *p)

                next_node = rb_next(&p->pushable_dl_tasks);
                dl_rq->pushable_dl_tasks_leftmost = next_node;
+               if (dl_rq->pushable_dl_tasks_leftmost)
+                       dl_rq->earliest_dl.next =
rb_entry(dl_rq->pushable_dl_tasks_leftmost,
+                               struct task_struct,
pushable_dl_tasks)->dl.deadline;
        }


Regards,
Wanpeng Li
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/