linux-kernel - Re: [PATCH v2 -tip] sched/deadline: switched_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52E7889E.9030706@gmail.com>
Date:	Tue, 28 Jan 2014 11:38:22 +0100
From:	Juri Lelli <juri.lelli@...il.com>
To:	Kirill Tkhai <ktkhai@...allels.com>, linux-kernel@...r.kernel.org
CC:	peterz@...radead.org, mingo@...nel.org, devel@...nvz.org
Subject: Re: [PATCH v2 -tip] sched/deadline: switched_to_dl() -- skip if task
 is current

On 01/28/2014 08:26 AM, Kirill Tkhai wrote:
> v2: Changed comment
> 
> When p is current and it's not of dl class, then there are no other
> dl taks in the rq. If we had had pushable tasks in some other rq,
       ^ tasks

> they would have been pushed earlier. So, skip "p == rq->curr" case.
> 
> [This is confirmed by Juri Lelli and LKML was CC'ed, but
>  unfotunately I can't find direct link on lkml.org]
> 

This was the story:

> On 01/08/2014 10:00 AM, Juri Lelli wrote:
>> On 12/18/2013 04:00 PM, Kirill Tkhai wrote:
>>> 17.12.2013, 16:48, "Peter Zijlstra" <peterz@...radead.org>:
>>> From: Dario Faggioli <raistlin@...ux.it>
>>>
>>> Introduces the data structures, constants and symbols needed for
>>> SCHED_DEADLINE implementation.
>>
>> [snipped]
>>
>>> +static void switched_to_dl(struct rq *rq, struct task_struct *p)
>>> +{
>>> +	/*
>>> +	 * If p is throttled, don't consider the possibility
>>> +	 * of preempting rq->curr, the check will be done right
>>> +	 * after its runtime will get replenished.
>>> +	 */
>>> +	if (unlikely(p->dl.dl_throttled))
>>> +		return;
>>> +
>>> +	if (!p->on_rq || rq->curr != p) {
>>> +		if (task_has_dl_policy(rq->curr))
>>> +			check_preempt_curr_dl(rq, p, 0);
>>> +		else
>>> +			resched_task(rq->curr);
>>> +	}
>>> +}
>>
>> The second if() looks a little strange. Why is "!p->on_rq ||" here? RT class
>> has another logic.
>>
>
> You are right, good catch! :)
>
> This has to be changed in
>
>   if (p->on_rq && rq->curr != p)
>
> as in RT.
>
> Thanks,
>
> - Juri

> Signed-off-by: Kirill Tkhai <ktkhai@...allels.com>
> CC: Juri Lelli <juri.lelli@...il.com>
> CC: Peter Zijlstra <peterz@...radead.org>
> CC: Ingo Molnar <mingo@...nel.org>
> ---
>  kernel/sched/deadline.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c
> index 0de2482..dd19d6d 100644
> --- a/kernel/sched/deadline.c
> +++ b/kernel/sched/deadline.c
> @@ -1559,7 +1559,7 @@ static void switched_to_dl(struct rq *rq, struct task_struct *p)
>  	if (unlikely(p->dl.dl_throttled))
>  		return;
>  
> -	if (p->on_rq || rq->curr != p) {
> +	if (p->on_rq && rq->curr != p) {
>  #ifdef CONFIG_SMP
>  		if (rq->dl.overloaded && push_dl_task(rq) && rq != task_rq(p))
>  			/* Only reschedule if pushing failed */
> 

So the patch looks good. Not sure about the changelog, though :).

Thanks,

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