| 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
| ||
|
Message-ID: <20251029081014.443psmqznd2pqm4i@lcpd911>
Date: Wed, 29 Oct 2025 13:40:14 +0530
From: Dhruva Gole <d-gole@...com>
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>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/4] PM: QoS: Introduce a CPU system-wakeup QoS limit
Hi Ulf,
On Oct 16, 2025 at 17:19:21 +0200, Ulf Hansson wrote:
> Some platforms supports multiple low-power states for CPUs that can be used
> when entering system-wide suspend. 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.
>
> Let's take the first step towards addressing this problem, by introducing
> an interface for user-space, that allows us to specify the CPU
> system-wakeup QoS limit. Subsequent changes will start taking into account
> the new QoS limit.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>
> ---
>
> Changes in v2:
> - Renamings to reflect the QoS are limited to CPUs.
> - Move code inside "CONFIG_CPU_IDLE".
>
> ---
> include/linux/pm_qos.h | 5 ++
> kernel/power/qos.c | 102 +++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 107 insertions(+)
>
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 4a69d4af3ff8..bf7524d38933 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -149,6 +149,7 @@ bool cpu_latency_qos_request_active(struct pm_qos_request *req);
> void cpu_latency_qos_add_request(struct pm_qos_request *req, s32 value);
> void cpu_latency_qos_update_request(struct pm_qos_request *req, s32 new_value);
> void cpu_latency_qos_remove_request(struct pm_qos_request *req);
> +s32 cpu_wakeup_latency_qos_limit(void);
> #else
> static inline s32 cpu_latency_qos_limit(void) { return INT_MAX; }
> static inline bool cpu_latency_qos_request_active(struct pm_qos_request *req)
> @@ -160,6 +161,10 @@ static inline void cpu_latency_qos_add_request(struct pm_qos_request *req,
> static inline void cpu_latency_qos_update_request(struct pm_qos_request *req,
> s32 new_value) {}
> static inline void cpu_latency_qos_remove_request(struct pm_qos_request *req) {}
> +static inline s32 cpu_wakeup_latency_qos_limit(void)
> +{
> + return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> +}
> #endif
>
> #ifdef CONFIG_PM
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 4244b069442e..8c024d7dc43e 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -415,6 +415,103 @@ static struct miscdevice cpu_latency_qos_miscdev = {
> .fops = &cpu_latency_qos_fops,
> };
>
> +/* The CPU system wakeup latency QoS. */
> +static struct pm_qos_constraints cpu_wakeup_latency_constraints = {
> + .list = PLIST_HEAD_INIT(cpu_wakeup_latency_constraints.list),
> + .target_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT,
> + .default_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT,
> + .no_constraint_value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT,
> + .type = PM_QOS_MIN,
> +};
> +
> +/**
> + * cpu_wakeup_latency_qos_limit - Current CPU system wakeup latency QoS limit.
> + *
> + * Returns the current CPU system wakeup latency QoS limit that may have been
> + * requested by user-space.
> + */
> +s32 cpu_wakeup_latency_qos_limit(void)
> +{
> + return pm_qos_read_value(&cpu_wakeup_latency_constraints);
> +}
> +
> +static int cpu_wakeup_latency_qos_open(struct inode *inode, struct file *filp)
> +{
> + struct pm_qos_request *req;
> +
> + req = kzalloc(sizeof(*req), GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + req->qos = &cpu_wakeup_latency_constraints;
> + pm_qos_update_target(req->qos, &req->node, PM_QOS_ADD_REQ,
> + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT);
> + filp->private_data = req;
> +
> + return 0;
> +}
> +
> +static int cpu_wakeup_latency_qos_release(struct inode *inode,
> + struct file *filp)
> +{
> + struct pm_qos_request *req = filp->private_data;
> +
> + filp->private_data = NULL;
> + pm_qos_update_target(req->qos, &req->node, PM_QOS_REMOVE_REQ,
> + PM_QOS_RESUME_LATENCY_NO_CONSTRAINT);
Please excuse the delay in reviewing these patches,
I was wondering why we have decided here in release to reset the
constraints set by a user. For eg. even when I was testing the previous
revision locally I'd just commented out this release hook, since I
wanted to be able to just echo 0xABCD into /dev/cpu_wakeup_latency...
It seems an overkill to me that a userspace program be required to hold
open this file just to make sure the constraints are honoured for the
lifetime of the device. We should definitely give the freedom to just be
able to echo and also be able to cat and read back from the same place
about the latency constraint being set.
One other thing on my mind is - and probably unrelated to this specific
series, but I think we must have some sysfs entry either appear in
/sys/.../cpu0/cpuidle or s2idle/ where we can show next feesible s2idle
state that the governor has chosen to enter based on the value set in
cpu_wakeup_latency.
Thoughts?
> + kfree(req);
> +
> + return 0;
> +}
> +
> +static ssize_t cpu_wakeup_latency_qos_read(struct file *filp, char __user *buf,
> + size_t count, loff_t *f_pos)
> +{
> + s32 value = pm_qos_read_value(&cpu_wakeup_latency_constraints);
> +
> + return simple_read_from_buffer(buf, count, f_pos, &value, sizeof(s32));
> +}
> +
> +static ssize_t cpu_wakeup_latency_qos_write(struct file *filp,
> + const char __user *buf,
> + size_t count, loff_t *f_pos)
> +{
> + struct pm_qos_request *req = filp->private_data;
> + s32 value;
> +
> + if (count == sizeof(s32)) {
> + if (copy_from_user(&value, buf, sizeof(s32)))
> + return -EFAULT;
> + } else {
> + int ret;
> +
> + ret = kstrtos32_from_user(buf, count, 16, &value);
> + if (ret)
> + return ret;
> + }
> +
> + if (value < 0)
> + return -EINVAL;
> +
> + pm_qos_update_target(req->qos, &req->node, PM_QOS_UPDATE_REQ, value);
> +
> + return count;
> +}
> +
> +static const struct file_operations cpu_wakeup_latency_qos_fops = {
> + .open = cpu_wakeup_latency_qos_open,
> + .release = cpu_wakeup_latency_qos_release,
> + .read = cpu_wakeup_latency_qos_read,
> + .write = cpu_wakeup_latency_qos_write,
> + .llseek = noop_llseek,
> +};
> +
> +static struct miscdevice cpu_wakeup_latency_qos_miscdev = {
> + .minor = MISC_DYNAMIC_MINOR,
> + .name = "cpu_wakeup_latency",
> + .fops = &cpu_wakeup_latency_qos_fops,
> +};
> +
> static int __init cpu_latency_qos_init(void)
> {
> int ret;
> @@ -424,6 +521,11 @@ static int __init cpu_latency_qos_init(void)
> pr_err("%s: %s setup failed\n", __func__,
> cpu_latency_qos_miscdev.name);
>
> + ret = misc_register(&cpu_wakeup_latency_qos_miscdev);
> + if (ret < 0)
> + pr_err("%s: %s setup failed\n", __func__,
> + cpu_wakeup_latency_qos_miscdev.name);
> +
> return ret;
> }
> late_initcall(cpu_latency_qos_init);
> --
> 2.43.0
>
--
Best regards,
Dhruva Gole
Texas Instruments Incorporated
Powered by blists - more mailing lists