All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Quentin Perret <qperret@google.com>
To: Qais Yousef <qais.yousef@arm.com>
Cc: mingo@redhat.com, peterz@infradead.org,
	vincent.guittot@linaro.org, dietmar.eggemann@arm.com,
	rickyiu@google.com, wvw@google.com, patrick.bellasi@matbug.net,
	xuewen.yan94@gmail.com, linux-kernel@vger.kernel.org,
	kernel-team@android.com
Subject: Re: [PATCH v2 3/3] sched: Make uclamp changes depend on CAP_SYS_NICE
Date: Fri, 11 Jun 2021 13:08:15 +0000	[thread overview]
Message-ID: <YMNgPyfiIaIIsjqq@google.com> (raw)
In-Reply-To: <20210611124820.ksydlg4ncw2xowd3@e107158-lin.cambridge.arm.com>

Hi Qais,

On Friday 11 Jun 2021 at 13:48:20 (+0100), Qais Yousef wrote:
> On 06/10/21 15:13, Quentin Perret wrote:
> > There is currently nothing preventing tasks from changing their per-task
> > clamp values in anyway that they like. The rationale is probably that
> > system administrators are still able to limit those clamps thanks to the
> > cgroup interface. However, this causes pain in a system where both
> > per-task and per-cgroup clamp values are expected to be under the
> > control of core system components (as is the case for Android).
> > 
> > To fix this, let's require CAP_SYS_NICE to increase per-task clamp
> > values. This allows unprivileged tasks to lower their requests, but not
> > increase them, which is consistent with the existing behaviour for nice
> > values.
> 
> Hmmm. I'm not in favour of this.
> 
> So uclamp is a performance and power management mechanism, it has no impact on
> fairness AFAICT, so it being a privileged operation doesn't make sense.
> 
> We had a thought about this in the past and we didn't think there's any harm if
> a task (app) wants to self manage. Yes a task could ask to run at max
> performance and waste power, but anyone can generate a busy loop and waste
> power too.
> 
> Now that doesn't mean your use case is not valid. I agree if there's a system
> wide framework that wants to explicitly manage performance and power of tasks
> via uclamp, then we can end up with 2 layers of controls overriding each
> others.

Right, that's the main issue. Also, the reality is that most of time the
'right' clamps are platform-dependent, so most userspace apps are simply
not equipped to decide what their own clamps should be.

> Would it make more sense to have a procfs/sysfs flag that is disabled by
> default that allows sys-admin to enforce a privileged uclamp access?
> 
> Something like
> 
> 	/proc/sys/kernel/sched_uclamp_privileged

Hmm, dunno, I'm not aware of anything else having a behaviour like that,
so that feels a bit odd.

> I think both usage scenarios are valid and giving sys-admins the power to
> enforce a behavior makes more sense for me.

Yes, I wouldn't mind something like that in general. I originally wanted
to suggest introducing a dedicated capability for uclamp, but that felt
a bit overkill. Now if others think this should be the way to go I'm
happy to go implement it.

Thanks,
Quentin

  reply	other threads:[~2021-06-11 13:09 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-10 15:13 [PATCH v2 0/3] A few uclamp fixes Quentin Perret
2021-06-10 15:13 ` [PATCH v2 1/3] sched: Fix UCLAMP_FLAG_IDLE setting Quentin Perret
2021-06-10 19:05   ` Peter Zijlstra
2021-06-11  7:25     ` Quentin Perret
2021-06-17 15:27       ` Dietmar Eggemann
2021-06-21 10:57         ` Quentin Perret
2021-06-10 15:13 ` [PATCH v2 2/3] sched: Skip priority checks with SCHED_FLAG_KEEP_PARAMS Quentin Perret
2021-06-10 19:15   ` Peter Zijlstra
2021-06-11  8:59     ` Quentin Perret
2021-06-11  9:07       ` Quentin Perret
2021-06-11  9:20       ` Peter Zijlstra
2021-06-10 15:13 ` [PATCH v2 3/3] sched: Make uclamp changes depend on CAP_SYS_NICE Quentin Perret
2021-06-11 12:48   ` Qais Yousef
2021-06-11 13:08     ` Quentin Perret [this message]
2021-06-11 13:26       ` Qais Yousef
2021-06-11 13:49         ` Quentin Perret
2021-06-11 14:17           ` Qais Yousef
2021-06-11 14:43             ` Quentin Perret
2021-06-14 15:03               ` Qais Yousef
2021-06-21 10:52                 ` Quentin Perret

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=YMNgPyfiIaIIsjqq@google.com \
    --to=qperret@google.com \
    --cc=dietmar.eggemann@arm.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=patrick.bellasi@matbug.net \
    --cc=peterz@infradead.org \
    --cc=qais.yousef@arm.com \
    --cc=rickyiu@google.com \
    --cc=vincent.guittot@linaro.org \
    --cc=wvw@google.com \
    --cc=xuewen.yan94@gmail.com \
    /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.