Re: [PATCH v3] sched/deadline: remove useless param from setup_new_dl_entity

[Xunlei Pang <xpang@redhat.com>]

On 2016/07/11 at 16:01, luca abeni wrote:
> Hello,
>
> On Mon, 11 Jul 2016 13:03:56 +0800
> Xunlei Pang <xpang@redhat.com> wrote:
>
>> On 2016/07/08 at 19:28, Juri Lelli wrote:
> [...]
>>> @@ -363,6 +364,15 @@ static inline void setup_new_dl_entity(struct
>>> sched_dl_entity *dl_se, return;
>>>  
>>>  	/*
>>> +	 * Use the scheduling parameters of the top pi-waiter task,
>>> +	 * if we have one from which we can inherit a deadline.
>>> +	 */
>>> +	if (dl_se->dl_boosted &&
>>> +	    (pi_task = rt_mutex_get_top_task(dl_task_of(dl_se))) &&
>>> +	    dl_prio(pi_task->normal_prio))
>>> +		pi_se = &pi_task->dl;
>>> +
>>> +	/*
>>>  	 * We use the regular wall clock time to set deadlines in
>>> the
>>>  	 * future; in fact, we must consider execution overheads
>>> (time
>>>  	 * spent on hardirq context, etc.).
>>> @@ -1721,7 +1731,7 @@ static void switched_from_dl(struct rq *rq,
>>> struct task_struct *p) static void switched_to_dl(struct rq *rq,
>>> struct task_struct *p) {
>>>  	if (dl_time_before(p->dl.deadline, rq_clock(rq)))
>>> -		setup_new_dl_entity(&p->dl, &p->dl);
>>> +		setup_new_dl_entity(&p->dl);  
>> I'm curious why we even call setup_new_dl_entity() for non-queued
>> cases? It seems more reasonable to do it when it really gets queued.
>> We can see that enqueue_task_dl()->update_dl_entity() also has the
>> same update logic as switched_to_dl().
> I wondered the same when removing the dl_new field from
> sched_dl_entity... But then I realised that enqueue_dl_entity() does
> not always invoke update_dl_entity() or replenish_dl_entity()... For
> example, when a task switches from SCHED_OTHER (or RT) to -deadline due
> to sched_setattr() (or similar) these functions are not invoked.

Yeah, but for wake-up cases it does, as ENQUEUE_WAKEUP is set.
What I meant is, can we only update for queued tasks in switched_to_dl()?

Regards,
Xunlei

>
>
> 				Luca
>
>> If so, for already queued and boosted cases, rt_mutex_setprio() will
>> call enqueue_task() with ENQUEUE_REPLENISH set, so enqueue_dl_entity()
>> ->replenish_dl_entity() will advance p->dl.deadline beforehand, see
>> code: replenish_dl_entity():
>>     if (dl_se->dl_deadline == 0) {
>>         dl_se->deadline = rq_clock(rq) + pi_se->dl_deadline;
>>         dl_se->runtime = pi_se->dl_runtime;
>>     }
>>
>> IOW, we don't need to handle !dl boosted cases in
>> setup_new_dl_entity().
>>
>> Regards,
>> Xunlei
>>
>>>  
>>>  	if (task_on_rq_queued(p) && rq->curr != p) {
>>>  #ifdef CONFIG_SMP