linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration
@ 2025-07-21 12:40 Zhongqiu Han
  2025-07-21 12:41 ` [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS Zhongqiu Han
                   ` (6 more replies)
  0 siblings, 7 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-21 12:40 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, quic_zhonhan

Hi all,

This patch series introduces support for CPU affinity-based latency
constraints in the PM QoS framework. The motivation is to allow
finer-grained power management by enabling latency QoS requests to target
specific CPUs, rather than applying system-wide constraints.

The current PM QoS framework supports global and per-device CPU latency
constraints. However, in many real-world scenarios, such as IRQ affinity
or CPU-bound kernel threads, only a subset of CPUs are
performance-critical. Applying global constraints in such cases
unnecessarily prevents other CPUs from entering deeper C-states, leading
to increased power consumption.

This series addresses that limitation by introducing a new interface that
allows latency constraints to be applied to a CPU mask. This is
particularly useful on heterogeneous platforms (e.g., big.LITTLE) and
embedded systems where power efficiency is critical for example:

                        driver A       rt kthread B      module C
  CPU IDs (mask):         0-3              2-5              6-7
  target latency(us):     20               30               100
                          |                |                |
                          v                v                v
                          +---------------------------------+
                          |        PM  QoS  Framework       |
                          +---------------------------------+
                          |                |                |
                          v                v                v
  CPU IDs (mask):        0-3            2-3,4-5            6-7
  runtime latency(us):   20             20, 30             100

The current implementation includes only cpu_affinity_latency_qos_add()
and cpu_affinity_latency_qos_remove() interfaces. An update interface is
planned for future submission, along with PM QoS optimizations in the UFS
subsystem.

Patch1 introduces the core support for CPU affinity latency QoS in the PM
QoS framework.

Patch2 removes redundant KERN_ERR prefixes in WARN() calls in the global
CPU PM QoS interface. This change addresses issues in existing code and is
not related to the new interface introduced in this patch series.

Patch3 adds documentation for the new interface.

Patch4 fixes a minor documentation issue related to the return type of
cpu_latency_qos_request_active(). This change addresses issues in existing
doc and is not related to the new interface introduced in this patch
series.

Patch5 updates the resctrl pseudo-locking logic to use the new CPU
affinity latency QoS helpers, improving clarity and consistency. The only
functional and beneficial change is that the new interface actively wakes
up CPUs whose latency QoS values have changed, ensuring the latency limit
takes effect immediately.

Changes since v1:
- Rebased on top of current next.
- Resolve the compilation warning due to a missing static function
  declaration.
- Remove the conditional compilation based on CONFIG_CPU_IDLE and make it
  depend solely on CONFIG_PM.
- Add support for cpu_affinity_latency_qos_active.
- Remove cpu_affinity_latency_qos_update; will reintroduce it when needed
  in the future.
- Optimize the code, for example by using cpu_affinity_latency_qos_active
  inside the add/remove functions to enhance robustness.
- Refine the commit message and fix a few minor issues unrelated to this
  series.
- Refactor the CPU latency PM QoS logic of resctrl pseudo_lock using the
  interfaces provided by this series.
- Link to v1: https://lore.kernel.org/all/20250424095228.1112558-1-quic_zhonhan@quicinc.com/

Zhongqiu Han (5):
  PM: QoS: Add support for CPU affinity latency PM QoS
  PM: QOS: Remove unnecessary KERN_ERR on WARN() calls
  Documentation: PM: QoS: Add CPU affinity latency PM QoS Interface
    documentation
  Documentation: PM: QoS: Fix return type and return value description
  resctrl: Replace PM QoS logic with cpu_affinity_latency_qos_* helpers

 Documentation/power/pm_qos_interface.rst |  63 ++++++++-
 fs/resctrl/pseudo_lock.c                 |  51 +------
 include/linux/pm_qos.h                   |  40 ++++++
 include/linux/resctrl.h                  |   3 +-
 kernel/power/qos.c                       | 166 ++++++++++++++++++++++-
 5 files changed, 268 insertions(+), 55 deletions(-)


base-commit: 024e09e444bd2b06aee9d1f3fe7b313c7a2df1bb
-- 
2.43.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
@ 2025-07-21 12:41 ` Zhongqiu Han
  2025-08-21 10:37   ` Rafael J. Wysocki
  2025-07-21 12:41 ` [PATCH v2 2/5] PM: QOS: Remove unnecessary KERN_ERR on WARN() calls Zhongqiu Han
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-21 12:41 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, quic_zhonhan

Currently, the PM QoS framework supports global CPU latency QoS and
per-device CPU latency QoS requests. An example of using global CPU
latency QoS is a commit 2777e73fc154 ("scsi: ufs: core: Add CPU latency
QoS support for UFS driver") that improved random io performance by 15%
for ufs on specific device platform.

However, this prevents all CPUs in the system from entering C states.
Typically, some threads or drivers know which specific CPUs they are
interested in. For example, drivers with IRQ affinity only want interrupts
to wake up and be handled on specific CPUs. Similarly, kernel thread bound
to specific CPUs through affinity only care about the latency of those
particular CPUs.

This patch introduces support for partial CPUs PM QoS using a CPU affinity
mask, allowing flexible and more precise latency QoS settings for specific
CPUs. This can help save power, especially on heterogeneous platforms with
big and little cores, as well as some power-conscious embedded systems for
example:

                        driver A       rt kthread B      module C
  CPU IDs (mask):         0-3              2-5              6-7
  target latency(us):     20               30               100
                          |                |                |
                          v                v                v
                          +---------------------------------+
                          |        PM  QoS  Framework       |
                          +---------------------------------+
                          |                |                |
                          v                v                v
  CPU IDs (mask):        0-3            2-3,4-5            6-7
  runtime latency(us):   20             20, 30             100

Implement this support based on per-device CPU latency PM QoS.

Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
---
 include/linux/pm_qos.h |  40 +++++++++++
 kernel/power/qos.c     | 160 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 200 insertions(+)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 4a69d4af3ff8..2dbad825f8bd 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -131,6 +131,15 @@ enum pm_qos_req_action {
 	PM_QOS_REMOVE_REQ	/* Remove an existing request */
 };
 
+/* cpu affinity pm latency qos request handle */
+struct cpu_affinity_qos_req {
+	struct list_head list;
+	union {
+		struct dev_pm_qos_request req;
+		void *req_ptr;
+	};
+};
+
 static inline int dev_pm_qos_request_active(struct dev_pm_qos_request *req)
 {
 	return req->dev != NULL;
@@ -208,6 +217,13 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
 		PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
 		pm_qos_read_value(&dev->power.qos->resume_latency);
 }
+
+int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
+				  const cpumask_t *affinity_mask, s32 latency_value);
+int cpu_affinity_latency_qos_remove(struct cpu_affinity_qos_req *pm_req);
+int cpu_affinity_latency_qos_release(struct cpu_affinity_qos_req *pm_req);
+bool cpu_affinity_latency_qos_active(struct cpu_affinity_qos_req *pm_req);
+void wakeup_qos_affinity_idle_cpu(int cpu);
 #else
 static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
 							  s32 mask)
@@ -289,6 +305,30 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
 {
 	return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
 }
+
+static inline int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
+						const cpumask_t *affinity_mask,
+						s32 latency_value)
+{
+	return 0;
+}
+
+static inline int cpu_affinity_latency_qos_remove(
+		   struct cpu_affinity_qos_req *pm_req)
+{
+	return 0;
+}
+static inline int cpu_affinity_latency_qos_release(
+		   struct cpu_affinity_qos_req *pm_req)
+{
+	return 0;
+}
+static inline bool cpu_affinity_latency_qos_active(
+		    struct cpu_affinity_qos_req *pm_req)
+{
+	return false;
+}
+static inline void wakeup_qos_affinity_idle_cpu(int cpu) {}
 #endif
 
 static inline int freq_qos_request_active(struct freq_qos_request *req)
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 4244b069442e..5e507ed8d077 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -335,6 +335,166 @@ void cpu_latency_qos_remove_request(struct pm_qos_request *req)
 }
 EXPORT_SYMBOL_GPL(cpu_latency_qos_remove_request);
 
+#ifdef CONFIG_PM
+
+/**
+ * wakeup_qos_affinity_idle_cpu - break one specific cpu out of idle.
+ * @cpu: the CPU to be woken up from idle.
+ */
+void wakeup_qos_affinity_idle_cpu(int cpu)
+{
+	preempt_disable();
+	if (cpu != smp_processor_id() && cpu_online(cpu))
+		wake_up_if_idle(cpu);
+	preempt_enable();
+}
+
+/**
+ * cpu_affinity_latency_qos_add - Add new CPU affinity latency QoS request.
+ * @pm_req: Pointer to a preallocated handle.
+ * @affinity_mask: Mask to determine which CPUs need latency QoS.
+ * @latency_value: New requested constraint value.
+ *
+ * Use @latency_value to initialize the request handle pointed to by @pm_req,
+ * insert it as a new entry to the CPU latency QoS list and recompute the
+ * effective QoS constraint for that list, @affinity_mask determine which CPUs
+ * need the latency QoS.
+ *
+ * Callers need to save the handle for later use in updates and removal of the
+ * QoS request represented by it.
+ *
+ * Returns 0 or a positive value on success, or a negative error code on failure.
+ */
+int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
+				  const cpumask_t *affinity_mask,
+				  s32 latency_value)
+{
+	int cpu;
+	cpumask_t actual_mask;
+	struct cpu_affinity_qos_req *cpu_pm_req;
+	int ret = 0;
+
+	if (!pm_req) {
+		pr_err("%s: invalid PM Qos request\n", __func__);
+		return -EINVAL;
+	}
+
+	if (cpu_affinity_latency_qos_active(pm_req)) {
+		WARN(1, "%s called for already added request\n", __func__);
+		return -EBUSY;
+	}
+
+	INIT_LIST_HEAD(&pm_req->list);
+
+	if (!affinity_mask || cpumask_empty(affinity_mask) ||
+	    latency_value < 0) {
+		pr_err("%s: invalid PM Qos request value\n", __func__);
+		return -EINVAL;
+	}
+
+	for_each_cpu(cpu, affinity_mask) {
+		cpu_pm_req = kzalloc(sizeof(struct cpu_affinity_qos_req),
+				     GFP_KERNEL);
+		if (!cpu_pm_req) {
+			ret = -ENOMEM;
+			goto out_err;
+		}
+		ret = dev_pm_qos_add_request(get_cpu_device(cpu),
+					     &cpu_pm_req->req,
+					     DEV_PM_QOS_RESUME_LATENCY,
+					     latency_value);
+		if (ret < 0) {
+			pr_err("failed to add latency req for cpu%d", cpu);
+			kfree(cpu_pm_req);
+			goto out_err;
+		} else if (ret > 0) {
+			wakeup_qos_affinity_idle_cpu(cpu);
+		}
+
+		cpumask_set_cpu(cpu, &actual_mask);
+		list_add(&cpu_pm_req->list, &pm_req->list);
+	}
+
+	pr_info("PM Qos latency: %d added on cpus %*pb\n", latency_value,
+		cpumask_pr_args(&actual_mask));
+	pm_req->req_ptr = pm_req;
+	return ret;
+
+out_err:
+	cpu_affinity_latency_qos_release(pm_req);
+	pr_err("failed to add PM QoS latency req, removed all added requests\n");
+	return ret;
+}
+EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_add);
+
+
+/**
+ * cpu_affinity_latency_qos_remove - Remove an existing CPU affinity latency QoS.
+ * @pm_req: Handle to the QoS request to be removed.
+ *
+ * Remove the CPU latency QoS request represented by @pm_req from the CPU latency
+ * QoS list. This handle must have been previously initialized and added via
+ * cpu_affinity_latency_qos_add().
+ */
+int cpu_affinity_latency_qos_remove(struct cpu_affinity_qos_req *pm_req)
+{
+	if (!pm_req) {
+		pr_err("%s: invalid PM Qos request value\n", __func__);
+		return -EINVAL;
+	}
+
+	if (!cpu_affinity_latency_qos_active(pm_req)) {
+		WARN(1, "%s called for unknown object\n", __func__);
+		return -EINVAL;
+	}
+
+	return cpu_affinity_latency_qos_release(pm_req);
+}
+EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_remove);
+
+/**
+ * cpu_affinity_latency_qos_release - Release pm_reqs latency QoS resource.
+ * @pm_req: QoS request to be released.
+ *
+ * Release pm_reqs managed CPU affinity latency QoS resource.
+ *
+ * Returns a negative value indicates failure.
+ */
+int cpu_affinity_latency_qos_release(struct cpu_affinity_qos_req *pm_req)
+{
+	int ret = 0;
+	struct cpu_affinity_qos_req *cpu_pm_req, *next;
+
+	list_for_each_entry_safe(cpu_pm_req, next, &pm_req->list, list) {
+		ret = dev_pm_qos_remove_request(&cpu_pm_req->req);
+		if (ret < 0)
+			pr_err("failed to remove qos request for %s\n",
+			       dev_name(cpu_pm_req->req.dev));
+		list_del(&cpu_pm_req->list);
+		kfree(cpu_pm_req);
+		cpu_pm_req = NULL;
+	}
+
+	memset(pm_req, 0, sizeof(*pm_req));
+	return ret;
+}
+
+/**
+ * cpu_affinity_latency_qos_active - Check if a CPU affinity latency QoS
+ * request is active.
+ * @pm_req: Handle to the QoS request.
+ *
+ * Return: 'true' if @pm_req has been added to the CPU latency QoS list,
+ * 'false' otherwise.
+ */
+bool cpu_affinity_latency_qos_active(struct cpu_affinity_qos_req *pm_req)
+{
+	return pm_req->req_ptr == pm_req;
+}
+EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_active);
+
+#endif /* CONFIG_PM */
+
 /* User space interface to the CPU latency QoS via misc device. */
 
 static int cpu_latency_qos_open(struct inode *inode, struct file *filp)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 2/5] PM: QOS: Remove unnecessary KERN_ERR on WARN() calls
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
  2025-07-21 12:41 ` [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS Zhongqiu Han
@ 2025-07-21 12:41 ` Zhongqiu Han
  2025-07-21 12:41 ` [PATCH v2 3/5] Documentation: PM: QoS: Add CPU affinity latency PM QoS Interface documentation Zhongqiu Han
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-21 12:41 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, quic_zhonhan

Remove unnecessary KERN_ERR in WARN() calls and the latest version of
scripts/checkpatch.pl also warns about such a pattern.

Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
---
 kernel/power/qos.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 5e507ed8d077..5c157004b78f 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -272,7 +272,7 @@ void cpu_latency_qos_add_request(struct pm_qos_request *req, s32 value)
 		return;
 
 	if (cpu_latency_qos_request_active(req)) {
-		WARN(1, KERN_ERR "%s called for already added request\n", __func__);
+		WARN(1, "%s called for already added request\n", __func__);
 		return;
 	}
 
@@ -298,7 +298,7 @@ void cpu_latency_qos_update_request(struct pm_qos_request *req, s32 new_value)
 		return;
 
 	if (!cpu_latency_qos_request_active(req)) {
-		WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+		WARN(1, "%s called for unknown object\n", __func__);
 		return;
 	}
 
@@ -324,7 +324,7 @@ void cpu_latency_qos_remove_request(struct pm_qos_request *req)
 		return;
 
 	if (!cpu_latency_qos_request_active(req)) {
-		WARN(1, KERN_ERR "%s called for unknown object\n", __func__);
+		WARN(1, "%s called for unknown object\n", __func__);
 		return;
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 3/5] Documentation: PM: QoS: Add CPU affinity latency PM QoS Interface documentation
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
  2025-07-21 12:41 ` [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS Zhongqiu Han
  2025-07-21 12:41 ` [PATCH v2 2/5] PM: QOS: Remove unnecessary KERN_ERR on WARN() calls Zhongqiu Han
@ 2025-07-21 12:41 ` Zhongqiu Han
  2025-07-21 12:41 ` [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description Zhongqiu Han
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-21 12:41 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, quic_zhonhan

Adds documentation for the CPU affinity latency PM QoS interface, enabling
latency constraints to be applied to specific CPUs according to a defined
CPU mask.

Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
---
 Documentation/power/pm_qos_interface.rst | 57 ++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/Documentation/power/pm_qos_interface.rst b/Documentation/power/pm_qos_interface.rst
index 5019c79c7710..1ede4cafc2e3 100644
--- a/Documentation/power/pm_qos_interface.rst
+++ b/Documentation/power/pm_qos_interface.rst
@@ -17,6 +17,13 @@ The latency unit used in the PM QoS framework is the microsecond (usec).
 1. PM QoS framework
 ===================
 
+For CPU latency QoS, there are two types of interfaces: one is the global CPU
+latency QoS interface, and the other is the CPU affinity latency QoS, where the
+CPU is determined by the CPU affinity mask, which can apply to part or all of
+the CPUs.
+
+1) Global CPU latency QoS interface:
+
 A global list of CPU latency QoS requests is maintained along with an aggregated
 (effective) target value.  The aggregated target value is updated with changes
 to the request list or elements of the list.  For CPU latency QoS, the
@@ -76,6 +83,56 @@ cpu_latency_qos_update_request() call.
 To remove the user mode request for a target value simply close the device
 node.
 
+2) CPU affinity latency QoS interface:
+
+The Global CPU latency QoS interface can easily limit the latency for all CPUs.
+If we want to limit the CPU latency for partial CPUs specified by a CPU
+affinity mask, we can use the CPU affinity latency QoS interface. Currently,
+this is only supported for kernel users. This will only prevent the CPUs
+specified by the mask from entering C states. Typically, some threads or
+drivers know which specific CPUs they are interested in. For example, drivers
+with IRQ affinity only want interrupts to wake up and be handled on specific
+CPUs. Similarly, kernel thread bound to specific CPUs through affinity only
+care about the latency of those particular CPUs.
+
+It allows flexible and precise latency QoS settings for specific CPUs. This can
+help save power, especially on heterogeneous platforms with big and little cores,
+as well as power-conscious embedded systems. For example:
+
+                         driver A       rt kthread B      module C
+    CPU IDs (mask):        0-3              2-5              6-7
+    target latency(us):    20               30               100
+                            |                |                |
+                            v                v                v
+                            +---------------------------------+
+                            |        PM  QoS  Framework       |
+                            +---------------------------------+
+                            |                |                |
+                            v                v                v
+    CPU IDs (mask):        0-3            2-3,4-5            6-7
+    runtime latency(us):   20              20,30             100
+
+The usage of kernel space is:
+
+int cpu_affinity_latency_qos_add(handle, affinity_mask, latency_value);
+  Will insert an element into the CPUs specified by the affinity_mask latency
+  QoS list with the target value. Upon change to this list the new target is
+  recomputed. Clients of PM QoS need to save the returned handle for future use
+  in other PM QoS API functions.
+
+int cpu_affinity_latency_qos_remove(handle);
+  Will remove the element. After removal it will update the aggregate target
+  and call the notification tree if the target was changed as a result of
+  removing the request.
+
+bool cpu_affinity_latency_qos_active(handle);
+  Returns true if the request is still active, i.e. it has not been removed from
+  the CPU latency QoS list.
+
+Note:
+a) The CPU affinity latency QoS interface uses a mutex, which might sleep.
+b) Use dev_pm_qos_raw_resume_latency(device) to read the CPU latency set by the
+   above interface, by passing the CPU device.
 
 2. PM QoS per-device latency and flags framework
 ================================================
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
                   ` (2 preceding siblings ...)
  2025-07-21 12:41 ` [PATCH v2 3/5] Documentation: PM: QoS: Add CPU affinity latency PM QoS Interface documentation Zhongqiu Han
@ 2025-07-21 12:41 ` Zhongqiu Han
  2025-08-01 11:27   ` Christian Loehle
  2025-07-21 12:41 ` [PATCH v2 5/5] resctrl: Replace PM QoS logic with cpu_affinity_latency_qos_* helpers Zhongqiu Han
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-21 12:41 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, quic_zhonhan

The documentation for cpu_latency_qos_request_active() incorrectly stated
the return type as 'int' instead of 'bool', and the return value
description was incomplete. This patch corrects the return type and
clarifies the return value semantics.

Fixes: b8e6e27c626e ("Documentation: PM: QoS: Update to reflect previous code changes")
Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
---
 Documentation/power/pm_qos_interface.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/Documentation/power/pm_qos_interface.rst b/Documentation/power/pm_qos_interface.rst
index 1ede4cafc2e3..c6b8b9cda166 100644
--- a/Documentation/power/pm_qos_interface.rst
+++ b/Documentation/power/pm_qos_interface.rst
@@ -55,9 +55,9 @@ void cpu_latency_qos_remove_request(handle):
 int cpu_latency_qos_limit():
   Returns the aggregated value for the CPU latency QoS.
 
-int cpu_latency_qos_request_active(handle):
-  Returns if the request is still active, i.e. it has not been removed from the
-  CPU latency QoS list.
+bool cpu_latency_qos_request_active(handle):
+  Returns true if the request is still active, i.e. it has not been removed from
+  the CPU latency QoS list.
 
 
 From user space:
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/5] resctrl: Replace PM QoS logic with cpu_affinity_latency_qos_* helpers
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
                   ` (3 preceding siblings ...)
  2025-07-21 12:41 ` [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description Zhongqiu Han
@ 2025-07-21 12:41 ` Zhongqiu Han
  2025-07-28 10:09 ` [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Christian Loehle
  2025-08-15 12:48 ` Zhongqiu Han
  6 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-21 12:41 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, quic_zhonhan

Use cpu_affinity_latency_qos_* helpers to replace the dev_pm_qos_request
handling for CPU masks in pseudo-locking.

These helpers encapsulate latency QoS operations for a group of CPUs,
providing a consistent mechanism for request addition, removal, and error
handling. This improves code clarity and maintainability. As resctrlfs is
used on different platforms, a unified interface also makes it easier to
adapt to platform-specific CPU masks and latency QoS values.

The only functional change, and a beneficial one, is that when the latency
QoS value is updated for a CPU in the mask, the interface will actively
wake up that CPU to ensure the QoS setting takes effect immediately. This
helps enforce latency constraints in a timely manner on
performance-critical paths.

Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
---
 fs/resctrl/pseudo_lock.c | 51 +++-------------------------------------
 include/linux/resctrl.h  |  3 ++-
 2 files changed, 5 insertions(+), 49 deletions(-)

diff --git a/fs/resctrl/pseudo_lock.c b/fs/resctrl/pseudo_lock.c
index 87bbc2605de1..8be3ae804af9 100644
--- a/fs/resctrl/pseudo_lock.c
+++ b/fs/resctrl/pseudo_lock.c
@@ -108,25 +108,9 @@ static struct rdtgroup *region_find_by_minor(unsigned int minor)
 	return rdtgrp_match;
 }
 
-/**
- * struct pseudo_lock_pm_req - A power management QoS request list entry
- * @list:	Entry within the @pm_reqs list for a pseudo-locked region
- * @req:	PM QoS request
- */
-struct pseudo_lock_pm_req {
-	struct list_head list;
-	struct dev_pm_qos_request req;
-};
-
 static void pseudo_lock_cstates_relax(struct pseudo_lock_region *plr)
 {
-	struct pseudo_lock_pm_req *pm_req, *next;
-
-	list_for_each_entry_safe(pm_req, next, &plr->pm_reqs, list) {
-		dev_pm_qos_remove_request(&pm_req->req);
-		list_del(&pm_req->list);
-		kfree(pm_req);
-	}
+	cpu_affinity_latency_qos_remove(&plr->pm_reqs);
 }
 
 /**
@@ -149,36 +133,8 @@ static void pseudo_lock_cstates_relax(struct pseudo_lock_region *plr)
  */
 static int pseudo_lock_cstates_constrain(struct pseudo_lock_region *plr)
 {
-	struct pseudo_lock_pm_req *pm_req;
-	int cpu;
-	int ret;
-
-	for_each_cpu(cpu, &plr->d->hdr.cpu_mask) {
-		pm_req = kzalloc(sizeof(*pm_req), GFP_KERNEL);
-		if (!pm_req) {
-			rdt_last_cmd_puts("Failure to allocate memory for PM QoS\n");
-			ret = -ENOMEM;
-			goto out_err;
-		}
-		ret = dev_pm_qos_add_request(get_cpu_device(cpu),
-					     &pm_req->req,
-					     DEV_PM_QOS_RESUME_LATENCY,
-					     30);
-		if (ret < 0) {
-			rdt_last_cmd_printf("Failed to add latency req CPU%d\n",
-					    cpu);
-			kfree(pm_req);
-			ret = -1;
-			goto out_err;
-		}
-		list_add(&pm_req->list, &plr->pm_reqs);
-	}
-
-	return 0;
-
-out_err:
-	pseudo_lock_cstates_relax(plr);
-	return ret;
+	return cpu_affinity_latency_qos_add(&plr->pm_reqs, &plr->d->hdr.cpu_mask,
+					    30);
 }
 
 /**
@@ -275,7 +231,6 @@ static int pseudo_lock_init(struct rdtgroup *rdtgrp)
 		return -ENOMEM;
 
 	init_waitqueue_head(&plr->lock_thread_wq);
-	INIT_LIST_HEAD(&plr->pm_reqs);
 	rdtgrp->plr = plr;
 	return 0;
 }
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 6fb4894b8cfd..521fe70b0425 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -6,6 +6,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/pid.h>
+#include <linux/pm_qos.h>
 #include <linux/resctrl_types.h>
 
 #ifdef CONFIG_ARCH_HAS_CPU_RESCTRL
@@ -108,7 +109,7 @@ struct pseudo_lock_region {
 	void			*kmem;
 	unsigned int		minor;
 	struct dentry		*debugfs_dir;
-	struct list_head	pm_reqs;
+	struct cpu_affinity_qos_req	pm_reqs;
 };
 
 /**
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
                   ` (4 preceding siblings ...)
  2025-07-21 12:41 ` [PATCH v2 5/5] resctrl: Replace PM QoS logic with cpu_affinity_latency_qos_* helpers Zhongqiu Han
@ 2025-07-28 10:09 ` Christian Loehle
  2025-07-28 10:40   ` Zhongqiu Han
  2025-08-15 12:48 ` Zhongqiu Han
  6 siblings, 1 reply; 15+ messages in thread
From: Christian Loehle @ 2025-07-28 10:09 UTC (permalink / raw)
  To: Zhongqiu Han, rafael, lenb, pavel, tony.luck, reinette.chatre,
	Dave.Martin, james.morse, ulf.hansson, amit.kucheria
  Cc: linux-pm, linux-kernel

On 7/21/25 13:40, Zhongqiu Han wrote:
> Hi all,
> 
> This patch series introduces support for CPU affinity-based latency
> constraints in the PM QoS framework. The motivation is to allow
> finer-grained power management by enabling latency QoS requests to target
> specific CPUs, rather than applying system-wide constraints.
> 
> The current PM QoS framework supports global and per-device CPU latency
> constraints. However, in many real-world scenarios, such as IRQ affinity
> or CPU-bound kernel threads, only a subset of CPUs are
> performance-critical. Applying global constraints in such cases
> unnecessarily prevents other CPUs from entering deeper C-states, leading
> to increased power consumption.
> 
> This series addresses that limitation by introducing a new interface that
> allows latency constraints to be applied to a CPU mask. This is
> particularly useful on heterogeneous platforms (e.g., big.LITTLE) and
> embedded systems where power efficiency is critical for example:
> 
>                         driver A       rt kthread B      module C
>   CPU IDs (mask):         0-3              2-5              6-7
>   target latency(us):     20               30               100
>                           |                |                |
>                           v                v                v
>                           +---------------------------------+
>                           |        PM  QoS  Framework       |
>                           +---------------------------------+
>                           |                |                |
>                           v                v                v
>   CPU IDs (mask):        0-3            2-3,4-5            6-7
>   runtime latency(us):   20             20, 30             100
> 
> The current implementation includes only cpu_affinity_latency_qos_add()
> and cpu_affinity_latency_qos_remove() interfaces. An update interface is
> planned for future submission, along with PM QoS optimizations in the UFS
> subsystem.

So what's needed for the UFS use-case additionally?
Would adding that here be too much?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration
  2025-07-28 10:09 ` [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Christian Loehle
@ 2025-07-28 10:40   ` Zhongqiu Han
  2025-08-02 14:38     ` Christian Loehle
  0 siblings, 1 reply; 15+ messages in thread
From: Zhongqiu Han @ 2025-07-28 10:40 UTC (permalink / raw)
  To: Christian Loehle, rafael, lenb, pavel, tony.luck, reinette.chatre,
	Dave.Martin, james.morse, ulf.hansson, amit.kucheria
  Cc: linux-pm, linux-kernel, Zhongqiu Han

On 7/28/2025 6:09 PM, Christian Loehle wrote:
> On 7/21/25 13:40, Zhongqiu Han wrote:
>> Hi all,
>>
>> This patch series introduces support for CPU affinity-based latency
>> constraints in the PM QoS framework. The motivation is to allow
>> finer-grained power management by enabling latency QoS requests to target
>> specific CPUs, rather than applying system-wide constraints.
>>
>> The current PM QoS framework supports global and per-device CPU latency
>> constraints. However, in many real-world scenarios, such as IRQ affinity
>> or CPU-bound kernel threads, only a subset of CPUs are
>> performance-critical. Applying global constraints in such cases
>> unnecessarily prevents other CPUs from entering deeper C-states, leading
>> to increased power consumption.
>>
>> This series addresses that limitation by introducing a new interface that
>> allows latency constraints to be applied to a CPU mask. This is
>> particularly useful on heterogeneous platforms (e.g., big.LITTLE) and
>> embedded systems where power efficiency is critical for example:
>>
>>                          driver A       rt kthread B      module C
>>    CPU IDs (mask):         0-3              2-5              6-7
>>    target latency(us):     20               30               100
>>                            |                |                |
>>                            v                v                v
>>                            +---------------------------------+
>>                            |        PM  QoS  Framework       |
>>                            +---------------------------------+
>>                            |                |                |
>>                            v                v                v
>>    CPU IDs (mask):        0-3            2-3,4-5            6-7
>>    runtime latency(us):   20             20, 30             100
>>
>> The current implementation includes only cpu_affinity_latency_qos_add()
>> and cpu_affinity_latency_qos_remove() interfaces. An update interface is
>> planned for future submission, along with PM QoS optimizations in the UFS
>> subsystem.
> 
> So what's needed for the UFS use-case additionally?
> Would adding that here be too much?
> 

Hi Christian,
Thanks for your review and discussion~

Currently my plan is only to move forward with the current patch series,
which includes only the below interfaces:

cpu_affinity_latency_qos_add()
cpu_affinity_latency_qos_remove()
cpu_affinity_latency_qos_active()


For most use-cases, seems these three interfaces already sufficient.


The reason I mentioned UFS is to explain why the update
interface cpu_affinity_latency_qos_update()

is not included at this stage. The UFS use-case is planned to
use the cpu_affinity_latency_qos_update() interface in the future, which
is similar to the global CPU PM QoS interface
cpu_latency_qos_update_request().



-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description
  2025-07-21 12:41 ` [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description Zhongqiu Han
@ 2025-08-01 11:27   ` Christian Loehle
  2025-08-04 11:58     ` Zhongqiu Han
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Loehle @ 2025-08-01 11:27 UTC (permalink / raw)
  To: Zhongqiu Han, rafael, lenb, pavel, tony.luck, reinette.chatre,
	Dave.Martin, james.morse, ulf.hansson, amit.kucheria
  Cc: linux-pm, linux-kernel

On 7/21/25 13:41, Zhongqiu Han wrote:
> The documentation for cpu_latency_qos_request_active() incorrectly stated
> the return type as 'int' instead of 'bool', and the return value
> description was incomplete. This patch corrects the return type and
> clarifies the return value semantics.
> 
> Fixes: b8e6e27c626e ("Documentation: PM: QoS: Update to reflect previous code changes")
> Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
> ---
>  Documentation/power/pm_qos_interface.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/power/pm_qos_interface.rst b/Documentation/power/pm_qos_interface.rst
> index 1ede4cafc2e3..c6b8b9cda166 100644
> --- a/Documentation/power/pm_qos_interface.rst
> +++ b/Documentation/power/pm_qos_interface.rst
> @@ -55,9 +55,9 @@ void cpu_latency_qos_remove_request(handle):
>  int cpu_latency_qos_limit():
>    Returns the aggregated value for the CPU latency QoS.
>  
> -int cpu_latency_qos_request_active(handle):
> -  Returns if the request is still active, i.e. it has not been removed from the
> -  CPU latency QoS list.
> +bool cpu_latency_qos_request_active(handle):
> +  Returns true if the request is still active, i.e. it has not been removed from
> +  the CPU latency QoS list.
>  
>  
>  From user space:

I guess this should be swapped in the series with patch 3? (First fix old, then add
new?)
Anyway it applies in and of itself.

Reviewed-by: Christian Loehle <christian.loehle@arm.com>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration
  2025-07-28 10:40   ` Zhongqiu Han
@ 2025-08-02 14:38     ` Christian Loehle
  2025-08-04 13:47       ` Zhongqiu Han
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Loehle @ 2025-08-02 14:38 UTC (permalink / raw)
  To: Zhongqiu Han, rafael, lenb, pavel, tony.luck, reinette.chatre,
	Dave.Martin, james.morse, ulf.hansson, amit.kucheria
  Cc: linux-pm, linux-kernel

On 7/28/25 11:40, Zhongqiu Han wrote:
> On 7/28/2025 6:09 PM, Christian Loehle wrote:
>> On 7/21/25 13:40, Zhongqiu Han wrote:
>>> Hi all,
>>>
>>> This patch series introduces support for CPU affinity-based latency
>>> constraints in the PM QoS framework. The motivation is to allow
>>> finer-grained power management by enabling latency QoS requests to target
>>> specific CPUs, rather than applying system-wide constraints.
>>>
>>> The current PM QoS framework supports global and per-device CPU latency
>>> constraints. However, in many real-world scenarios, such as IRQ affinity
>>> or CPU-bound kernel threads, only a subset of CPUs are
>>> performance-critical. Applying global constraints in such cases
>>> unnecessarily prevents other CPUs from entering deeper C-states, leading
>>> to increased power consumption.
>>>
>>> This series addresses that limitation by introducing a new interface that
>>> allows latency constraints to be applied to a CPU mask. This is
>>> particularly useful on heterogeneous platforms (e.g., big.LITTLE) and
>>> embedded systems where power efficiency is critical for example:
>>>
>>>                          driver A       rt kthread B      module C
>>>    CPU IDs (mask):         0-3              2-5              6-7
>>>    target latency(us):     20               30               100
>>>                            |                |                |
>>>                            v                v                v
>>>                            +---------------------------------+
>>>                            |        PM  QoS  Framework       |
>>>                            +---------------------------------+
>>>                            |                |                |
>>>                            v                v                v
>>>    CPU IDs (mask):        0-3            2-3,4-5            6-7
>>>    runtime latency(us):   20             20, 30             100
>>>
>>> The current implementation includes only cpu_affinity_latency_qos_add()
>>> and cpu_affinity_latency_qos_remove() interfaces. An update interface is
>>> planned for future submission, along with PM QoS optimizations in the UFS
>>> subsystem.
>>
>> So what's needed for the UFS use-case additionally?
>> Would adding that here be too much?
>>
> 
> Hi Christian,
> Thanks for your review and discussion~
> 
> Currently my plan is only to move forward with the current patch series,
> which includes only the below interfaces:
> 
> cpu_affinity_latency_qos_add()
> cpu_affinity_latency_qos_remove()
> cpu_affinity_latency_qos_active()
> 
> 
> For most use-cases, seems these three interfaces already sufficient.

Probably, but IMO there's no real user of the new extended interface yet,
making review harder and lacking justification.

FWIW in 2014 Lina also pushed for something like $SUBJECT
https://lore.kernel.org/all/1407945689-18494-5-git-send-email-lina.iyer@linaro.org/
Lina made an interface to tie the PM QoS to the relevant irq, which I think
was a great idea. Maybe that series is interesting for you, too?

> 
> 
> The reason I mentioned UFS is to explain why the update
> interface cpu_affinity_latency_qos_update()
> 
> is not included at this stage. The UFS use-case is planned to
> use the cpu_affinity_latency_qos_update() interface in the future, which
> is similar to the global CPU PM QoS interface
> cpu_latency_qos_update_request().


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description
  2025-08-01 11:27   ` Christian Loehle
@ 2025-08-04 11:58     ` Zhongqiu Han
  0 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-08-04 11:58 UTC (permalink / raw)
  To: Christian Loehle, rafael, lenb, pavel, tony.luck, reinette.chatre,
	Dave.Martin, james.morse, ulf.hansson, amit.kucheria
  Cc: linux-pm, linux-kernel, Zhongqiu Han

On 8/1/2025 7:27 PM, Christian Loehle wrote:
> On 7/21/25 13:41, Zhongqiu Han wrote:
>> The documentation for cpu_latency_qos_request_active() incorrectly stated
>> the return type as 'int' instead of 'bool', and the return value
>> description was incomplete. This patch corrects the return type and
>> clarifies the return value semantics.
>>
>> Fixes: b8e6e27c626e ("Documentation: PM: QoS: Update to reflect previous code changes")
>> Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
>> ---
>>   Documentation/power/pm_qos_interface.rst | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/power/pm_qos_interface.rst b/Documentation/power/pm_qos_interface.rst
>> index 1ede4cafc2e3..c6b8b9cda166 100644
>> --- a/Documentation/power/pm_qos_interface.rst
>> +++ b/Documentation/power/pm_qos_interface.rst
>> @@ -55,9 +55,9 @@ void cpu_latency_qos_remove_request(handle):
>>   int cpu_latency_qos_limit():
>>     Returns the aggregated value for the CPU latency QoS.
>>   
>> -int cpu_latency_qos_request_active(handle):
>> -  Returns if the request is still active, i.e. it has not been removed from the
>> -  CPU latency QoS list.
>> +bool cpu_latency_qos_request_active(handle):
>> +  Returns true if the request is still active, i.e. it has not been removed from
>> +  the CPU latency QoS list.
>>   
>>   
>>   From user space:
> 
> I guess this should be swapped in the series with patch 3? (First fix old, then add
> new?)
> Anyway it applies in and of itself.
> 
> Reviewed-by: Christian Loehle <christian.loehle@arm.com>
> 
Thanks Christian for the review~

Seems swapping the patches might help prevent misunderstanding and make 
the sequence clearer, if needed, I can swap them.

-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration
  2025-08-02 14:38     ` Christian Loehle
@ 2025-08-04 13:47       ` Zhongqiu Han
  0 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-08-04 13:47 UTC (permalink / raw)
  To: Christian Loehle, rafael, lenb, pavel, tony.luck, reinette.chatre,
	Dave.Martin, james.morse, ulf.hansson, amit.kucheria
  Cc: linux-pm, linux-kernel, Zhongqiu Han

On 8/2/2025 10:38 PM, Christian Loehle wrote:
> On 7/28/25 11:40, Zhongqiu Han wrote:
>> On 7/28/2025 6:09 PM, Christian Loehle wrote:
>>> On 7/21/25 13:40, Zhongqiu Han wrote:
>>>> Hi all,
>>>>
>>>> This patch series introduces support for CPU affinity-based latency
>>>> constraints in the PM QoS framework. The motivation is to allow
>>>> finer-grained power management by enabling latency QoS requests to target
>>>> specific CPUs, rather than applying system-wide constraints.
>>>>
>>>> The current PM QoS framework supports global and per-device CPU latency
>>>> constraints. However, in many real-world scenarios, such as IRQ affinity
>>>> or CPU-bound kernel threads, only a subset of CPUs are
>>>> performance-critical. Applying global constraints in such cases
>>>> unnecessarily prevents other CPUs from entering deeper C-states, leading
>>>> to increased power consumption.
>>>>
>>>> This series addresses that limitation by introducing a new interface that
>>>> allows latency constraints to be applied to a CPU mask. This is
>>>> particularly useful on heterogeneous platforms (e.g., big.LITTLE) and
>>>> embedded systems where power efficiency is critical for example:
>>>>
>>>>                           driver A       rt kthread B      module C
>>>>     CPU IDs (mask):         0-3              2-5              6-7
>>>>     target latency(us):     20               30               100
>>>>                             |                |                |
>>>>                             v                v                v
>>>>                             +---------------------------------+
>>>>                             |        PM  QoS  Framework       |
>>>>                             +---------------------------------+
>>>>                             |                |                |
>>>>                             v                v                v
>>>>     CPU IDs (mask):        0-3            2-3,4-5            6-7
>>>>     runtime latency(us):   20             20, 30             100
>>>>
>>>> The current implementation includes only cpu_affinity_latency_qos_add()
>>>> and cpu_affinity_latency_qos_remove() interfaces. An update interface is
>>>> planned for future submission, along with PM QoS optimizations in the UFS
>>>> subsystem.
>>>
>>> So what's needed for the UFS use-case additionally?
>>> Would adding that here be too much?
>>>
>>
>> Hi Christian,
>> Thanks for your review and discussion~
>>
>> Currently my plan is only to move forward with the current patch series,
>> which includes only the below interfaces:
>>
>> cpu_affinity_latency_qos_add()
>> cpu_affinity_latency_qos_remove()
>> cpu_affinity_latency_qos_active()
>>
>>
>> For most use-cases, seems these three interfaces already sufficient.
> 
> Probably, but IMO there's no real user of the new extended interface yet,
> making review harder and lacking justification.
> 
> FWIW in 2014 Lina also pushed for something like $SUBJECT
> https://lore.kernel.org/all/1407945689-18494-5-git-send-email-lina.iyer@linaro.org/
> Lina made an interface to tie the PM QoS to the relevant irq, which I think
> was a great idea. Maybe that series is interesting for you, too?
> 

Hi Christian,
Thanks for the review~

Just to clarify: in patch 5/5 of the current series, I’ve included a
user of the new extended interface — specifically,
cpu_affinity_latency_qos_active() is used internally within the add
remove interfaces.


I’ve roughly reviewed this patchset you mentioned. Please correct me if
my understanding is inaccurate.

https://lore.kernel.org/all/1407945689-18494-5-git-send-email-lina.iyer@linaro.org/

It seems that this patch series introduces an alternative implementation
and attempts to add a new type of PM QoS request — one that targets IRQs
instead of CPUs. Specifically, when the IRQ affinity changes, the
corresponding CPU latency constraint is updated to reflect the new CPUs
that the IRQ is now affine to.


And It appears that Kevin also recommended implementing this feature
using the per-device API:


https://lore.kernel.org/all/7h4mx9wdxe.fsf@paris.lan/

---->>> From Kevin:
I agree this is a needed feature.  I didn't study it in detail yet, but
after a quick glance, it looks like a good approach.
---->>>

https://lore.kernel.org/all/7hd2blerqz.fsf@paris.lan/

---->>> From Kevin:
I'm curious if you looked at using the per-device QoS API for this
instead of expending the system-wide API.  IOW, from a per-device QoS
POV, a CPU is no different than any other device, and since we already
have the per-device QoS API, I wondered if that might be a better choice
to implment this per-CPU feature.
---->>>


May I know are you suggesting that I should evaluate whether IRQ
affinity scenarios could also be valid use cases for the
cpu affinity latency pm qos interface?
However, there's a more fundamental assumption behind this — that a
particular IRQ actually requires a CPU latency PM QoS constraint, right?



>>
>>
>> The reason I mentioned UFS is to explain why the update
>> interface cpu_affinity_latency_qos_update()
>>
>> is not included at this stage. The UFS use-case is planned to
>> use the cpu_affinity_latency_qos_update() interface in the future, which
>> is similar to the global CPU PM QoS interface
>> cpu_latency_qos_update_request().
> 



-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration
  2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
                   ` (5 preceding siblings ...)
  2025-07-28 10:09 ` [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Christian Loehle
@ 2025-08-15 12:48 ` Zhongqiu Han
  6 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-08-15 12:48 UTC (permalink / raw)
  To: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle
  Cc: linux-pm, linux-kernel, zhongqiu.han

On 7/21/2025 8:40 PM, Zhongqiu Han wrote:
> Hi all,
> 
> This patch series introduces support for CPU affinity-based latency
> constraints in the PM QoS framework. The motivation is to allow
> finer-grained power management by enabling latency QoS requests to target
> specific CPUs, rather than applying system-wide constraints.
> 
> The current PM QoS framework supports global and per-device CPU latency
> constraints. However, in many real-world scenarios, such as IRQ affinity
> or CPU-bound kernel threads, only a subset of CPUs are
> performance-critical. Applying global constraints in such cases
> unnecessarily prevents other CPUs from entering deeper C-states, leading
> to increased power consumption.
> 
> This series addresses that limitation by introducing a new interface that
> allows latency constraints to be applied to a CPU mask. This is
> particularly useful on heterogeneous platforms (e.g., big.LITTLE) and
> embedded systems where power efficiency is critical for example:
> 
>                          driver A       rt kthread B      module C
>    CPU IDs (mask):         0-3              2-5              6-7
>    target latency(us):     20               30               100
>                            |                |                |
>                            v                v                v
>                            +---------------------------------+
>                            |        PM  QoS  Framework       |
>                            +---------------------------------+
>                            |                |                |
>                            v                v                v
>    CPU IDs (mask):        0-3            2-3,4-5            6-7
>    runtime latency(us):   20             20, 30             100
> 
> The current implementation includes only cpu_affinity_latency_qos_add()
> and cpu_affinity_latency_qos_remove() interfaces. An update interface is
> planned for future submission, along with PM QoS optimizations in the UFS
> subsystem.
> 
> Patch1 introduces the core support for CPU affinity latency QoS in the PM
> QoS framework.
> 
> Patch2 removes redundant KERN_ERR prefixes in WARN() calls in the global
> CPU PM QoS interface. This change addresses issues in existing code and is
> not related to the new interface introduced in this patch series.
> 
> Patch3 adds documentation for the new interface.
> 
> Patch4 fixes a minor documentation issue related to the return type of
> cpu_latency_qos_request_active(). This change addresses issues in existing
> doc and is not related to the new interface introduced in this patch
> series.
> 
> Patch5 updates the resctrl pseudo-locking logic to use the new CPU
> affinity latency QoS helpers, improving clarity and consistency. The only
> functional and beneficial change is that the new interface actively wakes
> up CPUs whose latency QoS values have changed, ensuring the latency limit
> takes effect immediately.

Hi Rafael,
I hope you're doing well. I just wanted to kindly check in regarding
current patch I submitted a while ago.

I understand things can get busy, and there's absolutely no rush — just
wanted to make sure it hasn't been missed.

Thank you~


> 
> Changes since v1:
> - Rebased on top of current next.
> - Resolve the compilation warning due to a missing static function
>    declaration.
> - Remove the conditional compilation based on CONFIG_CPU_IDLE and make it
>    depend solely on CONFIG_PM.
> - Add support for cpu_affinity_latency_qos_active.
> - Remove cpu_affinity_latency_qos_update; will reintroduce it when needed
>    in the future.
> - Optimize the code, for example by using cpu_affinity_latency_qos_active
>    inside the add/remove functions to enhance robustness.
> - Refine the commit message and fix a few minor issues unrelated to this
>    series.
> - Refactor the CPU latency PM QoS logic of resctrl pseudo_lock using the
>    interfaces provided by this series.
> - Link to v1: https://lore.kernel.org/all/20250424095228.1112558-1-quic_zhonhan@quicinc.com/
> 
> Zhongqiu Han (5):
>    PM: QoS: Add support for CPU affinity latency PM QoS
>    PM: QOS: Remove unnecessary KERN_ERR on WARN() calls
>    Documentation: PM: QoS: Add CPU affinity latency PM QoS Interface
>      documentation
>    Documentation: PM: QoS: Fix return type and return value description
>    resctrl: Replace PM QoS logic with cpu_affinity_latency_qos_* helpers
> 
>   Documentation/power/pm_qos_interface.rst |  63 ++++++++-
>   fs/resctrl/pseudo_lock.c                 |  51 +------
>   include/linux/pm_qos.h                   |  40 ++++++
>   include/linux/resctrl.h                  |   3 +-
>   kernel/power/qos.c                       | 166 ++++++++++++++++++++++-
>   5 files changed, 268 insertions(+), 55 deletions(-)
> 
> 
> base-commit: 024e09e444bd2b06aee9d1f3fe7b313c7a2df1bb


-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS
  2025-07-21 12:41 ` [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS Zhongqiu Han
@ 2025-08-21 10:37   ` Rafael J. Wysocki
  2025-08-22 10:31     ` Zhongqiu Han
  0 siblings, 1 reply; 15+ messages in thread
From: Rafael J. Wysocki @ 2025-08-21 10:37 UTC (permalink / raw)
  To: Zhongqiu Han
  Cc: rafael, lenb, pavel, tony.luck, reinette.chatre, Dave.Martin,
	james.morse, ulf.hansson, amit.kucheria, christian.loehle,
	linux-pm, linux-kernel

On Mon, Jul 21, 2025 at 2:41 PM Zhongqiu Han <quic_zhonhan@quicinc.com> wrote:
>
> Currently, the PM QoS framework supports global CPU latency QoS and
> per-device CPU latency QoS requests. An example of using global CPU
> latency QoS is a commit 2777e73fc154 ("scsi: ufs: core: Add CPU latency
> QoS support for UFS driver") that improved random io performance by 15%
> for ufs on specific device platform.
>
> However, this prevents all CPUs in the system from entering C states.
> Typically, some threads or drivers know which specific CPUs they are
> interested in. For example, drivers with IRQ affinity only want interrupts
> to wake up and be handled on specific CPUs. Similarly, kernel thread bound
> to specific CPUs through affinity only care about the latency of those
> particular CPUs.
>
> This patch introduces support for partial CPUs PM QoS using a CPU affinity
> mask, allowing flexible and more precise latency QoS settings for specific
> CPUs. This can help save power, especially on heterogeneous platforms with
> big and little cores, as well as some power-conscious embedded systems for
> example:
>
>                         driver A       rt kthread B      module C
>   CPU IDs (mask):         0-3              2-5              6-7
>   target latency(us):     20               30               100
>                           |                |                |
>                           v                v                v
>                           +---------------------------------+
>                           |        PM  QoS  Framework       |
>                           +---------------------------------+
>                           |                |                |
>                           v                v                v
>   CPU IDs (mask):        0-3            2-3,4-5            6-7
>   runtime latency(us):   20             20, 30             100
>
> Implement this support based on per-device CPU latency PM QoS.

I have a few concerns regarding this patch.

The first one is the naming.

You want to be able to set wakeup latency QoS limits for multiple CPUs
at the same time.  Fair enough, but what does it have to do with
affinity of any sort?

Why don't you call the functions cpu_latency_qos_add_multiple() and so on?

> Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
> ---
>  include/linux/pm_qos.h |  40 +++++++++++
>  kernel/power/qos.c     | 160 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 200 insertions(+)
>
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 4a69d4af3ff8..2dbad825f8bd 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -131,6 +131,15 @@ enum pm_qos_req_action {
>         PM_QOS_REMOVE_REQ       /* Remove an existing request */
>  };
>
> +/* cpu affinity pm latency qos request handle */
> +struct cpu_affinity_qos_req {
> +       struct list_head list;

Is the list really an adequate data structure here?

The number of CPUs in the mask is known at the request addition time
and it fails completely if the request cannot be added for one CPU
IIUC, so why don't you use an array of requests instead?

> +       union {
> +               struct dev_pm_qos_request req;
> +               void *req_ptr;
> +       };

Why do you need the union here?

Checking if the request is active only requires inspecting one CPU
involved in it AFAICS.

> +};
> +
>  static inline int dev_pm_qos_request_active(struct dev_pm_qos_request *req)
>  {
>         return req->dev != NULL;
> @@ -208,6 +217,13 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>                 PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
>                 pm_qos_read_value(&dev->power.qos->resume_latency);
>  }
> +
> +int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
> +                                 const cpumask_t *affinity_mask, s32 latency_value);
> +int cpu_affinity_latency_qos_remove(struct cpu_affinity_qos_req *pm_req);
> +int cpu_affinity_latency_qos_release(struct cpu_affinity_qos_req *pm_req);

Why is a separate release function needed?

Also, why don't you think that a separate function for updating an
existing request would be useful?

> +bool cpu_affinity_latency_qos_active(struct cpu_affinity_qos_req *pm_req);
> +void wakeup_qos_affinity_idle_cpu(int cpu);
>  #else
>  static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
>                                                           s32 mask)
> @@ -289,6 +305,30 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>  {
>         return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>  }
> +
> +static inline int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
> +                                               const cpumask_t *affinity_mask,
> +                                               s32 latency_value)
> +{
> +       return 0;
> +}
> +
> +static inline int cpu_affinity_latency_qos_remove(
> +                  struct cpu_affinity_qos_req *pm_req)
> +{
> +       return 0;
> +}
> +static inline int cpu_affinity_latency_qos_release(
> +                  struct cpu_affinity_qos_req *pm_req)
> +{
> +       return 0;
> +}
> +static inline bool cpu_affinity_latency_qos_active(
> +                   struct cpu_affinity_qos_req *pm_req)
> +{
> +       return false;
> +}
> +static inline void wakeup_qos_affinity_idle_cpu(int cpu) {}
>  #endif
>
>  static inline int freq_qos_request_active(struct freq_qos_request *req)
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 4244b069442e..5e507ed8d077 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -335,6 +335,166 @@ void cpu_latency_qos_remove_request(struct pm_qos_request *req)
>  }
>  EXPORT_SYMBOL_GPL(cpu_latency_qos_remove_request);
>
> +#ifdef CONFIG_PM
> +
> +/**
> + * wakeup_qos_affinity_idle_cpu - break one specific cpu out of idle.
> + * @cpu: the CPU to be woken up from idle.
> + */
> +void wakeup_qos_affinity_idle_cpu(int cpu)
> +{
> +       preempt_disable();
> +       if (cpu != smp_processor_id() && cpu_online(cpu))
> +               wake_up_if_idle(cpu);
> +       preempt_enable();
> +}

This duplicates code from wake_up_all_idle_cpus() that duplication is
easily avoidable.

> +
> +/**
> + * cpu_affinity_latency_qos_add - Add new CPU affinity latency QoS request.
> + * @pm_req: Pointer to a preallocated handle.
> + * @affinity_mask: Mask to determine which CPUs need latency QoS.
> + * @latency_value: New requested constraint value.
> + *
> + * Use @latency_value to initialize the request handle pointed to by @pm_req,
> + * insert it as a new entry to the CPU latency QoS list and recompute the
> + * effective QoS constraint for that list, @affinity_mask determine which CPUs
> + * need the latency QoS.
> + *
> + * Callers need to save the handle for later use in updates and removal of the
> + * QoS request represented by it.
> + *

It would be good to also say that callers are responsible for
synchronizing the calls to add and remove functions for the same
request.

> + * Returns 0 or a positive value on success, or a negative error code on failure.
> + */
> +int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
> +                                 const cpumask_t *affinity_mask,
> +                                 s32 latency_value)
> +{
> +       int cpu;
> +       cpumask_t actual_mask;
> +       struct cpu_affinity_qos_req *cpu_pm_req;
> +       int ret = 0;
> +
> +       if (!pm_req) {
> +               pr_err("%s: invalid PM Qos request\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (cpu_affinity_latency_qos_active(pm_req)) {
> +               WARN(1, "%s called for already added request\n", __func__);
> +               return -EBUSY;
> +       }

The usual pattern for checks like this is

if (WARN(cpu_affinity_latency_qos_active(pm_req), message))
         return error;

And, which is not related to the above, if a function is defined in
the same file as its caller, I prefer it to be defined before its
caller.

> +
> +       INIT_LIST_HEAD(&pm_req->list);
> +
> +       if (!affinity_mask || cpumask_empty(affinity_mask) ||
> +           latency_value < 0) {
> +               pr_err("%s: invalid PM Qos request value\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       for_each_cpu(cpu, affinity_mask) {
> +               cpu_pm_req = kzalloc(sizeof(struct cpu_affinity_qos_req),
> +                                    GFP_KERNEL);
> +               if (!cpu_pm_req) {
> +                       ret = -ENOMEM;
> +                       goto out_err;
> +               }
> +               ret = dev_pm_qos_add_request(get_cpu_device(cpu),
> +                                            &cpu_pm_req->req,
> +                                            DEV_PM_QOS_RESUME_LATENCY,
> +                                            latency_value);
> +               if (ret < 0) {
> +                       pr_err("failed to add latency req for cpu%d", cpu);

Why do you want to print an error message here?  Is this anything that
the sysadmin should know about?  If not, then maybe dev_dbg() should
be sufficient?

> +                       kfree(cpu_pm_req);
> +                       goto out_err;
> +               } else if (ret > 0) {
> +                       wakeup_qos_affinity_idle_cpu(cpu);
> +               }
> +
> +               cpumask_set_cpu(cpu, &actual_mask);
> +               list_add(&cpu_pm_req->list, &pm_req->list);
> +       }
> +
> +       pr_info("PM Qos latency: %d added on cpus %*pb\n", latency_value,
> +               cpumask_pr_args(&actual_mask));

I'm not sure why the actual_mask variable is needed.  AFAICS, the
function fails anyway if the request cannot be added for any CPU in
the original mask.

> +       pm_req->req_ptr = pm_req;
> +       return ret;
> +
> +out_err:
> +       cpu_affinity_latency_qos_release(pm_req);
> +       pr_err("failed to add PM QoS latency req, removed all added requests\n");

A message about this has already been printed.  Why print another one?

> +       return ret;
> +}
> +EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_add);
> +
> +
> +/**
> + * cpu_affinity_latency_qos_remove - Remove an existing CPU affinity latency QoS.
> + * @pm_req: Handle to the QoS request to be removed.
> + *
> + * Remove the CPU latency QoS request represented by @pm_req from the CPU latency
> + * QoS list. This handle must have been previously initialized and added via
> + * cpu_affinity_latency_qos_add().
> + */
> +int cpu_affinity_latency_qos_remove(struct cpu_affinity_qos_req *pm_req)
> +{
> +       if (!pm_req) {
> +               pr_err("%s: invalid PM Qos request value\n", __func__);
> +               return -EINVAL;
> +       }
> +
> +       if (!cpu_affinity_latency_qos_active(pm_req)) {
> +               WARN(1, "%s called for unknown object\n", __func__);
> +               return -EINVAL;
> +       }

Same pattern comment as above applies here.

> +
> +       return cpu_affinity_latency_qos_release(pm_req);
> +}
> +EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_remove);
> +
> +/**
> + * cpu_affinity_latency_qos_release - Release pm_reqs latency QoS resource.
> + * @pm_req: QoS request to be released.
> + *
> + * Release pm_reqs managed CPU affinity latency QoS resource.
> + *
> + * Returns a negative value indicates failure.
> + */
> +int cpu_affinity_latency_qos_release(struct cpu_affinity_qos_req *pm_req)
> +{
> +       int ret = 0;
> +       struct cpu_affinity_qos_req *cpu_pm_req, *next;
> +
> +       list_for_each_entry_safe(cpu_pm_req, next, &pm_req->list, list) {
> +               ret = dev_pm_qos_remove_request(&cpu_pm_req->req);
> +               if (ret < 0)
> +                       pr_err("failed to remove qos request for %s\n",
> +                              dev_name(cpu_pm_req->req.dev));
> +               list_del(&cpu_pm_req->list);
> +               kfree(cpu_pm_req);
> +               cpu_pm_req = NULL;
> +       }
> +
> +       memset(pm_req, 0, sizeof(*pm_req));
> +       return ret;
> +}
> +
> +/**
> + * cpu_affinity_latency_qos_active - Check if a CPU affinity latency QoS
> + * request is active.
> + * @pm_req: Handle to the QoS request.
> + *
> + * Return: 'true' if @pm_req has been added to the CPU latency QoS list,
> + * 'false' otherwise.
> + */
> +bool cpu_affinity_latency_qos_active(struct cpu_affinity_qos_req *pm_req)
> +{
> +       return pm_req->req_ptr == pm_req;
> +}
> +EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_active);
> +
> +#endif /* CONFIG_PM */
> +
>  /* User space interface to the CPU latency QoS via misc device. */
>
>  static int cpu_latency_qos_open(struct inode *inode, struct file *filp)
> --

I'll look at the rest of the series whey all of my comments on this
patch have been addressed.

Thanks!

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS
  2025-08-21 10:37   ` Rafael J. Wysocki
@ 2025-08-22 10:31     ` Zhongqiu Han
  0 siblings, 0 replies; 15+ messages in thread
From: Zhongqiu Han @ 2025-08-22 10:31 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: lenb, pavel, tony.luck, reinette.chatre, Dave.Martin, james.morse,
	ulf.hansson, amit.kucheria, christian.loehle, linux-pm,
	linux-kernel, Zhongqiu Han, zhongqiu.han

On 8/21/2025 6:37 PM, Rafael J. Wysocki wrote:
> On Mon, Jul 21, 2025 at 2:41 PM Zhongqiu Han <quic_zhonhan@quicinc.com> wrote:
>>
>> Currently, the PM QoS framework supports global CPU latency QoS and
>> per-device CPU latency QoS requests. An example of using global CPU
>> latency QoS is a commit 2777e73fc154 ("scsi: ufs: core: Add CPU latency
>> QoS support for UFS driver") that improved random io performance by 15%
>> for ufs on specific device platform.
>>
>> However, this prevents all CPUs in the system from entering C states.
>> Typically, some threads or drivers know which specific CPUs they are
>> interested in. For example, drivers with IRQ affinity only want interrupts
>> to wake up and be handled on specific CPUs. Similarly, kernel thread bound
>> to specific CPUs through affinity only care about the latency of those
>> particular CPUs.
>>
>> This patch introduces support for partial CPUs PM QoS using a CPU affinity
>> mask, allowing flexible and more precise latency QoS settings for specific
>> CPUs. This can help save power, especially on heterogeneous platforms with
>> big and little cores, as well as some power-conscious embedded systems for
>> example:
>>
>>                          driver A       rt kthread B      module C
>>    CPU IDs (mask):         0-3              2-5              6-7
>>    target latency(us):     20               30               100
>>                            |                |                |
>>                            v                v                v
>>                            +---------------------------------+
>>                            |        PM  QoS  Framework       |
>>                            +---------------------------------+
>>                            |                |                |
>>                            v                v                v
>>    CPU IDs (mask):        0-3            2-3,4-5            6-7
>>    runtime latency(us):   20             20, 30             100
>>
>> Implement this support based on per-device CPU latency PM QoS.

Hi Rafael,
Thanks for your review~

> 
> I have a few concerns regarding this patch.
> 
> The first one is the naming.
> 
> You want to be able to set wakeup latency QoS limits for multiple CPUs
> at the same time.  Fair enough, but what does it have to do with
> affinity of any sort?
> 
> Why don't you call the functions cpu_latency_qos_add_multiple() and so on?

My original intention was to bind a specific request to a specific group
of CPUs. But you're right — the essence here is really just adding QoS
to multiple CPUs. I will rename it accordingly.


> 
>> Signed-off-by: Zhongqiu Han <quic_zhonhan@quicinc.com>
>> ---
>>   include/linux/pm_qos.h |  40 +++++++++++
>>   kernel/power/qos.c     | 160 +++++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 200 insertions(+)
>>
>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>> index 4a69d4af3ff8..2dbad825f8bd 100644
>> --- a/include/linux/pm_qos.h
>> +++ b/include/linux/pm_qos.h
>> @@ -131,6 +131,15 @@ enum pm_qos_req_action {
>>          PM_QOS_REMOVE_REQ       /* Remove an existing request */
>>   };
>>
>> +/* cpu affinity pm latency qos request handle */
>> +struct cpu_affinity_qos_req {
>> +       struct list_head list;
> 
> Is the list really an adequate data structure here?
> 
> The number of CPUs in the mask is known at the request addition time
> and it fails completely if the request cannot be added for one CPU
> IIUC, so why don't you use an array of requests instead?

Yes, using an array is sufficient. the list tend to cause poor cache
locality and other issues. Thanks for your sharing.

> 
>> +       union {
>> +               struct dev_pm_qos_request req;
>> +               void *req_ptr;
>> +       };
> 
> Why do you need the union here?
> 
> Checking if the request is active only requires inspecting one CPU
> involved in it AFAICS.

yes, I can find an anchor point to determine whether it's active, for
example by reusing dev_pm_qos_request_active to check one of the CPUs
instead of using such obscure and complex data structures.

> 
>> +};
>> +
>>   static inline int dev_pm_qos_request_active(struct dev_pm_qos_request *req)
>>   {
>>          return req->dev != NULL;
>> @@ -208,6 +217,13 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>>                  PM_QOS_RESUME_LATENCY_NO_CONSTRAINT :
>>                  pm_qos_read_value(&dev->power.qos->resume_latency);
>>   }
>> +
>> +int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
>> +                                 const cpumask_t *affinity_mask, s32 latency_value);
>> +int cpu_affinity_latency_qos_remove(struct cpu_affinity_qos_req *pm_req);
>> +int cpu_affinity_latency_qos_release(struct cpu_affinity_qos_req *pm_req);
> 
> Why is a separate release function needed?

You're right, that part was redundant. I feel a bit awkward about it.


> 
> Also, why don't you think that a separate function for updating an
> existing request would be useful?

Yes, exactly, I also think it's needed. However, as I mentioned in the
cover letter, I haven't found actual users for it at the moment. This
patch series is intended to help reduce power consumption, so either I
or other developers can submit a request to use the update function when
it's needed in the future. As for me, I already plan to propose a patch
that uses the update interface in the UFS module. Since UFS is not my
technical focus, there are still some pending items on that from my
collaborator.

> 
>> +bool cpu_affinity_latency_qos_active(struct cpu_affinity_qos_req *pm_req);
>> +void wakeup_qos_affinity_idle_cpu(int cpu);
>>   #else
>>   static inline enum pm_qos_flags_status __dev_pm_qos_flags(struct device *dev,
>>                                                            s32 mask)
>> @@ -289,6 +305,30 @@ static inline s32 dev_pm_qos_raw_resume_latency(struct device *dev)
>>   {
>>          return PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
>>   }
>> +
>> +static inline int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
>> +                                               const cpumask_t *affinity_mask,
>> +                                               s32 latency_value)
>> +{
>> +       return 0;
>> +}
>> +
>> +static inline int cpu_affinity_latency_qos_remove(
>> +                  struct cpu_affinity_qos_req *pm_req)
>> +{
>> +       return 0;
>> +}
>> +static inline int cpu_affinity_latency_qos_release(
>> +                  struct cpu_affinity_qos_req *pm_req)
>> +{
>> +       return 0;
>> +}
>> +static inline bool cpu_affinity_latency_qos_active(
>> +                   struct cpu_affinity_qos_req *pm_req)
>> +{
>> +       return false;
>> +}
>> +static inline void wakeup_qos_affinity_idle_cpu(int cpu) {}
>>   #endif
>>
>>   static inline int freq_qos_request_active(struct freq_qos_request *req)
>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>> index 4244b069442e..5e507ed8d077 100644
>> --- a/kernel/power/qos.c
>> +++ b/kernel/power/qos.c
>> @@ -335,6 +335,166 @@ void cpu_latency_qos_remove_request(struct pm_qos_request *req)
>>   }
>>   EXPORT_SYMBOL_GPL(cpu_latency_qos_remove_request);
>>
>> +#ifdef CONFIG_PM
>> +
>> +/**
>> + * wakeup_qos_affinity_idle_cpu - break one specific cpu out of idle.
>> + * @cpu: the CPU to be woken up from idle.
>> + */
>> +void wakeup_qos_affinity_idle_cpu(int cpu)
>> +{
>> +       preempt_disable();
>> +       if (cpu != smp_processor_id() && cpu_online(cpu))
>> +               wake_up_if_idle(cpu);
>> +       preempt_enable();
>> +}
> 
> This duplicates code from wake_up_all_idle_cpus() that duplication is
> easily avoidable.

Sorry about that. I'll explore a cleaner solution, possibly by wrapping
it as an function and integrating the change into
sched module func wake_up_all_idle_cpus().

> 
>> +
>> +/**
>> + * cpu_affinity_latency_qos_add - Add new CPU affinity latency QoS request.
>> + * @pm_req: Pointer to a preallocated handle.
>> + * @affinity_mask: Mask to determine which CPUs need latency QoS.
>> + * @latency_value: New requested constraint value.
>> + *
>> + * Use @latency_value to initialize the request handle pointed to by @pm_req,
>> + * insert it as a new entry to the CPU latency QoS list and recompute the
>> + * effective QoS constraint for that list, @affinity_mask determine which CPUs
>> + * need the latency QoS.
>> + *
>> + * Callers need to save the handle for later use in updates and removal of the
>> + * QoS request represented by it.
>> + *
> 
> It would be good to also say that callers are responsible for
> synchronizing the calls to add and remove functions for the same
> request.

Thanks for pointing that out.The active api check alone can't really
prevent misuse by users.
I might also need to review the code and documentation of the
cpu_latency_qos_add_request interface, and include the fix in the next
patch version.


> 
>> + * Returns 0 or a positive value on success, or a negative error code on failure.
>> + */
>> +int cpu_affinity_latency_qos_add(struct cpu_affinity_qos_req *pm_req,
>> +                                 const cpumask_t *affinity_mask,
>> +                                 s32 latency_value)
>> +{
>> +       int cpu;
>> +       cpumask_t actual_mask;
>> +       struct cpu_affinity_qos_req *cpu_pm_req;
>> +       int ret = 0;
>> +
>> +       if (!pm_req) {
>> +               pr_err("%s: invalid PM Qos request\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (cpu_affinity_latency_qos_active(pm_req)) {
>> +               WARN(1, "%s called for already added request\n", __func__);
>> +               return -EBUSY;
>> +       }
> 
> The usual pattern for checks like this is
> 
> if (WARN(cpu_affinity_latency_qos_active(pm_req), message))
>           return error;

Got it, Thanks, will fix it~

> 
> And, which is not related to the above, if a function is defined in
> the same file as its caller, I prefer it to be defined before its
> caller.

Got it, that makes sense. I'll fix it~

> 
>> +
>> +       INIT_LIST_HEAD(&pm_req->list);
>> +
>> +       if (!affinity_mask || cpumask_empty(affinity_mask) ||
>> +           latency_value < 0) {
>> +               pr_err("%s: invalid PM Qos request value\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>> +       for_each_cpu(cpu, affinity_mask) {
>> +               cpu_pm_req = kzalloc(sizeof(struct cpu_affinity_qos_req),
>> +                                    GFP_KERNEL);
>> +               if (!cpu_pm_req) {
>> +                       ret = -ENOMEM;
>> +                       goto out_err;
>> +               }
>> +               ret = dev_pm_qos_add_request(get_cpu_device(cpu),
>> +                                            &cpu_pm_req->req,
>> +                                            DEV_PM_QOS_RESUME_LATENCY,
>> +                                            latency_value);
>> +               if (ret < 0) {
>> +                       pr_err("failed to add latency req for cpu%d", cpu);
> 
> Why do you want to print an error message here?  Is this anything that
> the sysadmin should know about?  If not, then maybe dev_dbg() should
> be sufficient?

Fair point. I initially considered a failure to set PM QoS as a
significant, rare, and hard-to-reproduce issue. However, it seems that
such debugging concerns are more relevant to developers than to system
administrators. I will fix it.


> 
>> +                       kfree(cpu_pm_req);
>> +                       goto out_err;
>> +               } else if (ret > 0) {
>> +                       wakeup_qos_affinity_idle_cpu(cpu);
>> +               }
>> +
>> +               cpumask_set_cpu(cpu, &actual_mask);
>> +               list_add(&cpu_pm_req->list, &pm_req->list);
>> +       }
>> +
>> +       pr_info("PM Qos latency: %d added on cpus %*pb\n", latency_value,
>> +               cpumask_pr_args(&actual_mask));
> 
> I'm not sure why the actual_mask variable is needed.  AFAICS, the
> function fails anyway if the request cannot be added for any CPU in
> the original mask.

This is because I'm concerned that the mask passed by the user might
contain invalid values, which could result in some CPUs not being
traversed at all. (Please note, this isn't a PM QoS failure — it's that
the loop doesn't even get entered.) Users might be confused about what's
going wrong, so I wanted to give them a hint. That said, I’m also open
to dropping this idea if it seems odd or unnecessary.


> 
>> +       pm_req->req_ptr = pm_req;
>> +       return ret;
>> +
>> +out_err:
>> +       cpu_affinity_latency_qos_release(pm_req);
>> +       pr_err("failed to add PM QoS latency req, removed all added requests\n");
> 
> A message about this has already been printed.  Why print another one?

yes, one should be enough and maybe just in out_err: .

> 
>> +       return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_add);
>> +
>> +
>> +/**
>> + * cpu_affinity_latency_qos_remove - Remove an existing CPU affinity latency QoS.
>> + * @pm_req: Handle to the QoS request to be removed.
>> + *
>> + * Remove the CPU latency QoS request represented by @pm_req from the CPU latency
>> + * QoS list. This handle must have been previously initialized and added via
>> + * cpu_affinity_latency_qos_add().
>> + */
>> +int cpu_affinity_latency_qos_remove(struct cpu_affinity_qos_req *pm_req)
>> +{
>> +       if (!pm_req) {
>> +               pr_err("%s: invalid PM Qos request value\n", __func__);
>> +               return -EINVAL;
>> +       }
>> +
>> +       if (!cpu_affinity_latency_qos_active(pm_req)) {
>> +               WARN(1, "%s called for unknown object\n", __func__);
>> +               return -EINVAL;
>> +       }
> 
> Same pattern comment as above applies here.

Got it~

> 
>> +
>> +       return cpu_affinity_latency_qos_release(pm_req);
>> +}
>> +EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_remove);
>> +
>> +/**
>> + * cpu_affinity_latency_qos_release - Release pm_reqs latency QoS resource.
>> + * @pm_req: QoS request to be released.
>> + *
>> + * Release pm_reqs managed CPU affinity latency QoS resource.
>> + *
>> + * Returns a negative value indicates failure.
>> + */
>> +int cpu_affinity_latency_qos_release(struct cpu_affinity_qos_req *pm_req)
>> +{
>> +       int ret = 0;
>> +       struct cpu_affinity_qos_req *cpu_pm_req, *next;
>> +
>> +       list_for_each_entry_safe(cpu_pm_req, next, &pm_req->list, list) {
>> +               ret = dev_pm_qos_remove_request(&cpu_pm_req->req);
>> +               if (ret < 0)
>> +                       pr_err("failed to remove qos request for %s\n",
>> +                              dev_name(cpu_pm_req->req.dev));
>> +               list_del(&cpu_pm_req->list);
>> +               kfree(cpu_pm_req);
>> +               cpu_pm_req = NULL;
>> +       }
>> +
>> +       memset(pm_req, 0, sizeof(*pm_req));
>> +       return ret;
>> +}
>> +
>> +/**
>> + * cpu_affinity_latency_qos_active - Check if a CPU affinity latency QoS
>> + * request is active.
>> + * @pm_req: Handle to the QoS request.
>> + *
>> + * Return: 'true' if @pm_req has been added to the CPU latency QoS list,
>> + * 'false' otherwise.
>> + */
>> +bool cpu_affinity_latency_qos_active(struct cpu_affinity_qos_req *pm_req)
>> +{
>> +       return pm_req->req_ptr == pm_req;
>> +}
>> +EXPORT_SYMBOL_GPL(cpu_affinity_latency_qos_active);
>> +
>> +#endif /* CONFIG_PM */
>> +
>>   /* User space interface to the CPU latency QoS via misc device. */
>>
>>   static int cpu_latency_qos_open(struct inode *inode, struct file *filp)
>> --
> 
> I'll look at the rest of the series whey all of my comments on this
> patch have been addressed.

Thanks a lot for the review~

> 
> Thanks!


-- 
Thx and BRs,
Zhongqiu Han

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-08-22 10:31 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21 12:40 [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Zhongqiu Han
2025-07-21 12:41 ` [PATCH v2 1/5] PM: QoS: Add support for CPU affinity latency PM QoS Zhongqiu Han
2025-08-21 10:37   ` Rafael J. Wysocki
2025-08-22 10:31     ` Zhongqiu Han
2025-07-21 12:41 ` [PATCH v2 2/5] PM: QOS: Remove unnecessary KERN_ERR on WARN() calls Zhongqiu Han
2025-07-21 12:41 ` [PATCH v2 3/5] Documentation: PM: QoS: Add CPU affinity latency PM QoS Interface documentation Zhongqiu Han
2025-07-21 12:41 ` [PATCH v2 4/5] Documentation: PM: QoS: Fix return type and return value description Zhongqiu Han
2025-08-01 11:27   ` Christian Loehle
2025-08-04 11:58     ` Zhongqiu Han
2025-07-21 12:41 ` [PATCH v2 5/5] resctrl: Replace PM QoS logic with cpu_affinity_latency_qos_* helpers Zhongqiu Han
2025-07-28 10:09 ` [PATCH v2 0/5] PM QoS: Add CPU affinity latency QoS support and resctrl integration Christian Loehle
2025-07-28 10:40   ` Zhongqiu Han
2025-08-02 14:38     ` Christian Loehle
2025-08-04 13:47       ` Zhongqiu Han
2025-08-15 12:48 ` Zhongqiu Han

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).