* [RFC PATCH 1/2] power: Userspace performance QoS
@ 2025-05-05 16:19 Daniel Lezcano
2025-05-05 16:19 ` [RFC PATCH 2/2] selftests: Add perf_qos selftests Daniel Lezcano
2025-05-08 21:05 ` [RFC PATCH 1/2] power: Userspace performance QoS Rafael J. Wysocki
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Lezcano @ 2025-05-05 16:19 UTC (permalink / raw)
To: daniel.lezcano, rafael
Cc: linux-kernel, linux-pm, ulf.hansson, arnd, saravanak
In the embedded ecosystem, the Linux kernel is modified to integrate
fake thermal cooling devices for the sake of the ABI exported in the
sysfs.
While investigating those different devices, it appears most of them
could fall under a performance QoS feature.
As discussed at the Linux Plumber Conference 2024, we want to let the
userspace to access the device performance knob via a char device
which would be created by the backend drivers and controlled with an
ioctl.
A performance constraint is a minimal or a maximal limit applied to a
device performance state. A process can only set one constraint per
limit, in other words a minimal performance and/or a maximal
performance constraint. A new value will change the current
constraint, not create a new one. If another constraint must be
stacked with the current one, then the char device file must be opened
again and the resulting new file descriptor must be used to create a
new constraint.
Constraint life cycle:
The userspace can be a place where buggy programs with root privileges
can tamper with the device performance. In order to prevent some dumb
logics to set a device performance state and then go away, thus
messing with the global system performance consistency, there is a
particular care of the constraint life cycles. These ones are directly
tied with the opened file descriptor of the char device. When it is
released, then the constraint is removed but only if its refcount
reaches zero. This situation exists if only process sets the
constraint and then closes the file descriptor (manually or at exit
time). If the process forks multiple time and the children inherit the
file descriptor, the constraint will be removed when all the children
close the file descriptor.
However, if another process opens the char device and sets a
constraint which already exists then that results in incrementing the
refcount of the constraint. The constraint is then removed when all
the processes have closed their file descriptor pointing to the char
device.
At creation time:
- if another process asked for the same limit of performance, then
the refcount constraint is incremented
- if there is an existing constraint with a higher priority, then the
requested constraint is queued in the ordered list of constraints
- if there is an existing constraint with a lower limit, then the
requested constrained is applied and the current constraint is
queued in the ordered list of constraints
At removal time:
- if the removed constraint is the current one, then the next
constraint in the ordered list is applied
- if the removed constraint is not the current one, then it is simply
removed from the ordered list
The changes allows the userspace to set a performance constraint for a
specific device but the kernel may also want to apply a performance
constraint. The in-kernel API is not yet implemented as it represents
a significant amount of work depending on the direction of this patch.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
include/linux/perf_qos.h | 45 ++
include/uapi/linux/perf_qos_ioctl.h | 47 ++
kernel/power/Makefile | 2 +-
kernel/power/perf_qos.c | 652 ++++++++++++++++++++++++++++
4 files changed, 745 insertions(+), 1 deletion(-)
create mode 100644 include/linux/perf_qos.h
create mode 100644 include/uapi/linux/perf_qos_ioctl.h
create mode 100644 kernel/power/perf_qos.c
diff --git a/include/linux/perf_qos.h b/include/linux/perf_qos.h
new file mode 100644
index 000000000000..57529c40be4d
--- /dev/null
+++ b/include/linux/perf_qos.h
@@ -0,0 +1,45 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Performance QoS device abstraction
+ *
+ * Copyright (2024) Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#ifndef __PERF_QOS_H
+#define __PERF_QOS_H
+
+#include <uapi/linux/perf_qos_ioctl.h>
+
+struct perf_qos;
+
+/**
+ * struct perf_qos_value_descr - Performance constraint description
+ *
+ * @unit: the unit used for the constraint (normalized, throughput, ...)
+ * @limit_min: the minimal constraint limit to be set
+ * @limit_max: the maximal constraint limit to be set
+ */
+struct perf_qos_value_descr {
+ perf_qos_unit_t unit;
+ int limit_min;
+ int limit_max;
+};
+
+typedef int (*set_perf_limit_cb_t)(int);
+
+struct perf_qos_ops {
+ set_perf_limit_cb_t set_perf_limit_max;
+ set_perf_limit_cb_t set_perf_limit_min;
+};
+
+extern struct perf_qos *perf_qos_device_create(const char *name,
+ struct perf_qos_ops *ops,
+ struct perf_qos_value_descr *descr);
+
+extern int perf_qos_is_allowed(struct perf_qos *pq, int performance);
+
+extern void perf_qos_device_destroy(struct perf_qos *pq);
+
+#endif
diff --git a/include/uapi/linux/perf_qos_ioctl.h b/include/uapi/linux/perf_qos_ioctl.h
new file mode 100644
index 000000000000..a9fb8940c175
--- /dev/null
+++ b/include/uapi/linux/perf_qos_ioctl.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Performance QoS device abstraction
+ *
+ * Copyright (2024) Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#ifndef __PERF_QOS_IOCTL_H
+#define __PERF_QOS_IOCTL_H
+
+#include <linux/types.h>
+
+enum {
+ PERF_QOS_IOC_SET_MIN_CMD,
+ PERF_QOS_IOC_GET_MIN_CMD,
+ PERF_QOS_IOC_SET_MAX_CMD,
+ PERF_QOS_IOC_GET_MAX_CMD,
+ PERF_QOS_IOC_GET_UNIT_CMD,
+ PERF_QOS_IOC_GET_LIMITS_CMD,
+ PERF_QOS_IOC_MAX_CMD,
+};
+
+typedef enum {
+ PERF_QOS_UNIT_NORMAL,
+ PERF_QOS_UNIT_KBPS,
+ PERF_QOS_UNIT_MAX
+} perf_qos_unit_t;
+
+struct perf_qos_ioctl_arg {
+ int value;
+ int limit_min;
+ int limit_max;
+ perf_qos_unit_t unit;
+};
+
+#define PERF_QOS_IOCTL_TYPE 'P'
+
+#define PERF_QOS_IOC_SET_MIN _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MIN_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_MIN _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MIN_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_SET_MAX _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MAX_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_MAX _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MAX_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_UNIT _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_UNIT_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_LIMITS _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_LIMITS_CMD, struct perf_qos_ioctl_arg *)
+
+#endif
diff --git a/kernel/power/Makefile b/kernel/power/Makefile
index 874ad834dc8d..e2e4d707ab6e 100644
--- a/kernel/power/Makefile
+++ b/kernel/power/Makefile
@@ -8,7 +8,7 @@ endif
KASAN_SANITIZE_snapshot.o := n
-obj-y += qos.o
+obj-y += qos.o perf_qos.o
obj-$(CONFIG_PM) += main.o
obj-$(CONFIG_VT_CONSOLE_SLEEP) += console.o
obj-$(CONFIG_FREEZER) += process.o
diff --git a/kernel/power/perf_qos.c b/kernel/power/perf_qos.c
new file mode 100644
index 000000000000..ca0619b07ae5
--- /dev/null
+++ b/kernel/power/perf_qos.c
@@ -0,0 +1,652 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <linux/cdev.h>
+#include <linux/perf_qos.h>
+#include <linux/list_sort.h>
+
+#define DEVNAME "perf_qos"
+#define NUM_PERF_QOS_MINORS 128
+
+static DEFINE_IDR(perf_qos_minors);
+static struct class *perf_qos_class;
+static dev_t perf_qos_devt;
+
+typedef enum {
+ PERF_QOS_LIMIT_MAX,
+ PERF_QOS_LIMIT_MIN,
+} perf_qos_limit_t;
+
+/**
+ * struct perf_qos_constraint - structure holding a constraint information
+ *
+ * @soft_limit: an integer corresponding of the limit value set
+ * @hard_limit: an integer corresponding to the limit value allowed by the driver
+ * @kref: a refcount to the constraint responsible of its life cycle
+ * @set_perf_limit_cb: a callback to notify the backend driver about the limit change
+ * @node: the list node to attach this constraint with the list of constraints
+ * @head: the list of constraints the @node
+ *
+ * This structure has a couple of instanciation per perf QoS file
+ * opened by a process. The process can apply one or two constraints
+ * to the device.
+ *
+ * Other processes will allocate their own constraints which will be
+ * added in the list of constraints.
+ */
+struct perf_qos_constraint {
+ int soft_limit;
+ int hard_limit;
+ set_perf_limit_cb_t set_perf_limit_cb;
+ struct kref kref;
+ struct list_head node;
+ struct list_head *head;
+};
+
+/**
+ * struct perf_qos - structure owning the constraint information for
+ * the device
+ *
+ * @lock: lock to protect the actions on the list of constraints
+ * @perf_qos_cdev: a struct cdev used for the device destruction
+ * @ops: the ops given by the backend driver to notify the change of constraint
+ * @descr: a constraint descriptor giving the units and the boundaries
+ * @perf_min: the list of the minimal performance constraints
+ * @perf_max: the list of the maximal performance constraints
+ */
+struct perf_qos {
+ spinlock_t lock;
+ struct cdev perf_qos_cdev;
+ struct perf_qos_ops *ops;
+ struct perf_qos_value_descr *descr;
+ struct list_head perf_min;
+ struct list_head perf_max;
+};
+
+/**
+ * struct perf_qos_data - structure with the requested constraints
+ *
+ * @pqc_min: the requested performance constraint giving the minimal value
+ * @pqc_max: the requested performance constraint giving the maximal value
+ */
+struct perf_qos_data {
+ struct perf_qos_constraint *pqc_min;
+ struct perf_qos_constraint *pqc_max;
+};
+
+static struct perf_qos_constraint *perf_qos_constraint_find(struct list_head *list, int value)
+{
+ struct perf_qos_constraint *pcq;
+
+ list_for_each_entry(pcq, list, node) {
+ if (pcq->soft_limit == value)
+ return pcq;
+ }
+
+ return NULL;
+}
+
+static int perf_qos_constraint_cmp(void *data,
+ const struct list_head *l1,
+ const struct list_head *l2)
+{
+ struct perf_qos_constraint *pqc1 = container_of(l1, struct perf_qos_constraint, node);
+ struct perf_qos_constraint *pqc2 = container_of(l2, struct perf_qos_constraint, node);
+
+ /*
+ * The comparison will depend if we apply a max or min
+ * performance constraint. If the soft limit is lesser than
+ * the hard limit, that means it is a maximum limitation.
+ */
+ if (pqc1->soft_limit < pqc1->hard_limit)
+ return pqc1->soft_limit - pqc2->soft_limit;
+
+ return pqc2->soft_limit - pqc1->soft_limit;
+}
+
+static int perf_qos_del(struct perf_qos_constraint *pcq)
+{
+ const struct perf_qos_constraint *first;
+ int new_limit;
+
+ first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
+
+ list_del(&pcq->node);
+
+ /*
+ * The active constraint is not the one we removed, so there
+ * is nothing more to do
+ */
+ if (first != pcq)
+ return 0;
+
+ /*
+ * As we remove the first entry, then get the new first entry
+ * to apply the next constraint. If there is no more
+ * constraint set, reset to the original limit. Otherwise, use
+ * the new constraint value.
+ */
+ if (list_empty(pcq->head))
+ new_limit = pcq->hard_limit;
+ else {
+ first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
+ new_limit = first->soft_limit;
+ };
+
+ /*
+ * Notify the backend driver to update its performance level
+ * if needed. If the performance level is currently inside the
+ * new limits, nothing will happen. Otherwise it must be
+ * adjust the current performance level to be inside the
+ * authorized limits
+ */
+ pcq->set_perf_limit_cb(new_limit);
+
+ return 1;
+}
+
+static int perf_qos_add(struct perf_qos_constraint *pcq)
+{
+ const struct perf_qos_constraint *first;
+
+ list_add(&pcq->node, pcq->head);
+
+ list_sort(NULL, pcq->head, perf_qos_constraint_cmp);
+
+ /*
+ * A sort happened resulting in a different constraint at the head
+ */
+ first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
+
+ /*
+ * The inserted constraint did not become the active one, so
+ * we can bail out
+ */
+ if (pcq != first)
+ return 0;
+
+ /*
+ * Notify the backend driver to update its performance level
+ * if needed. If the performance level is currently inside the
+ * new limits, nothing will happen. Otherwise it must be
+ * adjust the current performance level to be inside the
+ * authorized limits
+ */
+ pcq->set_perf_limit_cb(first->soft_limit);
+
+ return 1;
+}
+
+static void perf_qos_constraint_release(struct kref *kref)
+{
+ struct perf_qos_constraint *pcq;
+
+ pcq = container_of(kref, struct perf_qos_constraint, kref);
+
+ /*
+ * The removal of the constraint results in the change of the
+ * first entry of the list which means it was the active
+ * one. We need to apply the next constraint of the list
+ */
+ if (perf_qos_del(pcq)) {
+ /* Something to do */
+ }
+
+ kfree(pcq);
+}
+
+static void perf_qos_constraint_put(struct perf_qos_constraint *pcq)
+{
+ kref_put(&pcq->kref, perf_qos_constraint_release);
+}
+
+static void perf_qos_constraint_get(struct perf_qos_constraint *pcq)
+{
+ kref_get(&pcq->kref);
+}
+
+static struct perf_qos_constraint *perf_qos_constraint_alloc(struct perf_qos *pq, int soft_limit,
+ struct list_head *perf, perf_qos_limit_t limit)
+{
+ struct perf_qos_constraint *pqc;
+
+ pqc = kzalloc(sizeof(*pqc), GFP_KERNEL);
+ if (!pqc)
+ return NULL;
+
+ kref_init(&pqc->kref);
+ INIT_LIST_HEAD(&pqc->node);
+
+ if (limit == PERF_QOS_LIMIT_MAX) {
+ pqc->set_perf_limit_cb = pq->ops->set_perf_limit_max;
+ pqc->hard_limit = pq->descr->limit_max;
+ } else {
+ pqc->set_perf_limit_cb = pq->ops->set_perf_limit_min;
+ pqc->hard_limit = pq->descr->limit_min;
+ }
+
+ pqc->head = perf;
+ pqc->soft_limit = soft_limit;
+
+ return pqc;
+}
+
+static int perf_qos_open(struct inode *inode, struct file *file)
+{
+ struct perf_qos_data *pqd;
+ struct perf_qos *pq;
+
+ pq = idr_find(&perf_qos_minors, iminor(inode));
+ if (!pq)
+ return -ENODEV;
+
+ inode->i_private = pq;
+
+ pqd = kzalloc(sizeof(*pqd), GFP_KERNEL);
+ if (!pqd)
+ return -ENOMEM;
+
+ file->private_data = pqd;
+
+ return 0;
+}
+
+static int perf_qos_release(struct inode *inode, struct file *file)
+{
+ struct perf_qos *pq = inode->i_private;
+ struct perf_qos_data *pqd = file->private_data;
+
+ spin_lock(&pq->lock);
+
+ if (pqd->pqc_min)
+ perf_qos_constraint_put(pqd->pqc_min);
+
+ if (pqd->pqc_max)
+ perf_qos_constraint_put(pqd->pqc_max);
+
+ spin_unlock(&pq->lock);
+
+ kfree(pqd);
+
+ return 0;
+}
+
+static int perf_qos_unset(struct perf_qos_constraint **cur_pqc,
+ perf_qos_limit_t limit, struct list_head *perf, int value)
+{
+ /*
+ * Removing a constraint:
+ *
+ * - if it exists then *current_pqc is set. We decrement the
+ * refcount and update the current constraint by setting it
+ * to NULL
+ *
+ * - if the current constraint does not exist then, it is an
+ * error and we should exit with an error
+ */
+ if (!(*cur_pqc))
+ return -EINVAL;
+
+ perf_qos_constraint_put(*cur_pqc);
+ *cur_pqc = NULL;
+
+ return 0;
+}
+
+static int perf_qos_set(struct perf_qos *pq, struct perf_qos_constraint **cur_pqc,
+ perf_qos_limit_t limit, struct list_head *perf, int value)
+{
+ struct perf_qos_constraint *pqc;
+ int ret = 0;
+
+ /*
+ * We are trying to set the same constraint.
+ */
+ if (*cur_pqc && ((*cur_pqc)->soft_limit == value)) {
+ ret = -EALREADY;
+ goto out;
+ }
+
+ /*
+ * Case 2 : Adding a constraint:
+ *
+ * - it already exists because it was created by another
+ * process, we increment the refcount
+ *
+ * - it already exists because we created it before, we
+ * return an error
+ *
+ * - it does not exist but there is a previous different
+ * constraint we set before. It is a constraint change. We
+ * must release the previous constraint and create a new
+ * one. However, we apply the new constraint and then we
+ * remove the old one in order to not have the backend
+ * driver with a window where there is no constraint at all
+ *
+ * - it does not exist and there is no previous constraint. It
+ * is a new constraint. We allocate the constraint, apply it
+ * and set it as the current constraint
+ */
+ pqc = perf_qos_constraint_find(perf, value);
+ if (pqc) {
+ perf_qos_constraint_get(pqc);
+ } else {
+ pqc = perf_qos_constraint_alloc(pq, value, perf, limit);
+ if (!pqc) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /*
+ * The new constraint has to be applied because it
+ * results in a change of the first entry of the list
+ * of constraints
+ */
+ if (perf_qos_add(pqc)) {
+ /* Something to do */
+ }
+ }
+
+ /*
+ * We previously set a constraint, let's release the refcount
+ * as we change it. The constraint can be freed if we are the
+ * last one having a reference to it or if we are the creator
+ * and no other process held a refcount on it.
+ */
+ if ((*cur_pqc))
+ perf_qos_constraint_put(*cur_pqc);
+
+ *cur_pqc = pqc;
+out:
+ return ret;
+}
+
+static int ioctl_perf_qos_set_max(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia)
+{
+ if (pqia->value > pq->descr->limit_max)
+ return -EINVAL;
+
+ if (pqia->value == pq->descr->limit_max)
+ return perf_qos_unset(&pqd->pqc_max, PERF_QOS_LIMIT_MAX,
+ &pq->perf_max, pqia->value);
+ else
+ return perf_qos_set(pq, &pqd->pqc_max, PERF_QOS_LIMIT_MAX,
+ &pq->perf_max, pqia->value);
+}
+
+static int ioctl_perf_qos_set_min(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia)
+{
+ if (pqia->value < pq->descr->limit_min)
+ return -EINVAL;
+
+ if (pqia->value == pq->descr->limit_min)
+ return perf_qos_unset(&pqd->pqc_min, PERF_QOS_LIMIT_MIN,
+ &pq->perf_min, pqia->value);
+ else
+ return perf_qos_set(pq, &pqd->pqc_min, PERF_QOS_LIMIT_MIN,
+ &pq->perf_min, pqia->value);
+}
+
+static int perf_qos_get(struct list_head *perf, int *value)
+{
+ struct perf_qos_constraint *pqc;
+
+ /*
+ * We may not have set any performance constraint yet but
+ * another process may have set one, so we get the head of
+ * performance constraint list
+ */
+ if (list_empty(perf))
+ return -ENODATA;
+
+ pqc = list_first_entry(perf, struct perf_qos_constraint, node);
+
+ *value = pqc->soft_limit;
+
+ return 0;
+}
+
+static int ioctl_perf_qos_get_min(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia)
+{
+ return perf_qos_get(&pq->perf_min, &pqia->value);
+}
+
+static int ioctl_perf_qos_get_max(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia)
+{
+ return perf_qos_get(&pq->perf_max, &pqia->value);
+}
+
+static int ioctl_perf_qos_get_unit(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia)
+{
+ pqia->unit = pq->descr->unit;
+
+ return 0;
+}
+
+static int ioctl_perf_qos_get_limits(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia)
+{
+ pqia->limit_min = pq->descr->limit_min;
+ pqia->limit_max = pq->descr->limit_max;
+
+ return 0;
+}
+
+typedef int (*perf_qos_ioctl_ops_t)(struct perf_qos *pq,
+ struct perf_qos_data *pqd,
+ struct perf_qos_ioctl_arg *pqia);
+
+static long perf_qos_ioctl(struct file *file, unsigned int ucmd,
+ unsigned long arg)
+{
+ struct perf_qos_data *pqd = file->private_data;
+ struct perf_qos *pq = file->f_inode->i_private;
+ struct perf_qos_ioctl_arg pqia;
+ int cmd = _IOC_NR(ucmd);
+ int dir = _IOC_DIR(ucmd);
+ int type = _IOC_TYPE(ucmd);
+ int ret;
+
+ perf_qos_ioctl_ops_t perf_qos_ioctl_ops[] = {
+ [PERF_QOS_IOC_SET_MAX_CMD] = ioctl_perf_qos_set_max,
+ [PERF_QOS_IOC_SET_MIN_CMD] = ioctl_perf_qos_set_min,
+ [PERF_QOS_IOC_GET_MAX_CMD] = ioctl_perf_qos_get_max,
+ [PERF_QOS_IOC_GET_MIN_CMD] = ioctl_perf_qos_get_min,
+ [PERF_QOS_IOC_GET_UNIT_CMD] = ioctl_perf_qos_get_unit,
+ [PERF_QOS_IOC_GET_LIMITS_CMD] = ioctl_perf_qos_get_limits,
+ };
+
+ if (type != PERF_QOS_IOCTL_TYPE)
+ return -EINVAL;
+
+ if (cmd < 0 || cmd >= PERF_QOS_IOC_MAX_CMD)
+ return -EINVAL;
+
+ if (dir & _IOC_WRITE) {
+ if (copy_from_user(&pqia, (typeof(pqia) *)arg, sizeof(pqia)))
+ return -EACCES;
+ }
+
+ spin_lock(&pq->lock);
+ ret = perf_qos_ioctl_ops[cmd](pq, pqd, &pqia);
+ spin_unlock(&pq->lock);
+
+ if (ret)
+ goto out;
+
+ if (dir & _IOC_READ) {
+ if (copy_to_user((typeof(pqia) *)arg, &pqia, sizeof(pqia)))
+ return -EACCES;
+ }
+out:
+ return ret;
+}
+
+static const struct file_operations perf_qos_fops = {
+ .owner = THIS_MODULE,
+ .open = perf_qos_open,
+ .release = perf_qos_release,
+ .unlocked_ioctl = perf_qos_ioctl,
+#ifdef CONFIG_COMPAT
+ .compat_ioctl = perf_qos_ioctl,
+#endif
+};
+
+void perf_qos_device_destroy(struct perf_qos *pq)
+{
+ idr_remove(&perf_qos_minors, MINOR(pq->perf_qos_cdev.dev));
+ device_destroy(perf_qos_class, pq->perf_qos_cdev.dev);
+ cdev_del(&pq->perf_qos_cdev);
+ kfree(pq->descr);
+ kfree(pq->ops);
+ kfree(pq);
+}
+EXPORT_SYMBOL_GPL(perf_qos_device_destroy);
+
+int perf_qos_is_allowed(struct perf_qos *pq, int performance)
+{
+ const struct perf_qos_constraint *first;
+ int allowed = 1;
+
+ spin_lock(&pq->lock);
+
+ first = list_first_entry(&pq->perf_min, struct perf_qos_constraint, node);
+ if (performance < first->soft_limit)
+ allowed = 0;
+
+ first = list_first_entry(&pq->perf_max, struct perf_qos_constraint, node);
+ if (performance > first->soft_limit)
+ allowed = 0;
+
+ spin_unlock(&pq->lock);
+
+ return allowed;
+}
+EXPORT_SYMBOL_GPL(perf_qos_is_allowed);
+
+struct perf_qos *perf_qos_device_create(const char *name,
+ struct perf_qos_ops *ops,
+ struct perf_qos_value_descr *descr)
+{
+ struct device *dev;
+ struct perf_qos *pq;
+ dev_t devt;
+ int minor;
+ int ret;
+
+ if (!ops->set_perf_limit_max || !ops->set_perf_limit_min)
+ return ERR_PTR(-EINVAL);
+
+ if (descr->unit < 0 || descr->unit >= PERF_QOS_UNIT_MAX)
+ return ERR_PTR(-EINVAL);
+
+ if (descr->limit_min > descr->limit_max)
+ return ERR_PTR(-EINVAL);
+
+ if (descr->unit == PERF_QOS_UNIT_NORMAL) {
+ if (descr->limit_min < 0 || descr->limit_max > 1024)
+ return ERR_PTR(-EINVAL);
+ }
+
+ pq = kzalloc(sizeof(*pq), GFP_KERNEL);
+ if (!pq)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_LIST_HEAD(&pq->perf_min);
+ INIT_LIST_HEAD(&pq->perf_max);
+ spin_lock_init(&pq->lock);
+
+ pq->ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
+ if (!pq->ops) {
+ ret = -ENOMEM;
+ goto out_kfree_pq;
+ }
+
+ pq->descr = kmemdup(descr, sizeof(*descr), GFP_KERNEL);
+ if (!pq->descr) {
+ ret = -ENOMEM;
+ goto out_kfree_pq_ops;
+ }
+
+ minor = idr_alloc(&perf_qos_minors, pq, 0,
+ NUM_PERF_QOS_MINORS, GFP_KERNEL);
+ if (minor < 0)
+ goto out_kfree_pq_descr;
+
+ devt = MKDEV(MAJOR(perf_qos_devt), minor);
+
+ cdev_init(&pq->perf_qos_cdev, &perf_qos_fops);
+
+ ret = cdev_add(&pq->perf_qos_cdev, devt, 1);
+ if (ret < 0)
+ goto out_idr_remove;
+
+ dev = device_create(perf_qos_class, NULL, devt, NULL, name);
+ if (IS_ERR(dev)) {
+ ret = PTR_ERR(dev);
+ goto out_cdev_del;
+ }
+
+ return pq;
+
+out_cdev_del:
+ cdev_del(&pq->perf_qos_cdev);
+
+out_idr_remove:
+ idr_remove(&perf_qos_minors, minor);
+
+out_kfree_pq_descr:
+ kfree(pq->descr);
+
+out_kfree_pq_ops:
+ kfree(pq->ops);
+
+out_kfree_pq:
+ kfree(pq);
+
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(perf_qos_device_create);
+
+static char *perf_qos_devnode(const struct device *dev, umode_t *mode)
+{
+ return kasprintf(GFP_KERNEL, "%s/%s", DEVNAME, dev_name(dev));
+}
+
+static int perf_qos_init(void)
+{
+ int ret;
+
+ ret = alloc_chrdev_region(&perf_qos_devt, 0,
+ NUM_PERF_QOS_MINORS, DEVNAME);
+ if (ret)
+ return ret;
+
+ perf_qos_class = class_create(DEVNAME);
+ if (IS_ERR(perf_qos_class)) {
+ unregister_chrdev_region(perf_qos_devt, NUM_PERF_QOS_MINORS);
+ return PTR_ERR(perf_qos_class);
+ }
+ perf_qos_class->devnode = perf_qos_devnode;
+
+ return 0;
+}
+
+subsys_initcall(perf_qos_init);
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [RFC PATCH 2/2] selftests: Add perf_qos selftests
2025-05-05 16:19 [RFC PATCH 1/2] power: Userspace performance QoS Daniel Lezcano
@ 2025-05-05 16:19 ` Daniel Lezcano
2025-05-23 17:43 ` Eric Smith
2025-05-08 21:05 ` [RFC PATCH 1/2] power: Userspace performance QoS Rafael J. Wysocki
1 sibling, 1 reply; 7+ messages in thread
From: Daniel Lezcano @ 2025-05-05 16:19 UTC (permalink / raw)
To: daniel.lezcano, rafael
Cc: linux-kernel, linux-pm, ulf.hansson, arnd, saravanak
The performance QoS is a framework to allow the userspace to set the
performance limits of a device which is exported as a char device in
/dev. The performance constraints are set from userspace and their
life cycle is tied with the opened file descriptor and other processes
requesting the same constraint via another instance of the file
descriptor. This kind of non trivial behavior, involving constraint
limits to be enqueued in a sorted list and depending on the process
holding a file descriptor, deserves a set of testing programs.
This patch provides somes tests which depend on a kernel module
creating a dummy performance QoS device. More tests will be added
later.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
lib/Kconfig.debug | 9 ++
lib/Makefile | 1 +
lib/test_perf_qos.c | 69 ++++++++
tools/include/uapi/linux/perf_qos_ioctl.h | 47 ++++++
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/perf_qos/Makefile | 16 ++
tools/testing/selftests/perf_qos/get_limits.c | 61 +++++++
.../testing/selftests/perf_qos/get_set_max.c | 95 +++++++++++
.../selftests/perf_qos/get_set_max_forked.c | 150 ++++++++++++++++++
.../testing/selftests/perf_qos/get_set_min.c | 95 +++++++++++
.../selftests/perf_qos/get_set_min_forked.c | 150 ++++++++++++++++++
tools/testing/selftests/perf_qos/get_unit.c | 46 ++++++
.../selftests/perf_qos/set_max_forked.c | 147 +++++++++++++++++
.../selftests/perf_qos/set_min_forked.c | 147 +++++++++++++++++
.../selftests/perf_qos/set_multiple_maxs.c | 93 +++++++++++
.../selftests/perf_qos/set_multiple_mins.c | 87 ++++++++++
.../perf_qos/set_same_multiple_maxs.c | 84 ++++++++++
.../perf_qos/set_same_multiple_mins.c | 84 ++++++++++
18 files changed, 1382 insertions(+)
create mode 100644 lib/test_perf_qos.c
create mode 100644 tools/include/uapi/linux/perf_qos_ioctl.h
create mode 100644 tools/testing/selftests/perf_qos/Makefile
create mode 100644 tools/testing/selftests/perf_qos/get_limits.c
create mode 100644 tools/testing/selftests/perf_qos/get_set_max.c
create mode 100644 tools/testing/selftests/perf_qos/get_set_max_forked.c
create mode 100644 tools/testing/selftests/perf_qos/get_set_min.c
create mode 100644 tools/testing/selftests/perf_qos/get_set_min_forked.c
create mode 100644 tools/testing/selftests/perf_qos/get_unit.c
create mode 100644 tools/testing/selftests/perf_qos/set_max_forked.c
create mode 100644 tools/testing/selftests/perf_qos/set_min_forked.c
create mode 100644 tools/testing/selftests/perf_qos/set_multiple_maxs.c
create mode 100644 tools/testing/selftests/perf_qos/set_multiple_mins.c
create mode 100644 tools/testing/selftests/perf_qos/set_same_multiple_maxs.c
create mode 100644 tools/testing/selftests/perf_qos/set_same_multiple_mins.c
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 7312ae7c3cc5..5b66089d6103 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2585,6 +2585,15 @@ config TEST_FIRMWARE
If unsure, say N.
+config TEST_PERF_QOS
+ tristate "Create a dummy device for the performance QoS testing"
+ help
+ This builds the "test_perf_qos" module which creates an
+ userspace interface for testing the performance QoS API. It
+ is needed for the performance QoS selftests.
+
+ If unsure, say N.
+
config TEST_SYSCTL
tristate "sysctl test driver"
depends on PROC_SYSCTL
diff --git a/lib/Makefile b/lib/Makefile
index 773adf88af41..72d345071725 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_TEST_BPF) += test_bpf.o
test_dhry-objs := dhry_1.o dhry_2.o dhry_run.o
obj-$(CONFIG_TEST_DHRY) += test_dhry.o
obj-$(CONFIG_TEST_FIRMWARE) += test_firmware.o
+obj-$(CONFIG_TEST_PERF_QOS) += test_perf_qos.o
obj-$(CONFIG_TEST_BITOPS) += test_bitops.o
CFLAGS_test_bitops.o += -Werror
obj-$(CONFIG_CPUMASK_KUNIT_TEST) += cpumask_kunit.o
diff --git a/lib/test_perf_qos.c b/lib/test_perf_qos.c
new file mode 100644
index 000000000000..7d1c21f4170d
--- /dev/null
+++ b/lib/test_perf_qos.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kernel module for testing performance QoS
+ *
+ * Copyright (2024) Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@kernel.org>
+ *
+ */
+#include <linux/module.h>
+#include <linux/perf_qos.h>
+
+static struct perf_qos *pq;
+
+static int test_set_perf_limit_min(int limit)
+{
+ static int prev_limit = -1;
+
+ pr_info("Performance minimal limit set to %d->%d\n",
+ prev_limit, limit);
+
+ WARN_ON_ONCE(prev_limit == limit);
+
+ return 0;
+}
+
+static int test_set_perf_limit_max(int limit)
+{
+ static int prev_limit = -1;
+
+ pr_info("Performance maximal limit set to %d->%d\n",
+ prev_limit, limit);
+
+ WARN_ON_ONCE(prev_limit == limit);
+
+ return 0;
+}
+
+static int __init test_perf_qos_init(void)
+{
+ struct perf_qos_ops ops = {
+ .set_perf_limit_max = test_set_perf_limit_max,
+ .set_perf_limit_min = test_set_perf_limit_min,
+ };
+
+ struct perf_qos_value_descr descr = {
+ .unit = PERF_QOS_UNIT_NORMAL,
+ .limit_min = 0,
+ .limit_max = 1024,
+ };
+
+ pq = perf_qos_device_create("dummy", &ops, &descr);
+ if (IS_ERR(pq))
+ return PTR_ERR(pq);
+
+ return 0;
+}
+
+static void __exit test_perf_qos_exit(void)
+{
+ perf_qos_device_destroy(pq);
+}
+
+module_init(test_perf_qos_init);
+module_exit(test_perf_qos_exit);
+
+MODULE_AUTHOR("Daniel Lezcano <daniel.lezcano@kernel.org>");
+MODULE_DESCRIPTION("Kernel module for testing the performance QoS");
+MODULE_LICENSE("GPL");
diff --git a/tools/include/uapi/linux/perf_qos_ioctl.h b/tools/include/uapi/linux/perf_qos_ioctl.h
new file mode 100644
index 000000000000..a9fb8940c175
--- /dev/null
+++ b/tools/include/uapi/linux/perf_qos_ioctl.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Performance QoS device abstraction
+ *
+ * Copyright (2024) Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#ifndef __PERF_QOS_IOCTL_H
+#define __PERF_QOS_IOCTL_H
+
+#include <linux/types.h>
+
+enum {
+ PERF_QOS_IOC_SET_MIN_CMD,
+ PERF_QOS_IOC_GET_MIN_CMD,
+ PERF_QOS_IOC_SET_MAX_CMD,
+ PERF_QOS_IOC_GET_MAX_CMD,
+ PERF_QOS_IOC_GET_UNIT_CMD,
+ PERF_QOS_IOC_GET_LIMITS_CMD,
+ PERF_QOS_IOC_MAX_CMD,
+};
+
+typedef enum {
+ PERF_QOS_UNIT_NORMAL,
+ PERF_QOS_UNIT_KBPS,
+ PERF_QOS_UNIT_MAX
+} perf_qos_unit_t;
+
+struct perf_qos_ioctl_arg {
+ int value;
+ int limit_min;
+ int limit_max;
+ perf_qos_unit_t unit;
+};
+
+#define PERF_QOS_IOCTL_TYPE 'P'
+
+#define PERF_QOS_IOC_SET_MIN _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MIN_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_MIN _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MIN_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_SET_MAX _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MAX_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_MAX _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MAX_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_UNIT _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_UNIT_CMD, struct perf_qos_ioctl_arg *)
+#define PERF_QOS_IOC_GET_LIMITS _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_LIMITS_CMD, struct perf_qos_ioctl_arg *)
+
+#endif
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 363d031a16f7..d6f3706443c6 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -73,6 +73,7 @@ TARGETS += net/rds
TARGETS += net/tcp_ao
TARGETS += nsfs
TARGETS += perf_events
+TARGETS += perf_qos
TARGETS += pidfd
TARGETS += pid_namespace
TARGETS += power_supply
diff --git a/tools/testing/selftests/perf_qos/Makefile b/tools/testing/selftests/perf_qos/Makefile
new file mode 100644
index 000000000000..279a2bce1b82
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/Makefile
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TEST_GEN_PROGS = get_unit get_limits
+TEST_GEN_PROGS += get_set_min get_set_max
+TEST_GEN_PROGS += get_set_min_forked get_set_max_forked
+TEST_GEN_PROGS += set_min_forked set_max_forked
+TEST_GEN_PROGS += set_multiple_mins set_multiple_maxs
+TEST_GEN_PROGS += set_same_multiple_mins set_same_multiple_maxs
+
+include ../lib.mk
+
+TOOLSDIR := $(top_srcdir)/tools
+TOOLSINCDIR := $(TOOLSDIR)/include
+APIDIR := $(TOOLSINCDIR)/uapi
+
+CFLAGS += -Wall -O2 -I$(APIDIR)
diff --git a/tools/testing/selftests/perf_qos/get_limits.c b/tools/testing/selftests/perf_qos/get_limits.c
new file mode 100644
index 000000000000..e7559481a9a3
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/get_limits.c
@@ -0,0 +1,61 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg = {};
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_LIMITS, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.unit != PERF_QOS_UNIT_NORMAL) {
+ fprintf(stderr, "Invalid unit, expected 'PERF_QOS_UNIT_NORMAL'\n");
+ return 1;
+ }
+
+ if (arg.limit_min != 0) {
+ fprintf(stderr, "Invalid minimum unit %d != 0\n", arg.limit_min);
+ return 1;
+ }
+
+ if (arg.limit_max != 1024) {
+ fprintf(stderr, "Invalid maximum unit %d != 1024\n", arg.limit_max);
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/get_set_max.c b/tools/testing/selftests/perf_qos/get_set_max.c
new file mode 100644
index 000000000000..87bf5c26acf7
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/get_set_max.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg = { .value = 512 };
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * Test 1: Check the value is set
+ */
+ if (ioctl(fd, PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != 512) {
+ fprintf(stderr, "max value differs with set/get (arg=%d)\n",
+ arg.value);
+ return 1;
+ }
+
+ /*
+ * Test 2: Check we can not set the same constraint
+ */
+ if (ioctl(fd, PERF_QOS_IOC_SET_MAX, &arg) == 0) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ /*
+ * Test 3: Check the constraint is removed
+ */
+ if (ioctl(fd, PERF_QOS_IOC_GET_LIMITS, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = arg.limit_max;
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (!ioctl(fd, PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/get_set_max_forked.c b/tools/testing/selftests/perf_qos/get_set_max_forked.c
new file mode 100644
index 000000000000..1bffddd684c8
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/get_set_max_forked.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+static int test_forked_get_set_max(int fd, const char *path)
+{
+ pid_t pid;
+ int fds[2];
+ int result;
+ const int init_value = 256;
+ struct perf_qos_ioctl_arg arg = { .value = init_value };
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (pipe(fds)) {
+ fprintf(stderr, "Failed to pipe: %m\n");
+ return 1;
+ }
+
+ pid = fork();
+ if (pid < 0) {
+ fprintf(stderr, "Failed to fork: %m\n");
+ return 1;
+ }
+
+ if (!pid) {
+ close(fd);
+ close(fds[0]);
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ arg.value = 0;
+
+ /*
+ * At this point, we must have a 'init_value'
+ * constraint created by the parent process
+ */
+ if (ioctl(fd, PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ result = arg.value;
+
+ if (write(fds[1], &result, sizeof(result)) < 0) {
+ fprintf(stderr, "Failed to write result to pipe: %m\n");
+ exit(1);
+ }
+
+ exit(0);
+ }
+
+ close(fds[1]);
+
+ if (read(fds[0], &result, sizeof(result)) < 0) {
+ fprintf(stderr, "Failed to read pipe: %m\n");
+ return 1;
+ }
+
+ if (result != init_value) {
+ fprintf(stderr, "Child test failed: %d\n", result);
+ return 1;
+ }
+
+ if (waitpid(pid, NULL, 0) < 0) {
+ fprintf(stderr, "Failed to wait child pid: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != init_value) {
+ fprintf(stderr, "Perf constraints differ %d <> %d\n",
+ arg.value, init_value);
+ return 1;
+ }
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_LIMITS, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = arg.limit_max;
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (!ioctl(fd, PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ return test_forked_get_set_max(fd, path);
+}
diff --git a/tools/testing/selftests/perf_qos/get_set_min.c b/tools/testing/selftests/perf_qos/get_set_min.c
new file mode 100644
index 000000000000..89c0a6f9f106
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/get_set_min.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg = { .value = 512 };
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * Test 1: Check the value is set
+ */
+ if (ioctl(fd, PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != 512) {
+ fprintf(stderr, "min value differs with set/get (arg=%d)\n",
+ arg.value);
+ return 1;
+ }
+
+ /*
+ * Test 2: Check we can not set the same constraint
+ */
+ if (ioctl(fd, PERF_QOS_IOC_SET_MIN, &arg) == 0) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ /*
+ * Test 3: Check the constraint is removed
+ */
+ if (ioctl(fd, PERF_QOS_IOC_GET_LIMITS, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = arg.limit_min;
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (!ioctl(fd, PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/get_set_min_forked.c b/tools/testing/selftests/perf_qos/get_set_min_forked.c
new file mode 100644
index 000000000000..36971da265a5
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/get_set_min_forked.c
@@ -0,0 +1,150 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+static int test_forked_get_set_min(int fd, const char *path)
+{
+ pid_t pid;
+ int fds[2];
+ int result;
+ const int init_value = 256;
+ struct perf_qos_ioctl_arg arg = { .value = init_value };
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (pipe(fds)) {
+ fprintf(stderr, "Failed to pipe: %m\n");
+ return 1;
+ }
+
+ pid = fork();
+ if (pid < 0) {
+ fprintf(stderr, "Failed to fork: %m\n");
+ return 1;
+ }
+
+ if (!pid) {
+ close(fd);
+ close(fds[0]);
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ arg.value = 0;
+
+ /*
+ * At this point, we must have a 'init_value'
+ * constraint created by the parent process
+ */
+ if (ioctl(fd, PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ result = arg.value;
+
+ if (write(fds[1], &result, sizeof(result)) < 0) {
+ fprintf(stderr, "Failed to write result to pipe: %m\n");
+ exit(1);
+ }
+
+ exit(0);
+ }
+
+ close(fds[1]);
+
+ if (read(fds[0], &result, sizeof(result)) < 0) {
+ fprintf(stderr, "Failed to read pipe: %m\n");
+ return 1;
+ }
+
+ if (result != init_value) {
+ fprintf(stderr, "Child test failed: %d\n", result);
+ return 1;
+ }
+
+ if (waitpid(pid, NULL, 0) < 0) {
+ fprintf(stderr, "Failed to wait child pid: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != init_value) {
+ fprintf(stderr, "Perf constraints differ %d <> %d\n",
+ arg.value, init_value);
+ return 1;
+ }
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_LIMITS, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = arg.limit_min;
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (!ioctl(fd, PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
+
+int main(int argc, char *argv[])
+{
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ return test_forked_get_set_min(fd, path);
+}
diff --git a/tools/testing/selftests/perf_qos/get_unit.c b/tools/testing/selftests/perf_qos/get_unit.c
new file mode 100644
index 000000000000..938322d78599
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/get_unit.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg = {};
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_UNIT, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/set_max_forked.c b/tools/testing/selftests/perf_qos/set_max_forked.c
new file mode 100644
index 000000000000..cff06364ba81
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/set_max_forked.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+static int integer_cmp(const void *a, const void *b)
+{
+ int *ia = (typeof(ia))(a);
+ int *ib = (typeof(ib))(b);
+
+ return (*ia) - (*ib);
+}
+
+static int test_forked_set_max(int fd, const char *path)
+{
+ struct perf_qos_ioctl_arg arg;
+
+ int i;
+ int nr_pids = 100;
+ pid_t pids[nr_pids];
+ int value, values[nr_pids];
+ int fds[nr_pids][2];
+ int ret = 1;
+
+ memset(pids, 0, sizeof(pid_t) * nr_pids);
+
+ /*
+ * Random values in the interval 0-1024, to be set by each
+ * child process. The underlying framework will sort them out
+ * so when reading them, they should be ordered and while the
+ * child process exits, the new maximal will be set each time.
+ */
+ for (i = 0; i < nr_pids; i++) {
+ value = rand() % 1023;
+
+ if (pipe(fds[i])) {
+ fprintf(stderr, "Failed to pipe: %m\n");
+ goto out;
+ }
+
+ pids[i] = fork();
+ if (pids[i] < 0) {
+ fprintf(stderr, "Failed to fork: %m\n");
+ goto out;
+ }
+
+ if (!pids[i]) {
+
+ arg.value = value;
+
+ close(fd);
+ close(fds[i][0]);
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ goto out;
+ }
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ goto out;
+ }
+
+ if (write(fds[i][1], &value, sizeof(value)) < 0) {
+ fprintf(stderr, "Failed to write in the pipe: %m\n");
+ goto out;
+ }
+
+ poll(0, 0, -1);
+
+ exit(0);
+ }
+
+ close(fds[i][1]);
+ values[i] = value;
+ }
+
+ /*
+ * Wait for all the children to set the constraint and write
+ * to the pipe
+ */
+ for (i = 0; i < nr_pids; i++) {
+ if (read(fds[i][0], &value, sizeof(value)) < 0) {
+ fprintf(stderr, "Failed to read pipe: %m\n");
+ goto out;
+ }
+ }
+
+ qsort(values, nr_pids, sizeof(values[0]), integer_cmp);
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ goto out;
+ }
+
+ if (arg.value != values[0]) {
+ fprintf(stderr, "Unexcepted value order %d <> %d\n",
+ arg.value, values[0]);
+ goto out;
+ }
+
+ ret = 0;
+out:
+ for (i = 0; i < nr_pids; i++) {
+ kill(pids[i], SIGTERM);
+ waitpid(pids[i], NULL, 0);
+ }
+
+ return ret;
+}
+
+int main(int argc, char *argv[])
+{
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ return test_forked_set_max(fd, path);
+}
diff --git a/tools/testing/selftests/perf_qos/set_min_forked.c b/tools/testing/selftests/perf_qos/set_min_forked.c
new file mode 100644
index 000000000000..beda48c251f6
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/set_min_forked.c
@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+static int integer_cmp(const void *a, const void *b)
+{
+ int *ia = (typeof(ia))(a);
+ int *ib = (typeof(ib))(b);
+
+ return (*ib) - (*ia);
+}
+
+static int test_forked_set_min(int fd, const char *path)
+{
+ struct perf_qos_ioctl_arg arg;
+
+ int i;
+ int nr_pids = 100;
+ pid_t pids[nr_pids];
+ int value, values[nr_pids];
+ int fds[nr_pids][2];
+ int ret = 1;
+
+ memset(pids, 0, sizeof(pid_t) * nr_pids);
+
+ /*
+ * Random values in the interval 0-1024, to be set by each
+ * child process. The underlying framework will sort them out
+ * so when reading them, they should be ordered and while the
+ * child process exits, the new minimal will be set each time.
+ */
+ for (i = 0; i < nr_pids; i++) {
+ value = rand() % 1023;
+
+ if (pipe(fds[i])) {
+ fprintf(stderr, "Failed to pipe: %m\n");
+ goto out;
+ }
+
+ pids[i] = fork();
+ if (pids[i] < 0) {
+ fprintf(stderr, "Failed to fork: %m\n");
+ goto out;
+ }
+
+ if (!pids[i]) {
+
+ arg.value = value;
+
+ close(fd);
+ close(fds[i][0]);
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ goto out;
+ }
+
+ if (ioctl(fd, PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ goto out;
+ }
+
+ if (write(fds[i][1], &value, sizeof(value)) < 0) {
+ fprintf(stderr, "Failed to write in the pipe: %m\n");
+ goto out;
+ }
+
+ poll(0, 0, -1);
+
+ exit(0);
+ }
+
+ close(fds[i][1]);
+ values[i] = value;
+ }
+
+ /*
+ * Wait for all the children to set the constraint and write
+ * to the pipe
+ */
+ for (i = 0; i < nr_pids; i++) {
+ if (read(fds[i][0], &value, sizeof(value)) < 0) {
+ fprintf(stderr, "Failed to read pipe: %m\n");
+ goto out;
+ }
+ }
+
+ qsort(values, nr_pids, sizeof(values[0]), integer_cmp);
+
+ if (ioctl(fd, PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ goto out;
+ }
+
+ if (arg.value != values[0]) {
+ fprintf(stderr, "Unexcepted value order %d <> %d\n",
+ arg.value, values[0]);
+ goto out;
+ }
+
+ ret = 0;
+out:
+ for (i = 0; i < nr_pids; i++) {
+ kill(pids[i], SIGTERM);
+ waitpid(pids[i], NULL, 0);
+ }
+
+ return ret;
+}
+
+int main(int argc, char *argv[])
+{
+ const char *path = "/dev/perf_qos/dummy";
+ int fd;
+
+ if (argc == 2)
+ path = argv[1];
+
+ fd = open(path, 0, O_RDWR);
+ if (fd < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ return test_forked_set_min(fd, path);
+}
diff --git a/tools/testing/selftests/perf_qos/set_multiple_maxs.c b/tools/testing/selftests/perf_qos/set_multiple_maxs.c
new file mode 100644
index 000000000000..e30a81043283
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/set_multiple_maxs.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg;
+ const char *path = "/dev/perf_qos/dummy";
+ const int nr_fds = 256;
+ int i, fd[nr_fds];
+
+ if (argc == 2)
+ path = argv[1];
+
+ for (i = 0; i < nr_fds; i++) {
+
+ fd[i] = open(path, 0, O_RDWR);
+ if (fd[i] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * We want a value increasing so the value we set is
+ * always the first entry in the list of constraints
+ * and when we get the max, we get the last value we
+ * set.
+ */
+ arg.value = nr_fds - i;
+
+ if (ioctl(fd[i], PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd[i], PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != (nr_fds - i)) {
+ fprintf(stderr, "max value differs with set/get (arg=%d <> %d)\n",
+ arg.value, nr_fds - i);
+ return 1;
+ }
+ }
+
+ for (i = 0; i < nr_fds; i++)
+ close(fd[i]);
+
+ fd[0] = open(path, 0, O_RDWR);
+ if (fd[0] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * Test: Check the constraint is removed
+ */
+ if (!ioctl(fd[0], PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/set_multiple_mins.c b/tools/testing/selftests/perf_qos/set_multiple_mins.c
new file mode 100644
index 000000000000..e6412a592c3a
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/set_multiple_mins.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg;
+ const char *path = "/dev/perf_qos/dummy";
+ const int nr_fds = 256;
+ int i, fd[nr_fds];
+
+ if (argc == 2)
+ path = argv[1];
+
+ for (i = 0; i < nr_fds; i++) {
+
+ fd[i] = open(path, 0, O_RDWR);
+ if (fd[i] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ arg.value = i + 1;
+
+ if (ioctl(fd[i], PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd[i], PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != i + 1) {
+ fprintf(stderr, "min value differs with set/get (arg=%d <> %d)\n",
+ arg.value, nr_fds + 1);
+ return 1;
+ }
+ }
+
+ for (i = 0; i < nr_fds; i++)
+ close(fd[i]);
+
+ fd[0] = open(path, 0, O_RDWR);
+ if (fd[0] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * Test: Check the constraint is removed
+ */
+ if (!ioctl(fd[0], PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/set_same_multiple_maxs.c b/tools/testing/selftests/perf_qos/set_same_multiple_maxs.c
new file mode 100644
index 000000000000..ce36b28794fd
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/set_same_multiple_maxs.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg = { .value = 512 };
+ const char *path = "/dev/perf_qos/dummy";
+ const int nr_fds = 256;
+ int i, fd[nr_fds];
+
+ if (argc == 2)
+ path = argv[1];
+
+ for (i = 0; i < nr_fds; i++) {
+ fd[i] = open(path, 0, O_RDWR);
+ if (fd[i] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ if (ioctl(fd[i], PERF_QOS_IOC_SET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd[i], PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != 512) {
+ fprintf(stderr, "max value differs with set/get (arg=%d)\n",
+ arg.value);
+ return 1;
+ }
+ }
+
+ for (i = 0; i < nr_fds; i++)
+ close(fd[i]);
+
+ fd[0] = open(path, 0, O_RDWR);
+ if (fd[0] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * Test: Check the constraint is removed
+ */
+ if (!ioctl(fd[0], PERF_QOS_IOC_GET_MAX, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
diff --git a/tools/testing/selftests/perf_qos/set_same_multiple_mins.c b/tools/testing/selftests/perf_qos/set_same_multiple_mins.c
new file mode 100644
index 000000000000..90fd47be50f6
--- /dev/null
+++ b/tools/testing/selftests/perf_qos/set_same_multiple_mins.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Performance Quality of Service (Perf QoS) support base.
+ *
+ * Copyright (C) 2024 Linaro Ltd
+ *
+ * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
+ *
+ */
+#include <errno.h>
+#include <fcntl.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/poll.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+#include <linux/perf_qos_ioctl.h>
+
+int main(int argc, char *argv[])
+{
+ struct perf_qos_ioctl_arg arg = { .value = 512 };
+ const char *path = "/dev/perf_qos/dummy";
+ const int nr_fds = 256;
+ int i, fd[nr_fds];
+
+ if (argc == 2)
+ path = argv[1];
+
+ for (i = 0; i < nr_fds; i++) {
+ fd[i] = open(path, 0, O_RDWR);
+ if (fd[i] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ if (ioctl(fd[i], PERF_QOS_IOC_SET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ arg.value = 0;
+
+ if (ioctl(fd[i], PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "Failed to ioctl: %m\n");
+ return 1;
+ }
+
+ if (arg.value != 512) {
+ fprintf(stderr, "min value differs with set/get (arg=%d)\n",
+ arg.value);
+ return 1;
+ }
+ }
+
+ for (i = 0; i < nr_fds; i++)
+ close(fd[i]);
+
+ fd[0] = open(path, 0, O_RDWR);
+ if (fd[0] < 0) {
+ fprintf(stderr, "Failed to open '%s': %m\n", path);
+ return 1;
+ }
+
+ /*
+ * Test: Check the constraint is removed
+ */
+ if (!ioctl(fd[0], PERF_QOS_IOC_GET_MIN, &arg)) {
+ fprintf(stderr, "ioctl should have failed\n");
+ return 1;
+ }
+
+ if (errno != ENODATA) {
+ fprintf(stderr, "errno should have been ENODATA\n");
+ return 1;
+ }
+
+ return 0;
+}
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 1/2] power: Userspace performance QoS
2025-05-05 16:19 [RFC PATCH 1/2] power: Userspace performance QoS Daniel Lezcano
2025-05-05 16:19 ` [RFC PATCH 2/2] selftests: Add perf_qos selftests Daniel Lezcano
@ 2025-05-08 21:05 ` Rafael J. Wysocki
2025-05-09 9:54 ` Rafael J. Wysocki
1 sibling, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-05-08 21:05 UTC (permalink / raw)
To: Daniel Lezcano
Cc: rafael, linux-kernel, linux-pm, ulf.hansson, arnd, saravanak
On Mon, May 5, 2025 at 6:19 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
>
> In the embedded ecosystem, the Linux kernel is modified to integrate
> fake thermal cooling devices for the sake of the ABI exported in the
> sysfs.
>
> While investigating those different devices, it appears most of them
> could fall under a performance QoS feature.
>
> As discussed at the Linux Plumber Conference 2024, we want to let the
> userspace to access the device performance knob via a char device
> which would be created by the backend drivers and controlled with an
> ioctl.
>
> A performance constraint is a minimal or a maximal limit applied to a
> device performance state. A process can only set one constraint per
> limit, in other words a minimal performance and/or a maximal
> performance constraint. A new value will change the current
> constraint, not create a new one.
So how does this work for the constraints where
perf_qos_constraint_find() has returned a valid pointer in
perf_qos_set()? Is someone else's constraint updated without
notifying the original owner?
> If another constraint must be
> stacked with the current one, then the char device file must be opened
> again and the resulting new file descriptor must be used to create a
> new constraint.
>
> Constraint life cycle:
>
> The userspace can be a place where buggy programs with root privileges
> can tamper with the device performance. In order to prevent some dumb
> logics to set a device performance state and then go away, thus
> messing with the global system performance consistency, there is a
> particular care of the constraint life cycles. These ones are directly
> tied with the opened file descriptor of the char device. When it is
> released, then the constraint is removed but only if its refcount
> reaches zero.
So I'm totally unconvinced about the refcount thing.
I personally don't think that sharing constraints is a good idea at
all. In principle, it doesn't matter that the current constraint
value is the same as somebody else's constraint value: they are
different constraints because they have been set by different
entities. It should be possible to update any of them independently
at any time and the involved complexity is not worth the memory usage
reduction achieved by sharing constraints.
As an optimization, it is premature at best IMV.
> This situation exists if only process sets the
> constraint and then closes the file descriptor (manually or at exit
> time). If the process forks multiple time and the children inherit the
> file descriptor, the constraint will be removed when all the children
> close the file descriptor.
>
> However, if another process opens the char device and sets a
> constraint which already exists then that results in incrementing the
> refcount of the constraint. The constraint is then removed when all
> the processes have closed their file descriptor pointing to the char
> device.
>
> At creation time:
>
> - if another process asked for the same limit of performance, then
> the refcount constraint is incremented
>
> - if there is an existing constraint with a higher priority, then the
> requested constraint is queued in the ordered list of constraints
>
> - if there is an existing constraint with a lower limit, then the
> requested constrained is applied and the current constraint is
> queued in the ordered list of constraints
>
> At removal time:
>
> - if the removed constraint is the current one, then the next
> constraint in the ordered list is applied
>
> - if the removed constraint is not the current one, then it is simply
> removed from the ordered list
>
> The changes allows the userspace to set a performance constraint for a
> specific device but the kernel may also want to apply a performance
> constraint. The in-kernel API is not yet implemented as it represents
> a significant amount of work depending on the direction of this patch.
Apart from the above, I'm not sure if the locking is sufficient and
there are a few minor nits below.
However, at the general level, an in-kernel user of this is missing
which is needed to illustrate how this is going to be integrated with
the existing code. That is, what happens when user space sets a
constraint for a specific device, where that constraint is applied and
how exactly.
Also, in the cooling device interface, the user space agent chooses
the state to put the device into, which is not a constraint, but a
representation of the desired performance (or thermal pressure if you
will). This QoS interface instead operates min and max limits, so how
are the users supposed to know what to do with it? Is setting the max
limit equivalent to setting a specific cooling state? If so, then
what's the role of the min limit? And what about the soft limit
values? How are they supposed to be used?
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> include/linux/perf_qos.h | 45 ++
> include/uapi/linux/perf_qos_ioctl.h | 47 ++
> kernel/power/Makefile | 2 +-
> kernel/power/perf_qos.c | 652 ++++++++++++++++++++++++++++
> 4 files changed, 745 insertions(+), 1 deletion(-)
> create mode 100644 include/linux/perf_qos.h
> create mode 100644 include/uapi/linux/perf_qos_ioctl.h
> create mode 100644 kernel/power/perf_qos.c
>
> diff --git a/include/linux/perf_qos.h b/include/linux/perf_qos.h
> new file mode 100644
> index 000000000000..57529c40be4d
> --- /dev/null
> +++ b/include/linux/perf_qos.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Performance QoS device abstraction
> + *
> + * Copyright (2024) Linaro Ltd
> + *
> + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> + *
> + */
> +#ifndef __PERF_QOS_H
> +#define __PERF_QOS_H
> +
> +#include <uapi/linux/perf_qos_ioctl.h>
> +
> +struct perf_qos;
> +
> +/**
> + * struct perf_qos_value_descr - Performance constraint description
> + *
> + * @unit: the unit used for the constraint (normalized, throughput, ...)
> + * @limit_min: the minimal constraint limit to be set
> + * @limit_max: the maximal constraint limit to be set
> + */
> +struct perf_qos_value_descr {
> + perf_qos_unit_t unit;
"enum perf_qos_unit" would be better IMV.
Also, why is this enum needed at all?
> + int limit_min;
> + int limit_max;
Why not just min and max?
> +};
> +
> +typedef int (*set_perf_limit_cb_t)(int);
> +
> +struct perf_qos_ops {
> + set_perf_limit_cb_t set_perf_limit_max;
> + set_perf_limit_cb_t set_perf_limit_min;
> +};
> +
> +extern struct perf_qos *perf_qos_device_create(const char *name,
> + struct perf_qos_ops *ops,
> + struct perf_qos_value_descr *descr);
> +
> +extern int perf_qos_is_allowed(struct perf_qos *pq, int performance);
> +
> +extern void perf_qos_device_destroy(struct perf_qos *pq);
> +
> +#endif
> diff --git a/include/uapi/linux/perf_qos_ioctl.h b/include/uapi/linux/perf_qos_ioctl.h
> new file mode 100644
> index 000000000000..a9fb8940c175
> --- /dev/null
> +++ b/include/uapi/linux/perf_qos_ioctl.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
> +/*
> + * Performance QoS device abstraction
> + *
> + * Copyright (2024) Linaro Ltd
> + *
> + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> + *
> + */
> +#ifndef __PERF_QOS_IOCTL_H
> +#define __PERF_QOS_IOCTL_H
> +
> +#include <linux/types.h>
> +
> +enum {
> + PERF_QOS_IOC_SET_MIN_CMD,
> + PERF_QOS_IOC_GET_MIN_CMD,
> + PERF_QOS_IOC_SET_MAX_CMD,
> + PERF_QOS_IOC_GET_MAX_CMD,
> + PERF_QOS_IOC_GET_UNIT_CMD,
> + PERF_QOS_IOC_GET_LIMITS_CMD,
What's this one for?
> + PERF_QOS_IOC_MAX_CMD,
> +};
It would be nice to document this interface somehow, so it is not
necessary to reverse-engineer the code to find out how it is expected
to work.
> +
> +typedef enum {
> + PERF_QOS_UNIT_NORMAL,
> + PERF_QOS_UNIT_KBPS,
> + PERF_QOS_UNIT_MAX
> +} perf_qos_unit_t;
This is just an enum type. What's the typedef for?
> +
> +struct perf_qos_ioctl_arg {
> + int value;
> + int limit_min;
> + int limit_max;
Again, why not min and max?
> + perf_qos_unit_t unit;
> +};
> +
> +#define PERF_QOS_IOCTL_TYPE 'P'
> +
> +#define PERF_QOS_IOC_SET_MIN _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MIN_CMD, struct perf_qos_ioctl_arg *)
> +#define PERF_QOS_IOC_GET_MIN _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MIN_CMD, struct perf_qos_ioctl_arg *)
> +#define PERF_QOS_IOC_SET_MAX _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MAX_CMD, struct perf_qos_ioctl_arg *)
> +#define PERF_QOS_IOC_GET_MAX _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MAX_CMD, struct perf_qos_ioctl_arg *)
> +#define PERF_QOS_IOC_GET_UNIT _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_UNIT_CMD, struct perf_qos_ioctl_arg *)
> +#define PERF_QOS_IOC_GET_LIMITS _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_LIMITS_CMD, struct perf_qos_ioctl_arg *)
> +
> +#endif
> diff --git a/kernel/power/Makefile b/kernel/power/Makefile
> index 874ad834dc8d..e2e4d707ab6e 100644
> --- a/kernel/power/Makefile
> +++ b/kernel/power/Makefile
> @@ -8,7 +8,7 @@ endif
>
> KASAN_SANITIZE_snapshot.o := n
>
> -obj-y += qos.o
> +obj-y += qos.o perf_qos.o
> obj-$(CONFIG_PM) += main.o
> obj-$(CONFIG_VT_CONSOLE_SLEEP) += console.o
> obj-$(CONFIG_FREEZER) += process.o
> diff --git a/kernel/power/perf_qos.c b/kernel/power/perf_qos.c
> new file mode 100644
> index 000000000000..ca0619b07ae5
> --- /dev/null
> +++ b/kernel/power/perf_qos.c
> @@ -0,0 +1,652 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Performance Quality of Service (Perf QoS) support base.
> + *
> + * Copyright (C) 2024 Linaro Ltd
> + *
> + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> + *
I would expect some description of what the code in this file is for here.
> + */
> +#include <linux/cdev.h>
> +#include <linux/perf_qos.h>
> +#include <linux/list_sort.h>
> +
> +#define DEVNAME "perf_qos"
> +#define NUM_PERF_QOS_MINORS 128
> +
> +static DEFINE_IDR(perf_qos_minors);
> +static struct class *perf_qos_class;
> +static dev_t perf_qos_devt;
> +
> +typedef enum {
> + PERF_QOS_LIMIT_MAX,
> + PERF_QOS_LIMIT_MIN,
PERF_QOS_MAX/MIN?
> +} perf_qos_limit_t;
Why would "enum perf_qos_type" be insufficient?
> +
> +/**
> + * struct perf_qos_constraint - structure holding a constraint information
> + *
> + * @soft_limit: an integer corresponding of the limit value set
> + * @hard_limit: an integer corresponding to the limit value allowed by the driver
> + * @kref: a refcount to the constraint responsible of its life cycle
> + * @set_perf_limit_cb: a callback to notify the backend driver about the limit change
> + * @node: the list node to attach this constraint with the list of constraints
> + * @head: the list of constraints the @node
> + *
> + * This structure has a couple of instanciation per perf QoS file
> + * opened by a process. The process can apply one or two constraints
> + * to the device.
> + *
> + * Other processes will allocate their own constraints which will be
> + * added in the list of constraints.
In the absence of general documentation, this comment doesn't help too
much I'm afraid.
> + */
> +struct perf_qos_constraint {
> + int soft_limit;
> + int hard_limit;
> + set_perf_limit_cb_t set_perf_limit_cb;
> + struct kref kref;
> + struct list_head node;
> + struct list_head *head;
> +};
> +
> +/**
> + * struct perf_qos - structure owning the constraint information for
> + * the device
> + *
> + * @lock: lock to protect the actions on the list of constraints
> + * @perf_qos_cdev: a struct cdev used for the device destruction
> + * @ops: the ops given by the backend driver to notify the change of constraint
> + * @descr: a constraint descriptor giving the units and the boundaries
The meaning of this is kind of unclear.
> + * @perf_min: the list of the minimal performance constraints
> + * @perf_max: the list of the maximal performance constraints
s/maximal/maximum/
> + */
> +struct perf_qos {
> + spinlock_t lock;
Why spinlock?
> + struct cdev perf_qos_cdev;
Why not just cdev?
> + struct perf_qos_ops *ops;
> + struct perf_qos_value_descr *descr;
> + struct list_head perf_min;
> + struct list_head perf_max;
I would prefer constraiints_min and constraints_max.
> +};
> +
> +/**
> + * struct perf_qos_data - structure with the requested constraints
> + *
> + * @pqc_min: the requested performance constraint giving the minimal value
> + * @pqc_max: the requested performance constraint giving the maximal value
> + */
> +struct perf_qos_data {
> + struct perf_qos_constraint *pqc_min;
> + struct perf_qos_constraint *pqc_max;
> +};
> +
> +static struct perf_qos_constraint *perf_qos_constraint_find(struct list_head *list, int value)
> +{
> + struct perf_qos_constraint *pcq;
> +
> + list_for_each_entry(pcq, list, node) {
> + if (pcq->soft_limit == value)
> + return pcq;
Why does this check soft_limit and not hard_limit?
> + }
> +
> + return NULL;
> +}
> +
> +static int perf_qos_constraint_cmp(void *data,
> + const struct list_head *l1,
> + const struct list_head *l2)
> +{
> + struct perf_qos_constraint *pqc1 = container_of(l1, struct perf_qos_constraint, node);
> + struct perf_qos_constraint *pqc2 = container_of(l2, struct perf_qos_constraint, node);
> +
> + /*
> + * The comparison will depend if we apply a max or min
> + * performance constraint. If the soft limit is lesser than
"less than"
> + * the hard limit, that means it is a maximum limitation.
> + */
> + if (pqc1->soft_limit < pqc1->hard_limit)
> + return pqc1->soft_limit - pqc2->soft_limit;
> +
> + return pqc2->soft_limit - pqc1->soft_limit;
Again, why is this only comparing the soft limits?
> +}
> +
> +static int perf_qos_del(struct perf_qos_constraint *pcq)
> +{
> + const struct perf_qos_constraint *first;
> + int new_limit;
> +
I gather that this runs under a perf_qos lock.
> + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> +
> + list_del(&pcq->node);
> +
> + /*
> + * The active constraint is not the one we removed, so there
> + * is nothing more to do
> + */
> + if (first != pcq)
> + return 0;
> +
> + /*
> + * As we remove the first entry, then get the new first entry
> + * to apply the next constraint. If there is no more
> + * constraint set, reset to the original limit. Otherwise, use
> + * the new constraint value.
> + */
> + if (list_empty(pcq->head))
> + new_limit = pcq->hard_limit;
I don't quite get it, sorry. Shouldn't the new limit be a "no limit" here?
> + else {
> + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> + new_limit = first->soft_limit;
> + };
> +
> + /*
> + * Notify the backend driver to update its performance level
> + * if needed. If the performance level is currently inside the
> + * new limits, nothing will happen. Otherwise it must be
> + * adjust the current performance level to be inside the
> + * authorized limits
> + */
> + pcq->set_perf_limit_cb(new_limit);
So how's the provider of this callback supposed to know if this is the
min or the max limit?
> +
> + return 1;
> +}
> +
> +static int perf_qos_add(struct perf_qos_constraint *pcq)
> +{
> + const struct perf_qos_constraint *first;
> +
I gather that this runs under a perf_qos lock.
> + list_add(&pcq->node, pcq->head);
> +
> + list_sort(NULL, pcq->head, perf_qos_constraint_cmp);
And this may take some time in principle, so running it under a
spinlock may not be a good idea.
> +
> + /*
> + * A sort happened resulting in a different constraint at the head
> + */
> + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> +
> + /*
> + * The inserted constraint did not become the active one, so
> + * we can bail out
> + */
> + if (pcq != first)
> + return 0;
> +
> + /*
> + * Notify the backend driver to update its performance level
> + * if needed. If the performance level is currently inside the
> + * new limits, nothing will happen. Otherwise it must be
> + * adjust the current performance level to be inside the
> + * authorized limits
> + */
> + pcq->set_perf_limit_cb(first->soft_limit);
Again, how's the backend going to know which limit this is?
> +
> + return 1;
> +}
> +
> +static void perf_qos_constraint_release(struct kref *kref)
> +{
> + struct perf_qos_constraint *pcq;
> +
> + pcq = container_of(kref, struct perf_qos_constraint, kref);
> +
> + /*
> + * The removal of the constraint results in the change of the
> + * first entry of the list which means it was the active
> + * one. We need to apply the next constraint of the list
> + */
> + if (perf_qos_del(pcq)) {
> + /* Something to do */
Missing code?
> + }
> +
> + kfree(pcq);
> +}
> +
> +static void perf_qos_constraint_put(struct perf_qos_constraint *pcq)
> +{
> + kref_put(&pcq->kref, perf_qos_constraint_release);
> +}
> +
> +static void perf_qos_constraint_get(struct perf_qos_constraint *pcq)
> +{
> + kref_get(&pcq->kref);
> +}
> +
> +static struct perf_qos_constraint *perf_qos_constraint_alloc(struct perf_qos *pq, int soft_limit,
> + struct list_head *perf, perf_qos_limit_t limit)
> +{
> + struct perf_qos_constraint *pqc;
> +
> + pqc = kzalloc(sizeof(*pqc), GFP_KERNEL);
> + if (!pqc)
> + return NULL;
> +
> + kref_init(&pqc->kref);
> + INIT_LIST_HEAD(&pqc->node);
> +
> + if (limit == PERF_QOS_LIMIT_MAX) {
> + pqc->set_perf_limit_cb = pq->ops->set_perf_limit_max;
> + pqc->hard_limit = pq->descr->limit_max;
Oh, I see where the hard limits come from. Well, this is not
particularly straightforward.
> + } else {
> + pqc->set_perf_limit_cb = pq->ops->set_perf_limit_min;
> + pqc->hard_limit = pq->descr->limit_min;
> + }
> +
> + pqc->head = perf;
> + pqc->soft_limit = soft_limit;
> +
> + return pqc;
> +}
> +
> +static int perf_qos_open(struct inode *inode, struct file *file)
> +{
> + struct perf_qos_data *pqd;
> + struct perf_qos *pq;
> +
> + pq = idr_find(&perf_qos_minors, iminor(inode));
> + if (!pq)
> + return -ENODEV;
> +
> + inode->i_private = pq;
> +
> + pqd = kzalloc(sizeof(*pqd), GFP_KERNEL);
> + if (!pqd)
> + return -ENOMEM;
> +
> + file->private_data = pqd;
> +
> + return 0;
> +}
> +
> +static int perf_qos_release(struct inode *inode, struct file *file)
> +{
> + struct perf_qos *pq = inode->i_private;
> + struct perf_qos_data *pqd = file->private_data;
> +
> + spin_lock(&pq->lock);
> +
> + if (pqd->pqc_min)
> + perf_qos_constraint_put(pqd->pqc_min);
> +
> + if (pqd->pqc_max)
> + perf_qos_constraint_put(pqd->pqc_max);
> +
> + spin_unlock(&pq->lock);
> +
> + kfree(pqd);
> +
> + return 0;
> +}
> +
> +static int perf_qos_unset(struct perf_qos_constraint **cur_pqc,
> + perf_qos_limit_t limit, struct list_head *perf, int value)
> +{
> + /*
> + * Removing a constraint:
> + *
> + * - if it exists then *current_pqc is set. We decrement the
> + * refcount and update the current constraint by setting it
> + * to NULL
> + *
> + * - if the current constraint does not exist then, it is an
> + * error and we should exit with an error
> + */
> + if (!(*cur_pqc))
> + return -EINVAL;
> +
> + perf_qos_constraint_put(*cur_pqc);
> + *cur_pqc = NULL;
> +
> + return 0;
> +}
> +
> +static int perf_qos_set(struct perf_qos *pq, struct perf_qos_constraint **cur_pqc,
> + perf_qos_limit_t limit, struct list_head *perf, int value)
> +{
> + struct perf_qos_constraint *pqc;
> + int ret = 0;
> +
> + /*
> + * We are trying to set the same constraint.
> + */
> + if (*cur_pqc && ((*cur_pqc)->soft_limit == value)) {
> + ret = -EALREADY;
> + goto out;
> + }
> +
> + /*
> + * Case 2 : Adding a constraint:
> + *
> + * - it already exists because it was created by another
> + * process, we increment the refcount
> + *
> + * - it already exists because we created it before, we
> + * return an error
> + *
> + * - it does not exist but there is a previous different
> + * constraint we set before. It is a constraint change. We
> + * must release the previous constraint and create a new
> + * one. However, we apply the new constraint and then we
> + * remove the old one in order to not have the backend
> + * driver with a window where there is no constraint at all
> + *
> + * - it does not exist and there is no previous constraint. It
> + * is a new constraint. We allocate the constraint, apply it
> + * and set it as the current constraint
> + */
> + pqc = perf_qos_constraint_find(perf, value);
> + if (pqc) {
> + perf_qos_constraint_get(pqc);
This is the part I'm not a fan of.
> + } else {
> + pqc = perf_qos_constraint_alloc(pq, value, perf, limit);
> + if (!pqc) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + /*
> + * The new constraint has to be applied because it
> + * results in a change of the first entry of the list
> + * of constraints
> + */
> + if (perf_qos_add(pqc)) {
> + /* Something to do */
> + }
> + }
> +
> + /*
> + * We previously set a constraint, let's release the refcount
> + * as we change it. The constraint can be freed if we are the
> + * last one having a reference to it or if we are the creator
> + * and no other process held a refcount on it.
> + */
> + if ((*cur_pqc))
> + perf_qos_constraint_put(*cur_pqc);
> +
> + *cur_pqc = pqc;
> +out:
> + return ret;
> +}
> +
> +static int ioctl_perf_qos_set_max(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia)
> +{
> + if (pqia->value > pq->descr->limit_max)
> + return -EINVAL;
> +
> + if (pqia->value == pq->descr->limit_max)
> + return perf_qos_unset(&pqd->pqc_max, PERF_QOS_LIMIT_MAX,
> + &pq->perf_max, pqia->value);
> + else
> + return perf_qos_set(pq, &pqd->pqc_max, PERF_QOS_LIMIT_MAX,
> + &pq->perf_max, pqia->value);
> +}
> +
> +static int ioctl_perf_qos_set_min(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia)
> +{
> + if (pqia->value < pq->descr->limit_min)
> + return -EINVAL;
> +
> + if (pqia->value == pq->descr->limit_min)
> + return perf_qos_unset(&pqd->pqc_min, PERF_QOS_LIMIT_MIN,
> + &pq->perf_min, pqia->value);
> + else
> + return perf_qos_set(pq, &pqd->pqc_min, PERF_QOS_LIMIT_MIN,
> + &pq->perf_min, pqia->value);
> +}
> +
> +static int perf_qos_get(struct list_head *perf, int *value)
> +{
> + struct perf_qos_constraint *pqc;
> +
> + /*
> + * We may not have set any performance constraint yet but
> + * another process may have set one, so we get the head of
> + * performance constraint list
> + */
> + if (list_empty(perf))
> + return -ENODATA;
> +
> + pqc = list_first_entry(perf, struct perf_qos_constraint, node);
> +
> + *value = pqc->soft_limit;
> +
> + return 0;
> +}
> +
> +static int ioctl_perf_qos_get_min(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia)
> +{
> + return perf_qos_get(&pq->perf_min, &pqia->value);
> +}
> +
> +static int ioctl_perf_qos_get_max(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia)
> +{
> + return perf_qos_get(&pq->perf_max, &pqia->value);
> +}
> +
> +static int ioctl_perf_qos_get_unit(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia)
> +{
> + pqia->unit = pq->descr->unit;
> +
> + return 0;
> +}
> +
> +static int ioctl_perf_qos_get_limits(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia)
> +{
> + pqia->limit_min = pq->descr->limit_min;
> + pqia->limit_max = pq->descr->limit_max;
> +
> + return 0;
> +}
> +
> +typedef int (*perf_qos_ioctl_ops_t)(struct perf_qos *pq,
> + struct perf_qos_data *pqd,
> + struct perf_qos_ioctl_arg *pqia);
> +
> +static long perf_qos_ioctl(struct file *file, unsigned int ucmd,
> + unsigned long arg)
> +{
> + struct perf_qos_data *pqd = file->private_data;
> + struct perf_qos *pq = file->f_inode->i_private;
> + struct perf_qos_ioctl_arg pqia;
> + int cmd = _IOC_NR(ucmd);
> + int dir = _IOC_DIR(ucmd);
> + int type = _IOC_TYPE(ucmd);
> + int ret;
> +
> + perf_qos_ioctl_ops_t perf_qos_ioctl_ops[] = {
> + [PERF_QOS_IOC_SET_MAX_CMD] = ioctl_perf_qos_set_max,
> + [PERF_QOS_IOC_SET_MIN_CMD] = ioctl_perf_qos_set_min,
> + [PERF_QOS_IOC_GET_MAX_CMD] = ioctl_perf_qos_get_max,
> + [PERF_QOS_IOC_GET_MIN_CMD] = ioctl_perf_qos_get_min,
> + [PERF_QOS_IOC_GET_UNIT_CMD] = ioctl_perf_qos_get_unit,
> + [PERF_QOS_IOC_GET_LIMITS_CMD] = ioctl_perf_qos_get_limits,
> + };
> +
> + if (type != PERF_QOS_IOCTL_TYPE)
> + return -EINVAL;
> +
> + if (cmd < 0 || cmd >= PERF_QOS_IOC_MAX_CMD)
> + return -EINVAL;
> +
> + if (dir & _IOC_WRITE) {
> + if (copy_from_user(&pqia, (typeof(pqia) *)arg, sizeof(pqia)))
> + return -EACCES;
> + }
> +
> + spin_lock(&pq->lock);
> + ret = perf_qos_ioctl_ops[cmd](pq, pqd, &pqia);
> + spin_unlock(&pq->lock);
> +
> + if (ret)
> + goto out;
> +
> + if (dir & _IOC_READ) {
> + if (copy_to_user((typeof(pqia) *)arg, &pqia, sizeof(pqia)))
> + return -EACCES;
> + }
> +out:
> + return ret;
> +}
> +
> +static const struct file_operations perf_qos_fops = {
> + .owner = THIS_MODULE,
> + .open = perf_qos_open,
> + .release = perf_qos_release,
> + .unlocked_ioctl = perf_qos_ioctl,
> +#ifdef CONFIG_COMPAT
> + .compat_ioctl = perf_qos_ioctl,
> +#endif
> +};
> +
Missing kerneldoc.
> +void perf_qos_device_destroy(struct perf_qos *pq)
> +{
> + idr_remove(&perf_qos_minors, MINOR(pq->perf_qos_cdev.dev));
> + device_destroy(perf_qos_class, pq->perf_qos_cdev.dev);
> + cdev_del(&pq->perf_qos_cdev);
> + kfree(pq->descr);
> + kfree(pq->ops);
> + kfree(pq);
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_device_destroy);
> +
Missing kerneldoc.
> +int perf_qos_is_allowed(struct perf_qos *pq, int performance)
> +{
> + const struct perf_qos_constraint *first;
> + int allowed = 1;
> +
> + spin_lock(&pq->lock);
> +
> + first = list_first_entry(&pq->perf_min, struct perf_qos_constraint, node);
> + if (performance < first->soft_limit)
> + allowed = 0;
> +
> + first = list_first_entry(&pq->perf_max, struct perf_qos_constraint, node);
> + if (performance > first->soft_limit)
> + allowed = 0;
> +
> + spin_unlock(&pq->lock);
> +
> + return allowed;
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_is_allowed);
> +
Missing kerneldoc.
> +struct perf_qos *perf_qos_device_create(const char *name,
> + struct perf_qos_ops *ops,
> + struct perf_qos_value_descr *descr)
> +{
> + struct device *dev;
> + struct perf_qos *pq;
> + dev_t devt;
> + int minor;
> + int ret;
> +
> + if (!ops->set_perf_limit_max || !ops->set_perf_limit_min)
> + return ERR_PTR(-EINVAL);
> +
> + if (descr->unit < 0 || descr->unit >= PERF_QOS_UNIT_MAX)
> + return ERR_PTR(-EINVAL);
> +
> + if (descr->limit_min > descr->limit_max)
> + return ERR_PTR(-EINVAL);
> +
> + if (descr->unit == PERF_QOS_UNIT_NORMAL) {
> + if (descr->limit_min < 0 || descr->limit_max > 1024)
> + return ERR_PTR(-EINVAL);
> + }
> +
> + pq = kzalloc(sizeof(*pq), GFP_KERNEL);
> + if (!pq)
> + return ERR_PTR(-ENOMEM);
> +
> + INIT_LIST_HEAD(&pq->perf_min);
> + INIT_LIST_HEAD(&pq->perf_max);
> + spin_lock_init(&pq->lock);
> +
> + pq->ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> + if (!pq->ops) {
> + ret = -ENOMEM;
> + goto out_kfree_pq;
> + }
> +
> + pq->descr = kmemdup(descr, sizeof(*descr), GFP_KERNEL);
> + if (!pq->descr) {
> + ret = -ENOMEM;
> + goto out_kfree_pq_ops;
> + }
> +
> + minor = idr_alloc(&perf_qos_minors, pq, 0,
> + NUM_PERF_QOS_MINORS, GFP_KERNEL);
> + if (minor < 0)
> + goto out_kfree_pq_descr;
> +
> + devt = MKDEV(MAJOR(perf_qos_devt), minor);
> +
> + cdev_init(&pq->perf_qos_cdev, &perf_qos_fops);
> +
> + ret = cdev_add(&pq->perf_qos_cdev, devt, 1);
> + if (ret < 0)
> + goto out_idr_remove;
> +
> + dev = device_create(perf_qos_class, NULL, devt, NULL, name);
> + if (IS_ERR(dev)) {
> + ret = PTR_ERR(dev);
> + goto out_cdev_del;
> + }
> +
> + return pq;
> +
> +out_cdev_del:
> + cdev_del(&pq->perf_qos_cdev);
> +
> +out_idr_remove:
> + idr_remove(&perf_qos_minors, minor);
> +
> +out_kfree_pq_descr:
> + kfree(pq->descr);
> +
> +out_kfree_pq_ops:
> + kfree(pq->ops);
> +
> +out_kfree_pq:
> + kfree(pq);
> +
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_GPL(perf_qos_device_create);
> +
> +static char *perf_qos_devnode(const struct device *dev, umode_t *mode)
> +{
> + return kasprintf(GFP_KERNEL, "%s/%s", DEVNAME, dev_name(dev));
> +}
> +
> +static int perf_qos_init(void)
> +{
> + int ret;
> +
> + ret = alloc_chrdev_region(&perf_qos_devt, 0,
> + NUM_PERF_QOS_MINORS, DEVNAME);
> + if (ret)
> + return ret;
> +
> + perf_qos_class = class_create(DEVNAME);
> + if (IS_ERR(perf_qos_class)) {
> + unregister_chrdev_region(perf_qos_devt, NUM_PERF_QOS_MINORS);
> + return PTR_ERR(perf_qos_class);
> + }
> + perf_qos_class->devnode = perf_qos_devnode;
> +
> + return 0;
> +}
> +
> +subsys_initcall(perf_qos_init);
> --
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 1/2] power: Userspace performance QoS
2025-05-08 21:05 ` [RFC PATCH 1/2] power: Userspace performance QoS Rafael J. Wysocki
@ 2025-05-09 9:54 ` Rafael J. Wysocki
2025-05-10 12:38 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-05-09 9:54 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-kernel, linux-pm, ulf.hansson, arnd, saravanak
On Thu, May 8, 2025 at 11:05 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Mon, May 5, 2025 at 6:19 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> >
> > In the embedded ecosystem, the Linux kernel is modified to integrate
> > fake thermal cooling devices for the sake of the ABI exported in the
> > sysfs.
> >
> > While investigating those different devices, it appears most of them
> > could fall under a performance QoS feature.
> >
> > As discussed at the Linux Plumber Conference 2024, we want to let the
> > userspace to access the device performance knob via a char device
> > which would be created by the backend drivers and controlled with an
> > ioctl.
> >
> > A performance constraint is a minimal or a maximal limit applied to a
> > device performance state. A process can only set one constraint per
> > limit, in other words a minimal performance and/or a maximal
> > performance constraint. A new value will change the current
> > constraint, not create a new one.
>
> So how does this work for the constraints where
> perf_qos_constraint_find() has returned a valid pointer in
> perf_qos_set()? Is someone else's constraint updated without
> notifying the original owner?
>
> > If another constraint must be
> > stacked with the current one, then the char device file must be opened
> > again and the resulting new file descriptor must be used to create a
> > new constraint.
> >
> > Constraint life cycle:
> >
> > The userspace can be a place where buggy programs with root privileges
> > can tamper with the device performance. In order to prevent some dumb
> > logics to set a device performance state and then go away, thus
> > messing with the global system performance consistency, there is a
> > particular care of the constraint life cycles. These ones are directly
> > tied with the opened file descriptor of the char device. When it is
> > released, then the constraint is removed but only if its refcount
> > reaches zero.
>
> So I'm totally unconvinced about the refcount thing.
>
> I personally don't think that sharing constraints is a good idea at
> all. In principle, it doesn't matter that the current constraint
> value is the same as somebody else's constraint value: they are
> different constraints because they have been set by different
> entities. It should be possible to update any of them independently
> at any time and the involved complexity is not worth the memory usage
> reduction achieved by sharing constraints.
>
> As an optimization, it is premature at best IMV.
>
> > This situation exists if only process sets the
> > constraint and then closes the file descriptor (manually or at exit
> > time). If the process forks multiple time and the children inherit the
> > file descriptor, the constraint will be removed when all the children
> > close the file descriptor.
> >
> > However, if another process opens the char device and sets a
> > constraint which already exists then that results in incrementing the
> > refcount of the constraint. The constraint is then removed when all
> > the processes have closed their file descriptor pointing to the char
> > device.
> >
> > At creation time:
> >
> > - if another process asked for the same limit of performance, then
> > the refcount constraint is incremented
> >
> > - if there is an existing constraint with a higher priority, then the
> > requested constraint is queued in the ordered list of constraints
> >
> > - if there is an existing constraint with a lower limit, then the
> > requested constrained is applied and the current constraint is
> > queued in the ordered list of constraints
> >
> > At removal time:
> >
> > - if the removed constraint is the current one, then the next
> > constraint in the ordered list is applied
> >
> > - if the removed constraint is not the current one, then it is simply
> > removed from the ordered list
> >
> > The changes allows the userspace to set a performance constraint for a
> > specific device but the kernel may also want to apply a performance
> > constraint. The in-kernel API is not yet implemented as it represents
> > a significant amount of work depending on the direction of this patch.
>
> Apart from the above, I'm not sure if the locking is sufficient and
> there are a few minor nits below.
>
> However, at the general level, an in-kernel user of this is missing
> which is needed to illustrate how this is going to be integrated with
> the existing code. That is, what happens when user space sets a
> constraint for a specific device, where that constraint is applied and
> how exactly.
>
> Also, in the cooling device interface, the user space agent chooses
> the state to put the device into, which is not a constraint, but a
> representation of the desired performance (or thermal pressure if you
> will). This QoS interface instead operates min and max limits, so how
> are the users supposed to know what to do with it? Is setting the max
> limit equivalent to setting a specific cooling state? If so, then
> what's the role of the min limit? And what about the soft limit
> values? How are they supposed to be used?
>
> > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > ---
> > include/linux/perf_qos.h | 45 ++
> > include/uapi/linux/perf_qos_ioctl.h | 47 ++
> > kernel/power/Makefile | 2 +-
> > kernel/power/perf_qos.c | 652 ++++++++++++++++++++++++++++
> > 4 files changed, 745 insertions(+), 1 deletion(-)
> > create mode 100644 include/linux/perf_qos.h
> > create mode 100644 include/uapi/linux/perf_qos_ioctl.h
> > create mode 100644 kernel/power/perf_qos.c
> >
> > diff --git a/include/linux/perf_qos.h b/include/linux/perf_qos.h
> > new file mode 100644
> > index 000000000000..57529c40be4d
> > --- /dev/null
> > +++ b/include/linux/perf_qos.h
> > @@ -0,0 +1,45 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +/*
> > + * Performance QoS device abstraction
> > + *
> > + * Copyright (2024) Linaro Ltd
> > + *
> > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> > + *
> > + */
> > +#ifndef __PERF_QOS_H
> > +#define __PERF_QOS_H
> > +
> > +#include <uapi/linux/perf_qos_ioctl.h>
> > +
> > +struct perf_qos;
> > +
> > +/**
> > + * struct perf_qos_value_descr - Performance constraint description
> > + *
> > + * @unit: the unit used for the constraint (normalized, throughput, ...)
> > + * @limit_min: the minimal constraint limit to be set
> > + * @limit_max: the maximal constraint limit to be set
> > + */
> > +struct perf_qos_value_descr {
> > + perf_qos_unit_t unit;
>
> "enum perf_qos_unit" would be better IMV.
>
> Also, why is this enum needed at all?
>
> > + int limit_min;
> > + int limit_max;
>
> Why not just min and max?
>
> > +};
> > +
> > +typedef int (*set_perf_limit_cb_t)(int);
> > +
> > +struct perf_qos_ops {
> > + set_perf_limit_cb_t set_perf_limit_max;
> > + set_perf_limit_cb_t set_perf_limit_min;
> > +};
> > +
> > +extern struct perf_qos *perf_qos_device_create(const char *name,
> > + struct perf_qos_ops *ops,
> > + struct perf_qos_value_descr *descr);
> > +
> > +extern int perf_qos_is_allowed(struct perf_qos *pq, int performance);
> > +
> > +extern void perf_qos_device_destroy(struct perf_qos *pq);
> > +
> > +#endif
> > diff --git a/include/uapi/linux/perf_qos_ioctl.h b/include/uapi/linux/perf_qos_ioctl.h
> > new file mode 100644
> > index 000000000000..a9fb8940c175
> > --- /dev/null
> > +++ b/include/uapi/linux/perf_qos_ioctl.h
> > @@ -0,0 +1,47 @@
> > +/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
> > +/*
> > + * Performance QoS device abstraction
> > + *
> > + * Copyright (2024) Linaro Ltd
> > + *
> > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> > + *
> > + */
> > +#ifndef __PERF_QOS_IOCTL_H
> > +#define __PERF_QOS_IOCTL_H
> > +
> > +#include <linux/types.h>
> > +
> > +enum {
> > + PERF_QOS_IOC_SET_MIN_CMD,
> > + PERF_QOS_IOC_GET_MIN_CMD,
> > + PERF_QOS_IOC_SET_MAX_CMD,
> > + PERF_QOS_IOC_GET_MAX_CMD,
> > + PERF_QOS_IOC_GET_UNIT_CMD,
> > + PERF_QOS_IOC_GET_LIMITS_CMD,
>
> What's this one for?
>
> > + PERF_QOS_IOC_MAX_CMD,
> > +};
>
> It would be nice to document this interface somehow, so it is not
> necessary to reverse-engineer the code to find out how it is expected
> to work.
>
> > +
> > +typedef enum {
> > + PERF_QOS_UNIT_NORMAL,
> > + PERF_QOS_UNIT_KBPS,
> > + PERF_QOS_UNIT_MAX
> > +} perf_qos_unit_t;
>
> This is just an enum type. What's the typedef for?
>
> > +
> > +struct perf_qos_ioctl_arg {
> > + int value;
> > + int limit_min;
> > + int limit_max;
>
> Again, why not min and max?
>
> > + perf_qos_unit_t unit;
> > +};
> > +
> > +#define PERF_QOS_IOCTL_TYPE 'P'
> > +
> > +#define PERF_QOS_IOC_SET_MIN _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MIN_CMD, struct perf_qos_ioctl_arg *)
> > +#define PERF_QOS_IOC_GET_MIN _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MIN_CMD, struct perf_qos_ioctl_arg *)
> > +#define PERF_QOS_IOC_SET_MAX _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MAX_CMD, struct perf_qos_ioctl_arg *)
> > +#define PERF_QOS_IOC_GET_MAX _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MAX_CMD, struct perf_qos_ioctl_arg *)
> > +#define PERF_QOS_IOC_GET_UNIT _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_UNIT_CMD, struct perf_qos_ioctl_arg *)
> > +#define PERF_QOS_IOC_GET_LIMITS _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_LIMITS_CMD, struct perf_qos_ioctl_arg *)
> > +
> > +#endif
> > diff --git a/kernel/power/Makefile b/kernel/power/Makefile
> > index 874ad834dc8d..e2e4d707ab6e 100644
> > --- a/kernel/power/Makefile
> > +++ b/kernel/power/Makefile
> > @@ -8,7 +8,7 @@ endif
> >
> > KASAN_SANITIZE_snapshot.o := n
> >
> > -obj-y += qos.o
> > +obj-y += qos.o perf_qos.o
> > obj-$(CONFIG_PM) += main.o
> > obj-$(CONFIG_VT_CONSOLE_SLEEP) += console.o
> > obj-$(CONFIG_FREEZER) += process.o
> > diff --git a/kernel/power/perf_qos.c b/kernel/power/perf_qos.c
> > new file mode 100644
> > index 000000000000..ca0619b07ae5
> > --- /dev/null
> > +++ b/kernel/power/perf_qos.c
> > @@ -0,0 +1,652 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Performance Quality of Service (Perf QoS) support base.
> > + *
> > + * Copyright (C) 2024 Linaro Ltd
> > + *
> > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> > + *
>
> I would expect some description of what the code in this file is for here.
>
> > + */
> > +#include <linux/cdev.h>
> > +#include <linux/perf_qos.h>
> > +#include <linux/list_sort.h>
> > +
> > +#define DEVNAME "perf_qos"
> > +#define NUM_PERF_QOS_MINORS 128
> > +
> > +static DEFINE_IDR(perf_qos_minors);
> > +static struct class *perf_qos_class;
> > +static dev_t perf_qos_devt;
> > +
> > +typedef enum {
> > + PERF_QOS_LIMIT_MAX,
> > + PERF_QOS_LIMIT_MIN,
>
> PERF_QOS_MAX/MIN?
>
> > +} perf_qos_limit_t;
>
> Why would "enum perf_qos_type" be insufficient?
>
> > +
> > +/**
> > + * struct perf_qos_constraint - structure holding a constraint information
> > + *
> > + * @soft_limit: an integer corresponding of the limit value set
> > + * @hard_limit: an integer corresponding to the limit value allowed by the driver
> > + * @kref: a refcount to the constraint responsible of its life cycle
> > + * @set_perf_limit_cb: a callback to notify the backend driver about the limit change
> > + * @node: the list node to attach this constraint with the list of constraints
> > + * @head: the list of constraints the @node
> > + *
> > + * This structure has a couple of instanciation per perf QoS file
> > + * opened by a process. The process can apply one or two constraints
> > + * to the device.
> > + *
> > + * Other processes will allocate their own constraints which will be
> > + * added in the list of constraints.
>
> In the absence of general documentation, this comment doesn't help too
> much I'm afraid.
>
> > + */
> > +struct perf_qos_constraint {
> > + int soft_limit;
> > + int hard_limit;
> > + set_perf_limit_cb_t set_perf_limit_cb;
> > + struct kref kref;
> > + struct list_head node;
> > + struct list_head *head;
> > +};
> > +
> > +/**
> > + * struct perf_qos - structure owning the constraint information for
> > + * the device
> > + *
> > + * @lock: lock to protect the actions on the list of constraints
> > + * @perf_qos_cdev: a struct cdev used for the device destruction
> > + * @ops: the ops given by the backend driver to notify the change of constraint
> > + * @descr: a constraint descriptor giving the units and the boundaries
>
> The meaning of this is kind of unclear.
>
> > + * @perf_min: the list of the minimal performance constraints
> > + * @perf_max: the list of the maximal performance constraints
>
> s/maximal/maximum/
>
> > + */
> > +struct perf_qos {
> > + spinlock_t lock;
>
> Why spinlock?
>
> > + struct cdev perf_qos_cdev;
>
> Why not just cdev?
>
> > + struct perf_qos_ops *ops;
> > + struct perf_qos_value_descr *descr;
> > + struct list_head perf_min;
> > + struct list_head perf_max;
>
> I would prefer constraiints_min and constraints_max.
>
> > +};
> > +
> > +/**
> > + * struct perf_qos_data - structure with the requested constraints
> > + *
> > + * @pqc_min: the requested performance constraint giving the minimal value
> > + * @pqc_max: the requested performance constraint giving the maximal value
> > + */
> > +struct perf_qos_data {
> > + struct perf_qos_constraint *pqc_min;
> > + struct perf_qos_constraint *pqc_max;
> > +};
> > +
> > +static struct perf_qos_constraint *perf_qos_constraint_find(struct list_head *list, int value)
> > +{
> > + struct perf_qos_constraint *pcq;
> > +
> > + list_for_each_entry(pcq, list, node) {
> > + if (pcq->soft_limit == value)
> > + return pcq;
>
> Why does this check soft_limit and not hard_limit?
>
> > + }
> > +
> > + return NULL;
> > +}
> > +
> > +static int perf_qos_constraint_cmp(void *data,
> > + const struct list_head *l1,
> > + const struct list_head *l2)
> > +{
> > + struct perf_qos_constraint *pqc1 = container_of(l1, struct perf_qos_constraint, node);
> > + struct perf_qos_constraint *pqc2 = container_of(l2, struct perf_qos_constraint, node);
> > +
> > + /*
> > + * The comparison will depend if we apply a max or min
> > + * performance constraint. If the soft limit is lesser than
>
> "less than"
>
> > + * the hard limit, that means it is a maximum limitation.
> > + */
> > + if (pqc1->soft_limit < pqc1->hard_limit)
> > + return pqc1->soft_limit - pqc2->soft_limit;
> > +
> > + return pqc2->soft_limit - pqc1->soft_limit;
>
> Again, why is this only comparing the soft limits?
>
> > +}
> > +
> > +static int perf_qos_del(struct perf_qos_constraint *pcq)
> > +{
> > + const struct perf_qos_constraint *first;
> > + int new_limit;
> > +
>
> I gather that this runs under a perf_qos lock.
>
> > + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> > +
> > + list_del(&pcq->node);
> > +
> > + /*
> > + * The active constraint is not the one we removed, so there
> > + * is nothing more to do
> > + */
> > + if (first != pcq)
> > + return 0;
> > +
> > + /*
> > + * As we remove the first entry, then get the new first entry
> > + * to apply the next constraint. If there is no more
> > + * constraint set, reset to the original limit. Otherwise, use
> > + * the new constraint value.
> > + */
> > + if (list_empty(pcq->head))
> > + new_limit = pcq->hard_limit;
>
> I don't quite get it, sorry. Shouldn't the new limit be a "no limit" here?
>
> > + else {
> > + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> > + new_limit = first->soft_limit;
> > + };
> > +
> > + /*
> > + * Notify the backend driver to update its performance level
> > + * if needed. If the performance level is currently inside the
> > + * new limits, nothing will happen. Otherwise it must be
> > + * adjust the current performance level to be inside the
> > + * authorized limits
> > + */
> > + pcq->set_perf_limit_cb(new_limit);
>
> So how's the provider of this callback supposed to know if this is the
> min or the max limit?
>
> > +
> > + return 1;
> > +}
> > +
> > +static int perf_qos_add(struct perf_qos_constraint *pcq)
> > +{
> > + const struct perf_qos_constraint *first;
> > +
>
> I gather that this runs under a perf_qos lock.
>
> > + list_add(&pcq->node, pcq->head);
> > +
> > + list_sort(NULL, pcq->head, perf_qos_constraint_cmp);
>
> And this may take some time in principle, so running it under a
> spinlock may not be a good idea.
This sorting is actually not necessary at all AFAICS.
If the list is always sorted, an element can be added to it at the
right spot: just iterate over elements until you find the place. This
takes at most 1 list iteration, reads only and just a few writes to
update the next/prev pointers at the insertion time.
Moreover, the list can always be sorted in the same order regardless
of the constraint type ("min" or "max"). If the order is ascending,
then for the "min" constraint type the effective value is in the last
element and for the "max" constraint type it is in the first element.
This observation can be used to simplify the code quite a bit I think
(the "compare" function is not really needed for one).
And there are a few additional general observations that can be made.
First, the interface need not care about the units. Since user space
needs to know exactly which driver it is going to interact with
through this interface, it also knows the perf units used by that
driver, so it doesn't need to be told what the unit is.
Second, the hard limits are not necessary. The backend can deal with
any values that are passed to it and user space doesn't need to know
the device limits (and even if it does, there can be an ioctl to get
them, but they need to appear anywhere else in the interface because
the backend will observe them anyway).
Next, the backend should always be told the current effective min and
max limits when notified of a limit change. Otherwise it will need to
figure out which limit has been updated and so on.
Finally, I would call this whole thing "perf clamp" rather than "perf
QoS", because it really is a clamp type of an interface.
There is one more thing that if user space updates the limits faster
than the backend is able to set them, the device may end up running
too slow or too fast all the time. Maybe this is not a problem in
practice, but it may be worth taking care of in the future.
Thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 1/2] power: Userspace performance QoS
2025-05-09 9:54 ` Rafael J. Wysocki
@ 2025-05-10 12:38 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-05-10 12:38 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-kernel, linux-pm, ulf.hansson, arnd, saravanak
On Fri, May 9, 2025 at 11:54 AM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Thu, May 8, 2025 at 11:05 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
> >
> > On Mon, May 5, 2025 at 6:19 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote:
> > >
> > > In the embedded ecosystem, the Linux kernel is modified to integrate
> > > fake thermal cooling devices for the sake of the ABI exported in the
> > > sysfs.
> > >
> > > While investigating those different devices, it appears most of them
> > > could fall under a performance QoS feature.
> > >
> > > As discussed at the Linux Plumber Conference 2024, we want to let the
> > > userspace to access the device performance knob via a char device
> > > which would be created by the backend drivers and controlled with an
> > > ioctl.
> > >
> > > A performance constraint is a minimal or a maximal limit applied to a
> > > device performance state. A process can only set one constraint per
> > > limit, in other words a minimal performance and/or a maximal
> > > performance constraint. A new value will change the current
> > > constraint, not create a new one.
> >
> > So how does this work for the constraints where
> > perf_qos_constraint_find() has returned a valid pointer in
> > perf_qos_set()? Is someone else's constraint updated without
> > notifying the original owner?
> >
> > > If another constraint must be
> > > stacked with the current one, then the char device file must be opened
> > > again and the resulting new file descriptor must be used to create a
> > > new constraint.
> > >
> > > Constraint life cycle:
> > >
> > > The userspace can be a place where buggy programs with root privileges
> > > can tamper with the device performance. In order to prevent some dumb
> > > logics to set a device performance state and then go away, thus
> > > messing with the global system performance consistency, there is a
> > > particular care of the constraint life cycles. These ones are directly
> > > tied with the opened file descriptor of the char device. When it is
> > > released, then the constraint is removed but only if its refcount
> > > reaches zero.
> >
> > So I'm totally unconvinced about the refcount thing.
> >
> > I personally don't think that sharing constraints is a good idea at
> > all. In principle, it doesn't matter that the current constraint
> > value is the same as somebody else's constraint value: they are
> > different constraints because they have been set by different
> > entities. It should be possible to update any of them independently
> > at any time and the involved complexity is not worth the memory usage
> > reduction achieved by sharing constraints.
> >
> > As an optimization, it is premature at best IMV.
> >
> > > This situation exists if only process sets the
> > > constraint and then closes the file descriptor (manually or at exit
> > > time). If the process forks multiple time and the children inherit the
> > > file descriptor, the constraint will be removed when all the children
> > > close the file descriptor.
> > >
> > > However, if another process opens the char device and sets a
> > > constraint which already exists then that results in incrementing the
> > > refcount of the constraint. The constraint is then removed when all
> > > the processes have closed their file descriptor pointing to the char
> > > device.
> > >
> > > At creation time:
> > >
> > > - if another process asked for the same limit of performance, then
> > > the refcount constraint is incremented
> > >
> > > - if there is an existing constraint with a higher priority, then the
> > > requested constraint is queued in the ordered list of constraints
> > >
> > > - if there is an existing constraint with a lower limit, then the
> > > requested constrained is applied and the current constraint is
> > > queued in the ordered list of constraints
> > >
> > > At removal time:
> > >
> > > - if the removed constraint is the current one, then the next
> > > constraint in the ordered list is applied
> > >
> > > - if the removed constraint is not the current one, then it is simply
> > > removed from the ordered list
> > >
> > > The changes allows the userspace to set a performance constraint for a
> > > specific device but the kernel may also want to apply a performance
> > > constraint. The in-kernel API is not yet implemented as it represents
> > > a significant amount of work depending on the direction of this patch.
> >
> > Apart from the above, I'm not sure if the locking is sufficient and
> > there are a few minor nits below.
> >
> > However, at the general level, an in-kernel user of this is missing
> > which is needed to illustrate how this is going to be integrated with
> > the existing code. That is, what happens when user space sets a
> > constraint for a specific device, where that constraint is applied and
> > how exactly.
> >
> > Also, in the cooling device interface, the user space agent chooses
> > the state to put the device into, which is not a constraint, but a
> > representation of the desired performance (or thermal pressure if you
> > will). This QoS interface instead operates min and max limits, so how
> > are the users supposed to know what to do with it? Is setting the max
> > limit equivalent to setting a specific cooling state? If so, then
> > what's the role of the min limit? And what about the soft limit
> > values? How are they supposed to be used?
> >
> > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > ---
> > > include/linux/perf_qos.h | 45 ++
> > > include/uapi/linux/perf_qos_ioctl.h | 47 ++
> > > kernel/power/Makefile | 2 +-
> > > kernel/power/perf_qos.c | 652 ++++++++++++++++++++++++++++
> > > 4 files changed, 745 insertions(+), 1 deletion(-)
> > > create mode 100644 include/linux/perf_qos.h
> > > create mode 100644 include/uapi/linux/perf_qos_ioctl.h
> > > create mode 100644 kernel/power/perf_qos.c
> > >
> > > diff --git a/include/linux/perf_qos.h b/include/linux/perf_qos.h
> > > new file mode 100644
> > > index 000000000000..57529c40be4d
> > > --- /dev/null
> > > +++ b/include/linux/perf_qos.h
> > > @@ -0,0 +1,45 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/*
> > > + * Performance QoS device abstraction
> > > + *
> > > + * Copyright (2024) Linaro Ltd
> > > + *
> > > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > + *
> > > + */
> > > +#ifndef __PERF_QOS_H
> > > +#define __PERF_QOS_H
> > > +
> > > +#include <uapi/linux/perf_qos_ioctl.h>
> > > +
> > > +struct perf_qos;
> > > +
> > > +/**
> > > + * struct perf_qos_value_descr - Performance constraint description
> > > + *
> > > + * @unit: the unit used for the constraint (normalized, throughput, ...)
> > > + * @limit_min: the minimal constraint limit to be set
> > > + * @limit_max: the maximal constraint limit to be set
> > > + */
> > > +struct perf_qos_value_descr {
> > > + perf_qos_unit_t unit;
> >
> > "enum perf_qos_unit" would be better IMV.
> >
> > Also, why is this enum needed at all?
> >
> > > + int limit_min;
> > > + int limit_max;
> >
> > Why not just min and max?
> >
> > > +};
> > > +
> > > +typedef int (*set_perf_limit_cb_t)(int);
> > > +
> > > +struct perf_qos_ops {
> > > + set_perf_limit_cb_t set_perf_limit_max;
> > > + set_perf_limit_cb_t set_perf_limit_min;
> > > +};
> > > +
> > > +extern struct perf_qos *perf_qos_device_create(const char *name,
> > > + struct perf_qos_ops *ops,
> > > + struct perf_qos_value_descr *descr);
> > > +
> > > +extern int perf_qos_is_allowed(struct perf_qos *pq, int performance);
> > > +
> > > +extern void perf_qos_device_destroy(struct perf_qos *pq);
> > > +
> > > +#endif
> > > diff --git a/include/uapi/linux/perf_qos_ioctl.h b/include/uapi/linux/perf_qos_ioctl.h
> > > new file mode 100644
> > > index 000000000000..a9fb8940c175
> > > --- /dev/null
> > > +++ b/include/uapi/linux/perf_qos_ioctl.h
> > > @@ -0,0 +1,47 @@
> > > +/* SPDX-License-Identifier: LGPL-2.0+ WITH Linux-syscall-note */
> > > +/*
> > > + * Performance QoS device abstraction
> > > + *
> > > + * Copyright (2024) Linaro Ltd
> > > + *
> > > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > + *
> > > + */
> > > +#ifndef __PERF_QOS_IOCTL_H
> > > +#define __PERF_QOS_IOCTL_H
> > > +
> > > +#include <linux/types.h>
> > > +
> > > +enum {
> > > + PERF_QOS_IOC_SET_MIN_CMD,
> > > + PERF_QOS_IOC_GET_MIN_CMD,
> > > + PERF_QOS_IOC_SET_MAX_CMD,
> > > + PERF_QOS_IOC_GET_MAX_CMD,
> > > + PERF_QOS_IOC_GET_UNIT_CMD,
> > > + PERF_QOS_IOC_GET_LIMITS_CMD,
> >
> > What's this one for?
> >
> > > + PERF_QOS_IOC_MAX_CMD,
> > > +};
> >
> > It would be nice to document this interface somehow, so it is not
> > necessary to reverse-engineer the code to find out how it is expected
> > to work.
> >
> > > +
> > > +typedef enum {
> > > + PERF_QOS_UNIT_NORMAL,
> > > + PERF_QOS_UNIT_KBPS,
> > > + PERF_QOS_UNIT_MAX
> > > +} perf_qos_unit_t;
> >
> > This is just an enum type. What's the typedef for?
> >
> > > +
> > > +struct perf_qos_ioctl_arg {
> > > + int value;
> > > + int limit_min;
> > > + int limit_max;
> >
> > Again, why not min and max?
> >
> > > + perf_qos_unit_t unit;
> > > +};
> > > +
> > > +#define PERF_QOS_IOCTL_TYPE 'P'
> > > +
> > > +#define PERF_QOS_IOC_SET_MIN _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MIN_CMD, struct perf_qos_ioctl_arg *)
> > > +#define PERF_QOS_IOC_GET_MIN _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MIN_CMD, struct perf_qos_ioctl_arg *)
> > > +#define PERF_QOS_IOC_SET_MAX _IOW(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_SET_MAX_CMD, struct perf_qos_ioctl_arg *)
> > > +#define PERF_QOS_IOC_GET_MAX _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_MAX_CMD, struct perf_qos_ioctl_arg *)
> > > +#define PERF_QOS_IOC_GET_UNIT _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_UNIT_CMD, struct perf_qos_ioctl_arg *)
> > > +#define PERF_QOS_IOC_GET_LIMITS _IOR(PERF_QOS_IOCTL_TYPE, PERF_QOS_IOC_GET_LIMITS_CMD, struct perf_qos_ioctl_arg *)
> > > +
> > > +#endif
> > > diff --git a/kernel/power/Makefile b/kernel/power/Makefile
> > > index 874ad834dc8d..e2e4d707ab6e 100644
> > > --- a/kernel/power/Makefile
> > > +++ b/kernel/power/Makefile
> > > @@ -8,7 +8,7 @@ endif
> > >
> > > KASAN_SANITIZE_snapshot.o := n
> > >
> > > -obj-y += qos.o
> > > +obj-y += qos.o perf_qos.o
> > > obj-$(CONFIG_PM) += main.o
> > > obj-$(CONFIG_VT_CONSOLE_SLEEP) += console.o
> > > obj-$(CONFIG_FREEZER) += process.o
> > > diff --git a/kernel/power/perf_qos.c b/kernel/power/perf_qos.c
> > > new file mode 100644
> > > index 000000000000..ca0619b07ae5
> > > --- /dev/null
> > > +++ b/kernel/power/perf_qos.c
> > > @@ -0,0 +1,652 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Performance Quality of Service (Perf QoS) support base.
> > > + *
> > > + * Copyright (C) 2024 Linaro Ltd
> > > + *
> > > + * Author: Daniel Lezcano <daniel.lezcano@linaro.org>
> > > + *
> >
> > I would expect some description of what the code in this file is for here.
> >
> > > + */
> > > +#include <linux/cdev.h>
> > > +#include <linux/perf_qos.h>
> > > +#include <linux/list_sort.h>
> > > +
> > > +#define DEVNAME "perf_qos"
> > > +#define NUM_PERF_QOS_MINORS 128
> > > +
> > > +static DEFINE_IDR(perf_qos_minors);
> > > +static struct class *perf_qos_class;
> > > +static dev_t perf_qos_devt;
> > > +
> > > +typedef enum {
> > > + PERF_QOS_LIMIT_MAX,
> > > + PERF_QOS_LIMIT_MIN,
> >
> > PERF_QOS_MAX/MIN?
> >
> > > +} perf_qos_limit_t;
> >
> > Why would "enum perf_qos_type" be insufficient?
> >
> > > +
> > > +/**
> > > + * struct perf_qos_constraint - structure holding a constraint information
> > > + *
> > > + * @soft_limit: an integer corresponding of the limit value set
> > > + * @hard_limit: an integer corresponding to the limit value allowed by the driver
> > > + * @kref: a refcount to the constraint responsible of its life cycle
> > > + * @set_perf_limit_cb: a callback to notify the backend driver about the limit change
> > > + * @node: the list node to attach this constraint with the list of constraints
> > > + * @head: the list of constraints the @node
> > > + *
> > > + * This structure has a couple of instanciation per perf QoS file
> > > + * opened by a process. The process can apply one or two constraints
> > > + * to the device.
> > > + *
> > > + * Other processes will allocate their own constraints which will be
> > > + * added in the list of constraints.
> >
> > In the absence of general documentation, this comment doesn't help too
> > much I'm afraid.
> >
> > > + */
> > > +struct perf_qos_constraint {
> > > + int soft_limit;
> > > + int hard_limit;
> > > + set_perf_limit_cb_t set_perf_limit_cb;
> > > + struct kref kref;
> > > + struct list_head node;
> > > + struct list_head *head;
> > > +};
> > > +
> > > +/**
> > > + * struct perf_qos - structure owning the constraint information for
> > > + * the device
> > > + *
> > > + * @lock: lock to protect the actions on the list of constraints
> > > + * @perf_qos_cdev: a struct cdev used for the device destruction
> > > + * @ops: the ops given by the backend driver to notify the change of constraint
> > > + * @descr: a constraint descriptor giving the units and the boundaries
> >
> > The meaning of this is kind of unclear.
> >
> > > + * @perf_min: the list of the minimal performance constraints
> > > + * @perf_max: the list of the maximal performance constraints
> >
> > s/maximal/maximum/
> >
> > > + */
> > > +struct perf_qos {
> > > + spinlock_t lock;
> >
> > Why spinlock?
> >
> > > + struct cdev perf_qos_cdev;
> >
> > Why not just cdev?
> >
> > > + struct perf_qos_ops *ops;
> > > + struct perf_qos_value_descr *descr;
> > > + struct list_head perf_min;
> > > + struct list_head perf_max;
> >
> > I would prefer constraiints_min and constraints_max.
> >
> > > +};
> > > +
> > > +/**
> > > + * struct perf_qos_data - structure with the requested constraints
> > > + *
> > > + * @pqc_min: the requested performance constraint giving the minimal value
> > > + * @pqc_max: the requested performance constraint giving the maximal value
> > > + */
> > > +struct perf_qos_data {
> > > + struct perf_qos_constraint *pqc_min;
> > > + struct perf_qos_constraint *pqc_max;
> > > +};
> > > +
> > > +static struct perf_qos_constraint *perf_qos_constraint_find(struct list_head *list, int value)
> > > +{
> > > + struct perf_qos_constraint *pcq;
> > > +
> > > + list_for_each_entry(pcq, list, node) {
> > > + if (pcq->soft_limit == value)
> > > + return pcq;
> >
> > Why does this check soft_limit and not hard_limit?
> >
> > > + }
> > > +
> > > + return NULL;
> > > +}
> > > +
> > > +static int perf_qos_constraint_cmp(void *data,
> > > + const struct list_head *l1,
> > > + const struct list_head *l2)
> > > +{
> > > + struct perf_qos_constraint *pqc1 = container_of(l1, struct perf_qos_constraint, node);
> > > + struct perf_qos_constraint *pqc2 = container_of(l2, struct perf_qos_constraint, node);
> > > +
> > > + /*
> > > + * The comparison will depend if we apply a max or min
> > > + * performance constraint. If the soft limit is lesser than
> >
> > "less than"
> >
> > > + * the hard limit, that means it is a maximum limitation.
> > > + */
> > > + if (pqc1->soft_limit < pqc1->hard_limit)
> > > + return pqc1->soft_limit - pqc2->soft_limit;
> > > +
> > > + return pqc2->soft_limit - pqc1->soft_limit;
> >
> > Again, why is this only comparing the soft limits?
> >
> > > +}
> > > +
> > > +static int perf_qos_del(struct perf_qos_constraint *pcq)
> > > +{
> > > + const struct perf_qos_constraint *first;
> > > + int new_limit;
> > > +
> >
> > I gather that this runs under a perf_qos lock.
> >
> > > + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> > > +
> > > + list_del(&pcq->node);
> > > +
> > > + /*
> > > + * The active constraint is not the one we removed, so there
> > > + * is nothing more to do
> > > + */
> > > + if (first != pcq)
> > > + return 0;
> > > +
> > > + /*
> > > + * As we remove the first entry, then get the new first entry
> > > + * to apply the next constraint. If there is no more
> > > + * constraint set, reset to the original limit. Otherwise, use
> > > + * the new constraint value.
> > > + */
> > > + if (list_empty(pcq->head))
> > > + new_limit = pcq->hard_limit;
> >
> > I don't quite get it, sorry. Shouldn't the new limit be a "no limit" here?
> >
> > > + else {
> > > + first = list_first_entry(pcq->head, struct perf_qos_constraint, node);
> > > + new_limit = first->soft_limit;
> > > + };
> > > +
> > > + /*
> > > + * Notify the backend driver to update its performance level
> > > + * if needed. If the performance level is currently inside the
> > > + * new limits, nothing will happen. Otherwise it must be
> > > + * adjust the current performance level to be inside the
> > > + * authorized limits
> > > + */
> > > + pcq->set_perf_limit_cb(new_limit);
> >
> > So how's the provider of this callback supposed to know if this is the
> > min or the max limit?
> >
> > > +
> > > + return 1;
> > > +}
> > > +
> > > +static int perf_qos_add(struct perf_qos_constraint *pcq)
> > > +{
> > > + const struct perf_qos_constraint *first;
> > > +
> >
> > I gather that this runs under a perf_qos lock.
> >
> > > + list_add(&pcq->node, pcq->head);
> > > +
> > > + list_sort(NULL, pcq->head, perf_qos_constraint_cmp);
> >
> > And this may take some time in principle, so running it under a
> > spinlock may not be a good idea.
>
> This sorting is actually not necessary at all AFAICS.
>
> If the list is always sorted, an element can be added to it at the
> right spot: just iterate over elements until you find the place. This
> takes at most 1 list iteration, reads only and just a few writes to
> update the next/prev pointers at the insertion time.
>
> Moreover, the list can always be sorted in the same order regardless
> of the constraint type ("min" or "max"). If the order is ascending,
> then for the "min" constraint type the effective value is in the last
> element and for the "max" constraint type it is in the first element.
> This observation can be used to simplify the code quite a bit I think
> (the "compare" function is not really needed for one).
>
> And there are a few additional general observations that can be made.
>
> First, the interface need not care about the units. Since user space
> needs to know exactly which driver it is going to interact with
> through this interface, it also knows the perf units used by that
> driver, so it doesn't need to be told what the unit is.
>
> Second, the hard limits are not necessary. The backend can deal with
> any values that are passed to it and user space doesn't need to know
> the device limits (and even if it does, there can be an ioctl to get
> them, but they need to appear anywhere else in the interface because
> the backend will observe them anyway).
>
> Next, the backend should always be told the current effective min and
> max limits when notified of a limit change. Otherwise it will need to
> figure out which limit has been updated and so on.
>
> Finally, I would call this whole thing "perf clamp" rather than "perf
> QoS", because it really is a clamp type of an interface.
>
> There is one more thing that if user space updates the limits faster
> than the backend is able to set them, the device may end up running
> too slow or too fast all the time. Maybe this is not a problem in
> practice, but it may be worth taking care of in the future.
FWIW, my design of this would be as follows (I'll stick to the
clamping terminology if you will).
If a kernel entity (eg. a driver) wants to respond to perf clamping,
it calls perf_clamp_create() to create a special device file for the
interface. That will also cause a perf_clamp object associated with
that file to be created.
The perf_clamp object contains two list_head fields, clamp_min and
clamp_max, holding the min and max clamp lists, respectively, a
backend callback pointer, and a lock. Both clamp_min and clamp_max
lists are always sorted in ascending order.
When a task in user space wants to set a perf clamp, it will open the
device special file. That will cause a perf_clamp_instance object to
be created and associated with the open file descriptor.
A perf_clamp_instance object contains a pointer to the "parent"
perf_clamp object (associated with the opened device special file) and
two perf_clamp_limit fields, min and max.
A perf_clamp_limit consists of an unsigned integer value (val), a
list_head (node), and a type indicator (a bool value to distinguish
between the min and max limits).
Initially, the min perf_clamp_limit in a perf_clamp_instance becomes
the last element of the clamp_min list in the parent perf_clamp, and
the max perf_clamp_limit in a perf_clamp_instance becomes the first
element of the clamp_max list in the parent perf_clamp. val is set to
UINT_MAX and 0 (which means "no limit" in both cases) for them,
respectively.
To set a limit, the holder of a file descriptor associated with a
perf_clamp_instance uses a _SET ioctl() that will remove the target
perf_clamp_limit from the list, set the new value for it, and add it
back to the same list in a different place, in accordance with the
ascending order. If this causes the new value to become the new
effective clamp limit (that is, the first instance with a value
different from "no limit" in the "max" list or the last instance with
a value different from "no limit" in the "min" list), the backend
callback is invoked and both the min and max effective limits are
passed to it. All of this takes place under the parent perf_clamp
lock.
To get the min or max value from a perf_clamp_instance, its holder
uses a _GET ioctl() (this doesn't require the parent lock).
Overall, there are two _SET and two _GET ioctl()s (for the min and max
values in both cases).
In addition, there can be two _GET_EFFECTIVE ioctl()s returning the
effective clamp values (for the effective min and max).
Closing a previously opened file descriptor removes the
perf_clamp_instance associated with it from both the min and max lists
(under the parent lock) and if that causes the effective limits to
change, the backend callback is invoked. Next, the
perf_clamp_instance is deleted.
It would be good to also provide perf_clamp_destroy() for removing the
entire perf_clamp along with the special device file associated with
it, but that needs care because user space may be still using it.
I don't think that anything else is needed at this point and I hope this helps.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/2] selftests: Add perf_qos selftests
2025-05-05 16:19 ` [RFC PATCH 2/2] selftests: Add perf_qos selftests Daniel Lezcano
@ 2025-05-23 17:43 ` Eric Smith
2025-05-23 17:53 ` Daniel Lezcano
0 siblings, 1 reply; 7+ messages in thread
From: Eric Smith @ 2025-05-23 17:43 UTC (permalink / raw)
To: Daniel Lezcano, rafael
Cc: linux-kernel, linux-pm, ulf.hansson, arnd, saravanak,
deepti.jaggi, prasad.sodagudi
Hi Daniel,
On 5/5/2025 9:19 AM, Daniel Lezcano wrote:
> This patch provides somes tests which depend on a kernel module
> creating a dummy performance QoS device. More tests will be added
> later.
Thanks, I did not see a test where the perf_qos_is_allowed API is used.
In future patches, can you add a test using the perf_qos_is_allowed API?
Best regards,
Eric
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH 2/2] selftests: Add perf_qos selftests
2025-05-23 17:43 ` Eric Smith
@ 2025-05-23 17:53 ` Daniel Lezcano
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Lezcano @ 2025-05-23 17:53 UTC (permalink / raw)
To: Eric Smith, rafael
Cc: linux-kernel, linux-pm, ulf.hansson, arnd, saravanak,
deepti.jaggi, prasad.sodagudi
On 23/05/2025 19:43, Eric Smith wrote:
> Hi Daniel,
>
> On 5/5/2025 9:19 AM, Daniel Lezcano wrote:
>
>> This patch provides somes tests which depend on a kernel module
>> creating a dummy performance QoS device. More tests will be added
>> later.
>
> Thanks, I did not see a test where the perf_qos_is_allowed API is used.
> In future patches, can you add a test using the perf_qos_is_allowed API?
Sure, thanks for pointing this out
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-05-23 17:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-05-05 16:19 [RFC PATCH 1/2] power: Userspace performance QoS Daniel Lezcano
2025-05-05 16:19 ` [RFC PATCH 2/2] selftests: Add perf_qos selftests Daniel Lezcano
2025-05-23 17:43 ` Eric Smith
2025-05-23 17:53 ` Daniel Lezcano
2025-05-08 21:05 ` [RFC PATCH 1/2] power: Userspace performance QoS Rafael J. Wysocki
2025-05-09 9:54 ` Rafael J. Wysocki
2025-05-10 12:38 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®