linux-kernel - Re: [PATCH v3 0/3] Introduce per-task latency

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:   Thu, 20 Feb 2020 11:34:57 -0500
From:   chris hyser <chris.hyser@...cle.com>
To:     Qais Yousef <qais.yousef@....com>
Cc:     Parth Shah <parth@...ux.ibm.com>, vincent.guittot@...aro.org,
        patrick.bellasi@...bug.net, valentin.schneider@....com,
        dhaval.giani@...cle.com, dietmar.eggemann@....com,
        linux-kernel@...r.kernel.org, peterz@...radead.org,
        mingo@...hat.com, pavel@....cz, qperret@...rret.net,
        David.Laight@...LAB.COM, pjt@...gle.com, tj@...nel.org
Subject: Re: [PATCH v3 0/3] Introduce per-task latency_nice for scheduler
 hints

On 2/20/20 10:03 AM, Qais Yousef wrote:
> On 02/20/20 09:30, chris hyser wrote:
>>> The below diff works out well enough in-order to align permission checks
>>> with NICE.
>>>
>>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>>> index 2bfcff5623f9..ef4a397c9170 100644
>>> --- a/kernel/sched/core.c
>>> +++ b/kernel/sched/core.c
>>> @@ -4878,6 +4878,10 @@ static int __sched_setscheduler(struct task_struct *p,
>>>                           return -EINVAL;
>>>                   if (attr->sched_latency_nice < MIN_LATENCY_NICE)
>>>                           return -EINVAL;
>>> +               /* Use the same security checks as NICE */
>>> +               if (attr->sched_latency_nice < p->latency_nice &&
>>> +                   !can_nice(p, attr->sched_latency_nice))
>>> +                       return -EPERM;
>>>           }
>>>
>>>           if (pi)
>>>
>>> With the above in effect,
>>> A non-root user can only increase the value upto +19, and once increased
>>> cannot be decreased. e.g., a user once sets the value latency_nice = 19,
>>> the same user cannot set the value latency_nice = 18. This is the same
>>> effect as with NICE.
>>>
>>> Is such permission checks required?
>>>
>>> Unlike NICE, we are going to use latency_nice for scheduler hints only, and
>>> so won't it make more sense to allow a user to increase/decrease the values
>>> of their owned tasks?
>>
>> Whether called a hint or not, it is a trade-off to reduce latency of select
>> tasks at the expense of the throughput of the other tasks in the the system.
> 
> Does it actually affect the throughput of the other tasks? I thought this will
> allow the scheduler to reduce latencies, for instance, when selecting which cpu
> it should land on. I can't see how this could hurt other tasks.

This is why it is hard to argue about pure abstractions. The primary idea mentioned so far for how these latencies are 
reduced is by short cutting the brute-force search for something idle. If you don't find an idle cpu because you didn't 
spend the time to look, then you pre-empted a task, possibly with a large nice warm cache footprint that was cranking 
away on throughput. It is ultimately going to be the usual latency vs throughput trade off. If latency reduction were 
"free" we wouldn't need a per task attribute. We would just do the reduction for all tasks, everywhere, all the time.

> 
> Can you expand on the scenario you have in mind please?

Hopefully, the above helps. It was my original plan to introduce this with a data laden RFC on the topic, but I felt the 
need to respond to Parth immediately. I'm not currently pushing any particular change.

-chrish