linux-kernel - Re: [PATCH 06/16] sched: SCHED

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]

Date:	Sun, 08 Apr 2012 22:20:01 +0200
From:	Juri Lelli <juri.lelli@...il.com>
To:	Hillf Danton <dhillf@...il.com>
CC:	peterz@...radead.org, tglx@...utronix.de, mingo@...hat.com,
	rostedt@...dmis.org, oleg@...hat.com, fweisbec@...il.com,
	darren@...art.com, johan.eker@...csson.com, p.faure@...tech.ch,
	linux-kernel@...r.kernel.org, claudio@...dence.eu.com,
	michael@...rulasolutions.com, Fabio Checconi <fchecconi@...il.com>,
	Tommaso Cucinotta <tommaso.cucinotta@...up.it>,
	Juri Lelli <juri.lelli@...il.com>,
	Nicola Manica <nicola.manica@...i.unitn.it>,
	Luca Abeni <luca.abeni@...tn.it>,
	Dhaval Giani <dhaval.giani@...il.com>, hgu1972@...il.com,
	paulmck@...ux.vnet.ibm.com, Dario Faggioli <raistlin@...ux.it>,
	Insop Song <insop.song@...csson.com>, liming.wang@...driver.com
Subject: Re: [PATCH 06/16] sched: SCHED_DEADLINE push and pull logic

On 04/07/2012 04:32 AM, Hillf Danton wrote:
> On Sat, Apr 7, 2012 at 1:31 AM, Juri Lelli<juri.lelli@...il.com>  wrote:
>>>>
>>>>   kernel/sched_dl.c |  912
>>>>   kernel/sched_rt.c |    2 +-
>
> You are working on 2.6.3x, x<= 8 ?
> If so, what is the reason(just curious)?
> Already planned to add in 3.3 and above?
>

Dario answered on this :-).
   
>>>> +               if (!dl_entity_preempt(&entry->dl,&p->dl))
>>>
>>>                 if (dl_entity_preempt(&p->dl,&entry->dl))
>>>
>>
>> Any specific reason to reverse the condition?
>>
> Just for easing readers.
>

Ok, reasonable. Here and below.

>>>> +select_task_rq_dl(struct task_struct *p, int sd_flag, int flags)
>>>> +{
>>>> +       struct task_struct *curr;
>>>> +       struct rq *rq;
>>>> +       int cpu;
>>>> +
>>>> +       if (sd_flag != SD_BALANCE_WAKE)
>>>
>>>                 why is task_cpu(p) not eligible?
>>>
>>
>> Right, I'll change this.
>>
> No, you will first IMO sort out clear answer to the question.
>

task_cpu(p) is eligible and will be returned if sd_flag != SD_BALANCE_WAKE
&& sd_flag != SD_BALANCE_FORK as in sched_rt. I changed the code accordingly.

>>>> +           (rq->curr->dl.nr_cpus_allowed<    2 ||
>>>> +            dl_entity_preempt(&rq->curr->dl,&p->dl))&&
>>>
>>>                 !dl_entity_preempt(&p->dl,&rq->curr->dl))&&
>>
>> As above?
>>
> Just for easing reader.
>
>>>> +#ifdef CONFIG_SMP
>>>> +       /*
>>>> +        * In the unlikely case current and p have the same deadline
>>>> +        * let us try to decide what's the best thing to do...
>>>> +        */
>>>> +       if ((s64)(p->dl.deadline - rq->curr->dl.deadline) == 0&&
>>>> +           !need_resched())
>>>
>>> please recheck !need_resched(), say rq->curr need reschedule?
>>
>> Sorry, I don't get this..
>>
> Perhaps smp_processor_id() != rq->cpu
>

need_resched is actually checked...

>>>
>>>         if (task_running(rq, p))
>>>                 return 0;
>>>         return cpumask_test_cpu(cpu,&p->cpus_allowed);
>>
>> We use this inside pull_dl_task. Since we are searching for a task to
>> pull, you must be sure that the found task can actually migrate checking
>> nr_cpus_allowed>  1.
>>
> If cpu is certainly allowed for task to run, but nr_cpus_allowed is no more
> than one, which is corrupted?
>
>>
>> Well, ok with this and above. Anyway this code is completely removed in
>> 15/16.
>>
> Yup, another reason for monolith.
>

Monolithic is below. Anyway, please check the github repo for bug
fixes/new features. ;-)

>>>> +
>>>> +static DEFINE_PER_CPU(cpumask_var_t, local_cpu_mask_dl);
>>>> +
>>>> +static int find_later_rq(struct task_struct *task)
>>>> +{
>>>> +       struct sched_domain *sd;
>>>> +       struct cpumask *later_mask = __get_cpu_var(local_cpu_mask_dl);
>>>
>>>         please check is local_cpu_mask_dl valid
>>>
>>
>> Could you explain more why should I check for validity?
>>
> Only for the case that something comes in before it is initialized,
> IIRC encountered by Steven.
>

Do you mean at kernel_init time?
Could you be more precise about the problem Steven encountered?

>>
>> Ok, I'll prepare the monolithic patch and probably store it somewhere so
>> that it can be downloaded also by others.
>>
> Info Hillf once it is ready, thanks.
>

Here we go:
https://github.com/downloads/jlelli/sched-deadline/sched-dl-V4.patch

I noticed that the Cc list is changed... something went wrong?
Anyway, I restored it to the original one. :-)

Thanks and Regards,

- 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/