linux-kernel - Re: [PATCH v2 0/4] PM: QoS: Introduce a CPU system-wakeup QoS limit for s2idle

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: <CAJZ5v0jdT28Q0mnyU_2RKEYWhEHhQvWBAzsra1Jp9dBCLSCO1Q@mail.gmail.com>
Date: Thu, 30 Oct 2025 17:36:02 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, linux-pm@...r.kernel.org, 
	Vincent Guittot <vincent.guittot@...aro.org>, Peter Zijlstra <peterz@...radead.org>, 
	Kevin Hilman <khilman@...libre.com>, Pavel Machek <pavel@...nel.org>, Len Brown <len.brown@...el.com>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Saravana Kannan <saravanak@...gle.com>, 
	Maulik Shah <quic_mkshah@...cinc.com>, Prasad Sodagudi <psodagud@...cinc.com>, 
	Dhruva Gole <d-gole@...com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/4] PM: QoS: Introduce a CPU system-wakeup QoS limit
 for s2idle

On Thu, Oct 30, 2025 at 4:13 PM Ulf Hansson <ulf.hansson@...aro.org> wrote:
>
> On Thu, 30 Oct 2025 at 15:06, Rafael J. Wysocki <rafael@...nel.org> wrote:
> >
> > On Thu, Oct 30, 2025 at 1:44 PM Ulf Hansson <ulf.hansson@...aro.org> wrote:
> > >
> > > On Thu, 30 Oct 2025 at 13:29, Rafael J. Wysocki <rafael@...nel.org> wrote:
> > > >
> > > > On Thu, Oct 30, 2025 at 1:26 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
> > > > >
> > > > > On Thu, Oct 30, 2025 at 1:23 PM Ulf Hansson <ulf.hansson@...aro.org> wrote:
> > > > > >
> > > > > > On Wed, 29 Oct 2025 at 15:53, Rafael J. Wysocki <rafael@...nel.org> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 16, 2025 at 5:19 PM Ulf Hansson <ulf.hansson@...aro.org> wrote:
> > > > > > > >
> > > > > > > > Changes in v2:
> > > > > > > >         - Limit the new QoS to CPUs  and make some corresponding renaming of the
> > > > > > > >         functions along with name of the device node for user space.
> > > > > > > >         - Make sure we deal with the failure/error path correctly when there are
> > > > > > > >         no state available for s2idle.
> > > > > > > >         - Add documentation.
> > > > > > > >
> > > > > > > > Some platforms supports multiple low-power states for CPUs that can be used
> > > > > > > > when entering system-wide suspend and s2idle in particular. Currently we are
> > > > > > > > always selecting the deepest possible state for the CPUs, which can break the
> > > > > > > > system-wakeup latency constraint that may be required for some use-cases.
> > > > > > > >
> > > > > > > > Therefore, this series suggests to introduce a new interface for user-space,
> > > > > > > > allowing us to specify the CPU system-wakeup QoS limit. The QoS limit is then
> > > > > > > > taken into account when selecting a suitable low-power state for s2idle.
> > > > > > >
> > > > > > > Last time we discussed this I said I would like the new limit to be
> > > > > > > taken into account by regular "runtime" cpuidle because the "s2idle"
> > > > > > > limit should not be less that the "runtime" limit (or at least it
> > > > > > > would be illogical if that happened).
> > > > > >
> > > > > > Yes, we discussed this, but that was also before we concluded to add a
> > > > > > new file for user-space to operate on after all.
> > > > > >
> > > > > > To me, it looks unnecessarily limiting to not allow them to be
> > > > > > orthogonal,
> > > > >
> > > > > So what's the use case in which it makes sense to have a lower latency
> > > > > limit for s2idle than for runtime?
> > >
> > > Honestly, I don't know, but I just wanted to keep things more flexible.
> > >
> > > > >
> > > > > > but I am not insisting that it needs to be like this. I
> > > > > > was just thinking that we do not necessarily have to care about the
> > > > > > same use-case in runtime as in the system-suspend state. Moreover,
> > > > > > nothing would prevent user-space from applying the same constraint to
> > > > > > both of them, if that is needed.
> > > > > >
> > > > > > >
> > > > > > > It looks like that could be implemented by making
> > > > > > > cpuidle_governor_latency_req() take cpu_wakeup_latency_qos_limit()
> > > > > > > into account, couldn't it?
> > > > > >
> > > > > > Right, but I am not sure we want that. See above.
> > > > >
> > > > > I do or I need to be convinced that this is a bad idea.
> > > >
> > > > And there is a specific reason why I want that.
> > > >
> > > > Namely, say somebody wants to set the same limit for both s2idle and
> > > > "runtime" cpuidle.  If the s2idle limit did not affect "runtime", they
> > > > would need to open two device special files and write the same value
> > > > to both of them.  Otherwise, they just need to use the s2idle limit
> > > > and it will work for "runtime" automatically.
> > >
> > > Right. User-space would need to open two files instead of one, but is
> > > that really a problem?
> >
> > It is potentially confusing and error-prone.
> >
> > > What if user-space doesn't want to affect the runtime state-selection,
> > > but cares only about a use-case that requires a cpu-wakeup constraint
> > > when resuming from s2idle.
> >
> > Well, I'm not sure if this use case exists at all, which is key here.
> > If it doesn't exist, why make provisions for it?
>
> Well, because it's not possible to change afterwards as it becomes ABI.
>
> It would be silly having to add yet another file for userspace, down
> the road, if it turns out to be needed.

OTOH doing things without a good reason is also not particularly wise.