All of lore.kernel.org
 help / color / mirror / Atom feed
From: chris hyser <chris.hyser@oracle.com>
To: Qais Yousef <qais.yousef@arm.com>, Parth Shah <parth@linux.ibm.com>
Cc: vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	patrick.bellasi@matbug.net, valentin.schneider@arm.com,
	tim.c.chen@linux.intel.com, linux-kernel@vger.kernel.org,
	peterz@infradead.org, mingo@redhat.com, David.Laight@ACULAB.COM,
	pjt@google.com, pavel@ucw.cz, tj@kernel.org,
	dhaval.giani@oracle.com, qperret@google.com
Subject: Re: [PATCH v4 4/4] sched/core: Add permission checks for setting the latency_nice value
Date: Thu, 27 Feb 2020 09:46:31 -0500	[thread overview]
Message-ID: <23524cda-f2fb-b2cd-1dcb-93672022c684@oracle.com> (raw)
In-Reply-To: <20200227114401.uz5p2e7mcomzoo5k@e107158-lin.cambridge.arm.com>

On 2/27/20 6:44 AM, Qais Yousef wrote:
> On 02/25/20 12:17, Parth Shah wrote:
>>
>>
>> On 2/24/20 6:59 PM, Qais Yousef wrote:
>>> On 02/24/20 14:29, Parth Shah wrote:
>>>> Since the latency_nice uses the similar infrastructure as NICE, use the
>>>> already existing CAP_SYS_NICE security checks for the latency_nice. This
>>>> should return -EPERM for the non-root user when trying to set the task
>>>> latency_nice value to any lower than the current value.
>>>>
>>>> Signed-off-by: Parth Shah <parth@linux.ibm.com>
>>>
>>> I'm not against this, so I'm okay if it goes in as is.
>>>
>>> But IMO the definition of this flag is system dependent and I think it's
>>> prudent to keep it an admin only configuration.
>>>
>>> It'd be hard to predict how normal application could use and depend on this
>>> feature in the future, which could tie our hand in terms of extending it.
>>>
>>
>> I am fine with this going in too. But just to lie down the fact on single
>> page and starting the discussion, here are the pros and cons for including
>> this permission checks:
>>
>> Pros:
>> =====
>> - Having this permission checks will allow only root users to promote the
>> task, meaning lowering the latency_nice of the task. This is required in
>> case when the admin has increased the latency_nice value of a task and
>> non-root user can not lower it.
>> - In absence of this check, the non-root user can decrease the latency_nice
>> value against the admin configured value.
>>
>> Cons:
>> =====
>> - This permission check prevents the non-root user to lower the value. This
>> is a problem when the user itself has increased the latency_nice value in
>> the past but fails to lower it again.
>> - After task fork, non-root user cannot lower the inherited child task's
>> latency_nice value, which might be a problem in the future for extending
>> this latency_nice ideas for different optimizations.
> 
> Worth adding that if we start strict with root (or capable user) only, relaxing
> this to allow lowering the nice would still be possible in the future. But the
> opposite is not true, we can't reverse the users ability to lower its
> latency_nice value once we give it away.
> 
> Beside thinking a bit more about it now. If high latency_nice value means
> cutting short the idle search for instance, does this prevent someone using
> a lower latency nice to be aggressive in some code path to get higher
> throughput?

Short-cutting an idle cpu search reduces latency. There would be a mapping between the latency_nice values -20..-1 and 
the short cut. In that view 0 is the default and performs the full domain search as before and -20 presumably skips the 
entire search. Positive values then presumably indicate a trade-off in preference of throughput. I've not thought any 
about it till now, but maybe indicates that spending extra time (versus less) finding this task the perfect home to just 
sit and crank on throughput would be worth it.

-chrish

  reply	other threads:[~2020-02-27 14:48 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-24  8:59 [PATCH v4 0/4] Introduce per-task latency_nice for scheduler hints Parth Shah
2020-02-24  8:59 ` [PATCH v4 1/4] sched: Introduce latency-nice as a per-task attribute Parth Shah
2020-02-24  8:59 ` [PATCH v4 2/4] sched/core: Propagate parent task's latency requirements to the child task Parth Shah
2020-02-25  6:32   ` Pavan Kondeti
2020-02-25  8:16     ` Parth Shah
2020-02-24  8:59 ` [PATCH v4 3/4] sched: Allow sched_{get,set}attr to change latency_nice of the task Parth Shah
2020-02-25  6:54   ` Pavan Kondeti
2020-02-25 15:03     ` Parth Shah
2020-02-26  3:44       ` Pavan Kondeti
2020-02-24  8:59 ` [PATCH v4 4/4] sched/core: Add permission checks for setting the latency_nice value Parth Shah
2020-02-24 13:29   ` Qais Yousef
2020-02-25  6:47     ` Parth Shah
2020-02-27 11:44       ` Qais Yousef
2020-02-27 14:46         ` chris hyser [this message]
2020-02-24 23:08   ` chris hyser

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23524cda-f2fb-b2cd-1dcb-93672022c684@oracle.com \
    --to=chris.hyser@oracle.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=dhaval.giani@oracle.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=parth@linux.ibm.com \
    --cc=patrick.bellasi@matbug.net \
    --cc=pavel@ucw.cz \
    --cc=peterz@infradead.org \
    --cc=pjt@google.com \
    --cc=qais.yousef@arm.com \
    --cc=qperret@google.com \
    --cc=tim.c.chen@linux.intel.com \
    --cc=tj@kernel.org \
    --cc=valentin.schneider@arm.com \
    --cc=vincent.guittot@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.