All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] PM QoS: per-cpu PM QoS support
@ 2014-08-13 16:01 Lina Iyer
  2014-08-13 16:01 ` [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability Lina Iyer
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-13 16:01 UTC (permalink / raw)
  To: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw; +Cc: Lina Iyer


This patchset enables drivers to set per-cpu PM QoS. PM QoS requests currently
apply to all cpus. However, drivers may be interested in only a few cpu(s) that
need to gaurantee the QoS requirement. The reason being, some requests may be
needed only to allow a quicker response to an IRQ, while requests may apply to
a certain cpu or a cluster of cpus, as dictated by an usecase. For example, in
ARM architectures, it may be benefical to set a QoS requirement to handle use
cases on big/Little cluster, while the other cluster can be in the deepest idle
state it can be.

An intended application of this feature could be the cpuidle governor. For
example, governor can now look at a CPU_DMA_LATENCY request only for that cpu
when deciding the allowable C-States. In a clustered environment, the governor
would look at the QoS request for a set of cpus before deciding the cluster's
low power mode.

With these requirements in mind, the patches do the following -

- Reorganize the PM QoS and dev-PM QoS frameworks to pass around data structures
rather than member variables. Do not want to pass plist around. It is much more
scalable, if we pass the pm_qos_request object rather than its member.

- Enhance PM QoS framework to support the per-cpu requests. This is done by
adding a per-cpu array for each constraint. This array holds the per-cpu QoS
value for each constraint. The intelligence behind the per-cpu is on the driver.
The framework just adds the request against the list held by the constraint
structure. Adding a request adds the request to the list of requests
against a constraint and also parses the array to see if this request superseeds
any existing QoS value for the cpus that this request has specified. The
workload of parsing the request is borne when updating the request.

- The driver that does an add request passes in the PM QoS request
  structure, can now pass in an additional request type that indicates if the
request is to be applied to all cores (default and existing behavior) or applies
to a subset of the cpus using a cpumask.

- To get the request value of a constraint back, you could use the
pm_qos_request() or its new cpu variants pm_qos_request_for_cpu() and
pm_qos_request_for_cpumask(). The _for_cpu() allows returns an s32 value without
locking.

- The next feature builds up on the per-cpu PM QoS feature to provide the
ability to allow a driver to tag a QoS request against an IRQ instead of a cpu.
Drivers may be concerned with an IRQ handling latency because of CPU low
power states, for example. When adding a request, they could specify the request
type to tag an IRQ and provide the IRQ numbers. When using tools like
irq-balancer, the IRQ affinity may jump from one cpu to another and in such a
case, the request needs to be modified correctly. irq/manage.c provides such
notification whenever the affinity changes. However, the current mechanism
allows for only one notification callback. 

	- Change to the smp_affinity_notifications. Modify the exisiting
	  mechanism of a single callback to support a list of requests. Drivers
	  can register for change notifications.

	- From PM QoS register for notification on behalf of a driver whenever
	  a request with IRQ is specified. When the notification arrives, modify
	  the cpu affinity of the QoS request to the cpus, that the IRQ is affine to.

- Remarks
I do not have the governor changes here at this point. Also, there is no change
to userspace at this point, i.e, userspace cannot specify a QoS request against
a cpu or specify an IRQ to tag this request against.

Thanks,
Lina


Lina Iyer (4):
  QoS: Modify data structures and function arguments for scalability.
  QoS: Enhance framework to support per-cpu PM QoS request
  irq: Allow multiple clients to register for irq affinity notification
  QoS: Enable PM QoS requests to apply only on smp_affinity of an IRQ

 Documentation/power/pm_qos_interface.txt |  18 +++
 drivers/base/power/qos.c                 |  14 +--
 drivers/infiniband/hw/qib/qib_iba7322.c  |   4 +-
 include/linux/interrupt.h                |  12 +-
 include/linux/irq.h                      |   1 +
 include/linux/irqdesc.h                  |   6 +-
 include/linux/pm_qos.h                   |  25 +++-
 kernel/irq/irqdesc.c                     |   1 +
 kernel/irq/manage.c                      |  85 +++++++++-----
 kernel/power/qos.c                       | 191 ++++++++++++++++++++++++++++++-
 lib/cpu_rmap.c                           |   2 +-
 11 files changed, 306 insertions(+), 53 deletions(-)

-- 
1.9.1


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

* [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability.
  2014-08-13 16:01 [PATCH v2 0/4] PM QoS: per-cpu PM QoS support Lina Iyer
@ 2014-08-13 16:01 ` Lina Iyer
  2014-08-18 23:38   ` Kevin Hilman
  2014-08-27 17:44   ` Kevin Hilman
  2014-08-13 16:01 ` [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request Lina Iyer
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-13 16:01 UTC (permalink / raw)
  To: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw
  Cc: Lina Iyer, Praveen Chidambaram

QoS add requests uses a handle to the priority list that is used
internally to save the request, but this does not extend well. Also,
dev_pm_qos structure definition seems to use a list object directly.
The 'derivative' relationship seems to be broken.

Use pm_qos_request objects instead of passing around the protected
priority list object.

Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/base/power/qos.c | 14 +++++++-------
 include/linux/pm_qos.h   |  7 ++++---
 kernel/power/qos.c       | 14 ++++++++------
 3 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/base/power/qos.c b/drivers/base/power/qos.c
index 36b9eb4..67a66b1 100644
--- a/drivers/base/power/qos.c
+++ b/drivers/base/power/qos.c
@@ -143,7 +143,7 @@ static int apply_constraint(struct dev_pm_qos_request *req,
 	switch(req->type) {
 	case DEV_PM_QOS_RESUME_LATENCY:
 		ret = pm_qos_update_target(&qos->resume_latency,
-					   &req->data.pnode, action, value);
+					   &req->data.lat, action, value);
 		if (ret) {
 			value = pm_qos_read_value(&qos->resume_latency);
 			blocking_notifier_call_chain(&dev_pm_notifiers,
@@ -153,7 +153,7 @@ static int apply_constraint(struct dev_pm_qos_request *req,
 		break;
 	case DEV_PM_QOS_LATENCY_TOLERANCE:
 		ret = pm_qos_update_target(&qos->latency_tolerance,
-					   &req->data.pnode, action, value);
+					   &req->data.lat, action, value);
 		if (ret) {
 			value = pm_qos_read_value(&qos->latency_tolerance);
 			req->dev->power.set_latency_tolerance(req->dev, value);
@@ -254,7 +254,7 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
 
 	/* Flush the constraints lists for the device. */
 	c = &qos->resume_latency;
-	plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
+	plist_for_each_entry_safe(req, tmp, &c->list, data.lat.node) {
 		/*
 		 * Update constraints list and call the notification
 		 * callbacks if needed
@@ -263,7 +263,7 @@ void dev_pm_qos_constraints_destroy(struct device *dev)
 		memset(req, 0, sizeof(*req));
 	}
 	c = &qos->latency_tolerance;
-	plist_for_each_entry_safe(req, tmp, &c->list, data.pnode) {
+	plist_for_each_entry_safe(req, tmp, &c->list, data.lat.node) {
 		apply_constraint(req, PM_QOS_REMOVE_REQ, PM_QOS_DEFAULT_VALUE);
 		memset(req, 0, sizeof(*req));
 	}
@@ -378,7 +378,7 @@ static int __dev_pm_qos_update_request(struct dev_pm_qos_request *req,
 	switch(req->type) {
 	case DEV_PM_QOS_RESUME_LATENCY:
 	case DEV_PM_QOS_LATENCY_TOLERANCE:
-		curr_value = req->data.pnode.prio;
+		curr_value = req->data.lat.node.prio;
 		break;
 	case DEV_PM_QOS_FLAGS:
 		curr_value = req->data.flr.flags;
@@ -831,8 +831,8 @@ s32 dev_pm_qos_get_user_latency_tolerance(struct device *dev)
 	mutex_lock(&dev_pm_qos_mtx);
 	ret = IS_ERR_OR_NULL(dev->power.qos)
 		|| !dev->power.qos->latency_tolerance_req ?
-			PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT :
-			dev->power.qos->latency_tolerance_req->data.pnode.prio;
+		PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT :
+		dev->power.qos->latency_tolerance_req->data.lat.node.prio;
 	mutex_unlock(&dev_pm_qos_mtx);
 	return ret;
 }
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 9ab4bf7..e1b763d 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -60,7 +60,7 @@ enum dev_pm_qos_req_type {
 struct dev_pm_qos_request {
 	enum dev_pm_qos_req_type type;
 	union {
-		struct plist_node pnode;
+		struct pm_qos_request lat;
 		struct pm_qos_flags_request flr;
 	} data;
 	struct device *dev;
@@ -112,7 +112,8 @@ static inline int dev_pm_qos_request_active(struct dev_pm_qos_request *req)
 	return req->dev != NULL;
 }
 
-int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node,
+int pm_qos_update_target(struct pm_qos_constraints *c,
+			 struct pm_qos_request *req,
 			 enum pm_qos_req_action action, int value);
 bool pm_qos_update_flags(struct pm_qos_flags *pqf,
 			 struct pm_qos_flags_request *req,
@@ -210,7 +211,7 @@ int dev_pm_qos_update_user_latency_tolerance(struct device *dev, s32 val);
 
 static inline s32 dev_pm_qos_requested_resume_latency(struct device *dev)
 {
-	return dev->power.qos->resume_latency_req->data.pnode.prio;
+	return dev->power.qos->resume_latency_req->data.lat.node.prio;
 }
 
 static inline s32 dev_pm_qos_requested_flags(struct device *dev)
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 884b770..d0b9c0f 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -161,19 +161,21 @@ static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
  * pm_qos_update_target - manages the constraints list and calls the notifiers
  *  if needed
  * @c: constraints data struct
- * @node: request to add to the list, to update or to remove
+ * @req: request to add to the list, to update or to remove
  * @action: action to take on the constraints list
  * @value: value of the request to add or update
  *
  * This function returns 1 if the aggregated constraint value has changed, 0
  *  otherwise.
  */
-int pm_qos_update_target(struct pm_qos_constraints *c, struct plist_node *node,
+int pm_qos_update_target(struct pm_qos_constraints *c,
+			 struct pm_qos_request *req,
 			 enum pm_qos_req_action action, int value)
 {
 	unsigned long flags;
 	int prev_value, curr_value, new_value;
 	int ret;
+	struct plist_node *node = &req->node;
 
 	spin_lock_irqsave(&pm_qos_lock, flags);
 	prev_value = pm_qos_get_value(c);
@@ -310,7 +312,7 @@ static void __pm_qos_update_request(struct pm_qos_request *req,
 	if (new_value != req->node.prio)
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
-			&req->node, PM_QOS_UPDATE_REQ, new_value);
+			req, PM_QOS_UPDATE_REQ, new_value);
 }
 
 /**
@@ -355,7 +357,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
 	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
 	trace_pm_qos_add_request(pm_qos_class, value);
 	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
-			     &req->node, PM_QOS_ADD_REQ, value);
+			     req, PM_QOS_ADD_REQ, value);
 }
 EXPORT_SYMBOL_GPL(pm_qos_add_request);
 
@@ -409,7 +411,7 @@ void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
 	if (new_value != req->node.prio)
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
-			&req->node, PM_QOS_UPDATE_REQ, new_value);
+			req, PM_QOS_UPDATE_REQ, new_value);
 
 	schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
 }
@@ -437,7 +439,7 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 
 	trace_pm_qos_remove_request(req->pm_qos_class, PM_QOS_DEFAULT_VALUE);
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
-			     &req->node, PM_QOS_REMOVE_REQ,
+			     req, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
 	memset(req, 0, sizeof(*req));
 }
-- 
1.9.1


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

* [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-13 16:01 [PATCH v2 0/4] PM QoS: per-cpu PM QoS support Lina Iyer
  2014-08-13 16:01 ` [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability Lina Iyer
@ 2014-08-13 16:01 ` Lina Iyer
  2014-08-15 12:37   ` Javi Merino
                     ` (2 more replies)
  2014-08-13 16:01 ` [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification Lina Iyer
  2014-08-13 16:01 ` [PATCH v2 4/4] QoS: Enable PM QoS requests to apply only on smp_affinity of an IRQ Lina Iyer
  3 siblings, 3 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-13 16:01 UTC (permalink / raw)
  To: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw
  Cc: Lina Iyer, Praveen Chidambaram

QoS request can be better optimized if the request can be set only for
the required cpus and not all cpus. This helps save power on other
cores, while still gauranteeing the quality of service on the desired
cores.

Add a new enumeration to specify the PM QoS request type. The enums help
specify what is the intended target cpu of the request.

Enhance the QoS constraints data structures to support target value for
each core. Requests specify if the QoS is applicable to all cores
(default) or to a selective subset of the cores or to a core(s).

Idle and interested drivers can request a PM QoS value for a constraint
across all cpus, or a specific cpu or a set of cpus. Separate APIs have
been added to request for individual cpu or a cpumask.  The default
behaviour of PM QoS is maintained i.e, requests that do not specify a
type of the request will continue to be effected on all cores.

The userspace sysfs interface does not support setting cpumask of a PM
QoS request.

Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/power/pm_qos_interface.txt |  16 +++++
 include/linux/pm_qos.h                   |  13 ++++
 kernel/power/qos.c                       | 102 +++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+)

diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
index a5da5c7..c129517 100644
--- a/Documentation/power/pm_qos_interface.txt
+++ b/Documentation/power/pm_qos_interface.txt
@@ -41,6 +41,15 @@ registered notifiers are called only if the target value is now different.
 Clients of pm_qos need to save the returned handle for future use in other
 pm_qos API functions.
 
+The handle is a pm_qos_request object. By default the request object sets the
+request type to PM_QOS_REQ_ALL_CORES, in which case, the PM QoS request
+applies to all cores. However, the driver can also specify a request type to
+be either of
+        PM_QOS_REQ_ALL_CORES,
+        PM_QOS_REQ_AFFINE_CORES,
+
+Specify the cpumask when type is set to PM_QOS_REQ_AFFINE_CORES.
+
 void pm_qos_update_request(handle, new_target_value):
 Will update the list element pointed to by the handle with the new target value
 and recompute the new aggregated target, calling the notification tree if the
@@ -54,6 +63,13 @@ the request.
 int pm_qos_request(param_class):
 Returns the aggregated value for a given PM QoS class.
 
+int pm_qos_request_for_cpu(param_class, cpu):
+Returns the aggregated value for a given PM QoS class for the specified cpu.
+
+int pm_qos_request_for_cpumask(param_class, cpumask):
+Returns the aggregated value for a given PM QoS class for the specified
+cpumask.
+
 int pm_qos_request_active(handle):
 Returns if the request is still active, i.e. it has not been removed from a
 PM QoS class constraints list.
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index e1b763d..a3aa5b5 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -9,6 +9,8 @@
 #include <linux/miscdevice.h>
 #include <linux/device.h>
 #include <linux/workqueue.h>
+#include <linux/cpumask.h>
+#include <linux/interrupt.h>
 
 enum {
 	PM_QOS_RESERVED = 0,
@@ -40,7 +42,15 @@ enum pm_qos_flags_status {
 #define PM_QOS_FLAG_NO_POWER_OFF	(1 << 0)
 #define PM_QOS_FLAG_REMOTE_WAKEUP	(1 << 1)
 
+enum pm_qos_req_type {
+	PM_QOS_REQ_ALL_CORES = 0,
+	PM_QOS_REQ_AFFINE_CORES,
+};
+
 struct pm_qos_request {
+	enum pm_qos_req_type type;
+	struct cpumask cpus_affine;
+	/* Internal structure members */
 	struct plist_node node;
 	int pm_qos_class;
 	struct delayed_work work; /* for pm_qos_update_request_timeout */
@@ -80,6 +90,7 @@ enum pm_qos_type {
 struct pm_qos_constraints {
 	struct plist_head list;
 	s32 target_value;	/* Do not change to 64 bit */
+	s32 target_per_cpu[NR_CPUS];
 	s32 default_value;
 	s32 no_constraint_value;
 	enum pm_qos_type type;
@@ -127,6 +138,8 @@ void pm_qos_update_request_timeout(struct pm_qos_request *req,
 void pm_qos_remove_request(struct pm_qos_request *req);
 
 int pm_qos_request(int pm_qos_class);
+int pm_qos_request_for_cpu(int pm_qos_class, int cpu);
+int pm_qos_request_for_cpumask(int pm_qos_class, struct cpumask *mask);
 int pm_qos_add_notifier(int pm_qos_class, struct notifier_block *notifier);
 int pm_qos_remove_notifier(int pm_qos_class, struct notifier_block *notifier);
 int pm_qos_request_active(struct pm_qos_request *req);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index d0b9c0f..27f84a2 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -65,6 +65,8 @@ static BLOCKING_NOTIFIER_HEAD(cpu_dma_lat_notifier);
 static struct pm_qos_constraints cpu_dma_constraints = {
 	.list = PLIST_HEAD_INIT(cpu_dma_constraints.list),
 	.target_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
+	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
+				PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE },
 	.default_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
 	.no_constraint_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
 	.type = PM_QOS_MIN,
@@ -79,6 +81,8 @@ static BLOCKING_NOTIFIER_HEAD(network_lat_notifier);
 static struct pm_qos_constraints network_lat_constraints = {
 	.list = PLIST_HEAD_INIT(network_lat_constraints.list),
 	.target_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
+	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
+				PM_QOS_NETWORK_LAT_DEFAULT_VALUE },
 	.default_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
 	.no_constraint_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
 	.type = PM_QOS_MIN,
@@ -94,6 +98,8 @@ static BLOCKING_NOTIFIER_HEAD(network_throughput_notifier);
 static struct pm_qos_constraints network_tput_constraints = {
 	.list = PLIST_HEAD_INIT(network_tput_constraints.list),
 	.target_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
+	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
+				PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE },
 	.default_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
 	.no_constraint_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
 	.type = PM_QOS_MAX,
@@ -157,6 +163,43 @@ static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
 	c->target_value = value;
 }
 
+static inline void pm_qos_set_value_for_cpus(struct pm_qos_constraints *c)
+{
+	struct pm_qos_request *req = NULL;
+	int cpu;
+	s32 *qos_val;
+
+	qos_val = kcalloc(NR_CPUS, sizeof(*qos_val), GFP_KERNEL);
+	if (!qos_val) {
+		WARN("%s: No memory for PM QoS\n", __func__);
+		return;
+	}
+
+	for_each_possible_cpu(cpu)
+		qos_val[cpu] = c->default_value;
+
+	plist_for_each_entry(req, &c->list, node) {
+		for_each_cpu(cpu, &req->cpus_affine) {
+			switch (c->type) {
+			case PM_QOS_MIN:
+				if (qos_val[cpu] > req->node.prio)
+					qos_val[cpu] = req->node.prio;
+				break;
+			case PM_QOS_MAX:
+				if (req->node.prio > qos_val[cpu])
+					qos_val[cpu] = req->node.prio;
+				break;
+			default:
+				BUG();
+				break;
+			}
+		}
+	}
+
+	for_each_possible_cpu(cpu)
+		c->target_per_cpu[cpu] = qos_val[cpu];
+}
+
 /**
  * pm_qos_update_target - manages the constraints list and calls the notifiers
  *  if needed
@@ -206,6 +249,7 @@ int pm_qos_update_target(struct pm_qos_constraints *c,
 
 	curr_value = pm_qos_get_value(c);
 	pm_qos_set_value(c, curr_value);
+	pm_qos_set_value_for_cpus(c);
 
 	spin_unlock_irqrestore(&pm_qos_lock, flags);
 
@@ -298,6 +342,44 @@ int pm_qos_request(int pm_qos_class)
 }
 EXPORT_SYMBOL_GPL(pm_qos_request);
 
+int pm_qos_request_for_cpu(int pm_qos_class, int cpu)
+{
+	return pm_qos_array[pm_qos_class]->constraints->target_per_cpu[cpu];
+}
+EXPORT_SYMBOL(pm_qos_request_for_cpu);
+
+int pm_qos_request_for_cpumask(int pm_qos_class, struct cpumask *mask)
+{
+	unsigned long irqflags;
+	int cpu;
+	struct pm_qos_constraints *c = NULL;
+	int val;
+
+	spin_lock_irqsave(&pm_qos_lock, irqflags);
+	c = pm_qos_array[pm_qos_class]->constraints;
+	val = c->default_value;
+
+	for_each_cpu(cpu, mask) {
+		switch (c->type) {
+		case PM_QOS_MIN:
+			if (c->target_per_cpu[cpu] < val)
+				val = c->target_per_cpu[cpu];
+			break;
+		case PM_QOS_MAX:
+			if (c->target_per_cpu[cpu] > val)
+				val = c->target_per_cpu[cpu];
+			break;
+		default:
+			BUG();
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&pm_qos_lock, irqflags);
+
+	return val;
+}
+EXPORT_SYMBOL(pm_qos_request_for_cpumask);
+
 int pm_qos_request_active(struct pm_qos_request *req)
 {
 	return req->pm_qos_class != 0;
@@ -353,6 +435,24 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		WARN(1, KERN_ERR "pm_qos_add_request() called for already added request\n");
 		return;
 	}
+
+	switch (req->type) {
+	case PM_QOS_REQ_AFFINE_CORES:
+		if (cpumask_empty(&req->cpus_affine)) {
+			req->type = PM_QOS_REQ_ALL_CORES;
+			cpumask_setall(&req->cpus_affine);
+			WARN(1, KERN_ERR "Affine cores not set for request with affinity flag\n");
+		}
+		break;
+
+	default:
+		WARN(1, KERN_ERR "Unknown request type %d\n", req->type);
+		/* fall through */
+	case PM_QOS_REQ_ALL_CORES:
+		cpumask_setall(&req->cpus_affine);
+		break;
+	}
+
 	req->pm_qos_class = pm_qos_class;
 	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
 	trace_pm_qos_add_request(pm_qos_class, value);
@@ -426,6 +526,7 @@ void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
  */
 void pm_qos_remove_request(struct pm_qos_request *req)
 {
+
 	if (!req) /*guard against callers passing in null */
 		return;
 		/* silent return to keep pcm code cleaner */
@@ -441,6 +542,7 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
 			     req, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
+
 	memset(req, 0, sizeof(*req));
 }
 EXPORT_SYMBOL_GPL(pm_qos_remove_request);
-- 
1.9.1


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

* [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification
  2014-08-13 16:01 [PATCH v2 0/4] PM QoS: per-cpu PM QoS support Lina Iyer
  2014-08-13 16:01 ` [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability Lina Iyer
  2014-08-13 16:01 ` [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request Lina Iyer
@ 2014-08-13 16:01 ` Lina Iyer
  2014-08-19  0:04   ` Kevin Hilman
  2014-08-13 16:01 ` [PATCH v2 4/4] QoS: Enable PM QoS requests to apply only on smp_affinity of an IRQ Lina Iyer
  3 siblings, 1 reply; 15+ messages in thread
From: Lina Iyer @ 2014-08-13 16:01 UTC (permalink / raw)
  To: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw; +Cc: Lina Iyer

The current implementation allows only a single notification callback
whenever the IRQ's SMP affinity is changed. Adding a second notification
punts the existing notifier function out of registration.

Add a list of notifiers, allowing multiple clients to register for irq
affinity notifications.

Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 drivers/infiniband/hw/qib/qib_iba7322.c |  4 +-
 include/linux/interrupt.h               | 12 ++++-
 include/linux/irq.h                     |  1 +
 include/linux/irqdesc.h                 |  6 ++-
 kernel/irq/irqdesc.c                    |  1 +
 kernel/irq/manage.c                     | 85 ++++++++++++++++++++++-----------
 lib/cpu_rmap.c                          |  2 +-
 7 files changed, 74 insertions(+), 37 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_iba7322.c b/drivers/infiniband/hw/qib/qib_iba7322.c
index a7eb325..62cb77d 100644
--- a/drivers/infiniband/hw/qib/qib_iba7322.c
+++ b/drivers/infiniband/hw/qib/qib_iba7322.c
@@ -3345,9 +3345,7 @@ static void reset_dca_notifier(struct qib_devdata *dd, struct qib_msix_entry *m)
 		"Disabling notifier on HCA %d irq %d\n",
 		dd->unit,
 		m->msix.vector);
-	irq_set_affinity_notifier(
-		m->msix.vector,
-		NULL);
+	irq_release_affinity_notifier(m->notifier);
 	m->notifier = NULL;
 }
 
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 698ad05..c1e227c 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -203,7 +203,7 @@ static inline int check_wakeup_irqs(void) { return 0; }
  * struct irq_affinity_notify - context for notification of IRQ affinity changes
  * @irq:		Interrupt to which notification applies
  * @kref:		Reference count, for internal use
- * @work:		Work item, for internal use
+ * @list:		Add to the notifier list, for internal use
  * @notify:		Function to be called on change.  This will be
  *			called in process context.
  * @release:		Function to be called on release.  This will be
@@ -214,7 +214,7 @@ static inline int check_wakeup_irqs(void) { return 0; }
 struct irq_affinity_notify {
 	unsigned int irq;
 	struct kref kref;
-	struct work_struct work;
+	struct list_head list;
 	void (*notify)(struct irq_affinity_notify *, const cpumask_t *mask);
 	void (*release)(struct kref *ref);
 };
@@ -265,6 +265,8 @@ extern int irq_set_affinity_hint(unsigned int irq, const struct cpumask *m);
 extern int
 irq_set_affinity_notifier(unsigned int irq, struct irq_affinity_notify *notify);
 
+extern int
+irq_release_affinity_notifier(struct irq_affinity_notify *notify);
 #else /* CONFIG_SMP */
 
 static inline int irq_set_affinity(unsigned int irq, const struct cpumask *m)
@@ -295,6 +297,12 @@ irq_set_affinity_notifier(unsigned int irq, struct irq_affinity_notify *notify)
 {
 	return 0;
 }
+
+static inline int
+irq_release_affinity_notifier(struct irq_affinity_notify *notify)
+{
+	return 0;
+}
 #endif /* CONFIG_SMP */
 
 /*
diff --git a/include/linux/irq.h b/include/linux/irq.h
index 62af592..2634a48 100644
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -20,6 +20,7 @@
 #include <linux/errno.h>
 #include <linux/topology.h>
 #include <linux/wait.h>
+#include <linux/list.h>
 
 #include <asm/irq.h>
 #include <asm/ptrace.h>
diff --git a/include/linux/irqdesc.h b/include/linux/irqdesc.h
index 472c021..db3509e 100644
--- a/include/linux/irqdesc.h
+++ b/include/linux/irqdesc.h
@@ -31,7 +31,8 @@ struct irq_desc;
  * @threads_handled_last: comparator field for deferred spurious detection of theraded handlers
  * @lock:		locking for SMP
  * @affinity_hint:	hint to user space for preferred irq affinity
- * @affinity_notify:	context for notification of affinity changes
+ * @affinity_notify:	list of notification clients for affinity changes
+ * @affinity_work:	Work queue for handling affinity change notifications
  * @pending_mask:	pending rebalanced interrupts
  * @threads_oneshot:	bitfield to handle shared oneshot threads
  * @threads_active:	number of irqaction threads currently running
@@ -60,7 +61,8 @@ struct irq_desc {
 	struct cpumask		*percpu_enabled;
 #ifdef CONFIG_SMP
 	const struct cpumask	*affinity_hint;
-	struct irq_affinity_notify *affinity_notify;
+	struct list_head	affinity_notify;
+	struct work_struct	affinity_work;
 #ifdef CONFIG_GENERIC_PENDING_IRQ
 	cpumask_var_t		pending_mask;
 #endif
diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 1487a12..c95e1f3 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -91,6 +91,7 @@ static void desc_set_defaults(unsigned int irq, struct irq_desc *desc, int node,
 	for_each_possible_cpu(cpu)
 		*per_cpu_ptr(desc->kstat_irqs, cpu) = 0;
 	desc_smp_init(desc, node);
+	INIT_LIST_HEAD(&desc->affinity_notify);
 }
 
 int nr_irqs = NR_IRQS;
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 88657d7..99fc0e7 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -209,10 +209,9 @@ int irq_set_affinity_locked(struct irq_data *data, const struct cpumask *mask,
 		irq_copy_pending(desc, mask);
 	}
 
-	if (desc->affinity_notify) {
-		kref_get(&desc->affinity_notify->kref);
-		schedule_work(&desc->affinity_notify->work);
-	}
+	if (!list_empty(&desc->affinity_notify))
+		schedule_work(&desc->affinity_work);
+
 	irqd_set(data, IRQD_AFFINITY_SET);
 
 	return ret;
@@ -248,14 +247,14 @@ EXPORT_SYMBOL_GPL(irq_set_affinity_hint);
 
 static void irq_affinity_notify(struct work_struct *work)
 {
-	struct irq_affinity_notify *notify =
-		container_of(work, struct irq_affinity_notify, work);
-	struct irq_desc *desc = irq_to_desc(notify->irq);
+	struct irq_desc *desc =
+			container_of(work, struct irq_desc, affinity_work);
 	cpumask_var_t cpumask;
 	unsigned long flags;
+	struct irq_affinity_notify *notify;
 
 	if (!desc || !alloc_cpumask_var(&cpumask, GFP_KERNEL))
-		goto out;
+		return;
 
 	raw_spin_lock_irqsave(&desc->lock, flags);
 	if (irq_move_pending(&desc->irq_data))
@@ -264,11 +263,20 @@ static void irq_affinity_notify(struct work_struct *work)
 		cpumask_copy(cpumask, desc->irq_data.affinity);
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
 
-	notify->notify(notify, cpumask);
+	list_for_each_entry(notify, &desc->affinity_notify, list) {
+		/**
+		 * Check and get the kref only if the kref has not been
+		 * released by now. Its possible that the reference count
+		 * is already 0, we dont want to notify those if they are
+		 * already released.
+		 */
+		if (!kref_get_unless_zero(&notify->kref))
+			continue;
+		notify->notify(notify, cpumask);
+		kref_put(&notify->kref, notify->release);
+	}
 
 	free_cpumask_var(cpumask);
-out:
-	kref_put(&notify->kref, notify->release);
 }
 
 /**
@@ -286,34 +294,50 @@ int
 irq_set_affinity_notifier(unsigned int irq, struct irq_affinity_notify *notify)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
-	struct irq_affinity_notify *old_notify;
 	unsigned long flags;
 
-	/* The release function is promised process context */
-	might_sleep();
-
 	if (!desc)
 		return -EINVAL;
 
-	/* Complete initialisation of *notify */
-	if (notify) {
-		notify->irq = irq;
-		kref_init(&notify->kref);
-		INIT_WORK(&notify->work, irq_affinity_notify);
+	if (!notify) {
+		WARN("%s called with NULL notifier - use irq_release_affinity_notifier function instead.\n",
+				__func__);
+		return -EINVAL;
 	}
 
+	notify->irq = irq;
+	kref_init(&notify->kref);
+	INIT_LIST_HEAD(&notify->list);
 	raw_spin_lock_irqsave(&desc->lock, flags);
-	old_notify = desc->affinity_notify;
-	desc->affinity_notify = notify;
+	list_add(&notify->list, &desc->affinity_notify);
 	raw_spin_unlock_irqrestore(&desc->lock, flags);
 
-	if (old_notify)
-		kref_put(&old_notify->kref, old_notify->release);
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(irq_set_affinity_notifier);
 
+/**
+ *	irq_release_affinity_notifier - Remove us from notifications
+ *	@notify: Context for notification
+ */
+int irq_release_affinity_notifier(struct irq_affinity_notify *notify)
+{
+	struct irq_desc *desc;
+	unsigned long flags;
+
+	if (!notify)
+		return -EINVAL;
+
+	desc = irq_to_desc(notify->irq);
+	raw_spin_lock_irqsave(&desc->lock, flags);
+	list_del(&notify->list);
+	raw_spin_unlock_irqrestore(&desc->lock, flags);
+	kref_put(&notify->kref, notify->release);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(irq_release_affinity_notifier);
+
 #ifndef CONFIG_AUTO_IRQ_AFFINITY
 /*
  * Generic version of the affinity autoselector.
@@ -348,6 +372,8 @@ setup_affinity(unsigned int irq, struct irq_desc *desc, struct cpumask *mask)
 		if (cpumask_intersects(mask, nodemask))
 			cpumask_and(mask, mask, nodemask);
 	}
+	INIT_LIST_HEAD(&desc->affinity_notify);
+	INIT_WORK(&desc->affinity_work, irq_affinity_notify);
 	irq_do_set_affinity(&desc->irq_data, mask, false);
 	return 0;
 }
@@ -1414,14 +1440,15 @@ EXPORT_SYMBOL_GPL(remove_irq);
 void free_irq(unsigned int irq, void *dev_id)
 {
 	struct irq_desc *desc = irq_to_desc(irq);
+	struct irq_affinity_notify *notify;
 
 	if (!desc || WARN_ON(irq_settings_is_per_cpu_devid(desc)))
 		return;
 
-#ifdef CONFIG_SMP
-	if (WARN_ON(desc->affinity_notify))
-		desc->affinity_notify = NULL;
-#endif
+	WARN_ON(!list_empty(&desc->affinity_notify));
+
+	list_for_each_entry(notify, &desc->affinity_notify, list)
+		kref_put(&notify->kref, notify->release);
 
 	chip_bus_lock(desc);
 	kfree(__free_irq(irq, dev_id));
diff --git a/lib/cpu_rmap.c b/lib/cpu_rmap.c
index 4f134d8..0c8da50 100644
--- a/lib/cpu_rmap.c
+++ b/lib/cpu_rmap.c
@@ -235,7 +235,7 @@ void free_irq_cpu_rmap(struct cpu_rmap *rmap)
 
 	for (index = 0; index < rmap->used; index++) {
 		glue = rmap->obj[index];
-		irq_set_affinity_notifier(glue->notify.irq, NULL);
+		irq_release_affinity_notifier(&glue->notify);
 	}
 
 	cpu_rmap_put(rmap);
-- 
1.9.1


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

* [PATCH v2 4/4] QoS: Enable PM QoS requests to apply only on smp_affinity of an IRQ
  2014-08-13 16:01 [PATCH v2 0/4] PM QoS: per-cpu PM QoS support Lina Iyer
                   ` (2 preceding siblings ...)
  2014-08-13 16:01 ` [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification Lina Iyer
@ 2014-08-13 16:01 ` Lina Iyer
  3 siblings, 0 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-13 16:01 UTC (permalink / raw)
  To: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw
  Cc: Lina Iyer, Praveen Chidambaram

QoS requests that need to track an IRQ can be set to apply only on the
cpus to which the IRQ's smp_affinity attribute is set to. The PM QoS
framework will automatically track IRQ migration between the cores. The
QoS is updated to be applied only to the core(s) that the IRQ has been
migrated to.

The userspace sysfs interface does not support IRQ affinity.

Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
---
 Documentation/power/pm_qos_interface.txt |  4 +-
 include/linux/pm_qos.h                   |  5 +++
 kernel/power/qos.c                       | 77 +++++++++++++++++++++++++++++++-
 3 files changed, 84 insertions(+), 2 deletions(-)

diff --git a/Documentation/power/pm_qos_interface.txt b/Documentation/power/pm_qos_interface.txt
index c129517..32b864d 100644
--- a/Documentation/power/pm_qos_interface.txt
+++ b/Documentation/power/pm_qos_interface.txt
@@ -47,8 +47,10 @@ applies to all cores. However, the driver can also specify a request type to
 be either of
         PM_QOS_REQ_ALL_CORES,
         PM_QOS_REQ_AFFINE_CORES,
+        PM_QOS_REQ_AFFINE_IRQ,
 
-Specify the cpumask when type is set to PM_QOS_REQ_AFFINE_CORES.
+Specify the cpumask when type is set to PM_QOS_REQ_AFFINE_CORES and specify
+the IRQ number with PM_QOS_REQ_AFFINE_IRQ.
 
 void pm_qos_update_request(handle, new_target_value):
 Will update the list element pointed to by the handle with the new target value
diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index a3aa5b5..68b16b8 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -11,6 +11,7 @@
 #include <linux/workqueue.h>
 #include <linux/cpumask.h>
 #include <linux/interrupt.h>
+#include <linux/completion.h>
 
 enum {
 	PM_QOS_RESERVED = 0,
@@ -45,12 +46,16 @@ enum pm_qos_flags_status {
 enum pm_qos_req_type {
 	PM_QOS_REQ_ALL_CORES = 0,
 	PM_QOS_REQ_AFFINE_CORES,
+	PM_QOS_REQ_AFFINE_IRQ,
 };
 
 struct pm_qos_request {
 	enum pm_qos_req_type type;
 	struct cpumask cpus_affine;
+	uint32_t irq;
 	/* Internal structure members */
+	struct irq_affinity_notify irq_notify;
+	struct completion irq_released;
 	struct plist_node node;
 	int pm_qos_class;
 	struct delayed_work work; /* for pm_qos_update_request_timeout */
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 27f84a2..c10e8bc 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -41,6 +41,9 @@
 #include <linux/platform_device.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
+#include <linux/delay.h>
 
 #include <linux/uaccess.h>
 #include <linux/export.h>
@@ -412,6 +415,37 @@ static void pm_qos_work_fn(struct work_struct *work)
 	__pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
 }
 
+static void pm_qos_irq_release(struct kref *ref)
+{
+	unsigned long flags;
+	struct irq_affinity_notify *notify = container_of(ref,
+			struct irq_affinity_notify, kref);
+	struct pm_qos_request *req = container_of(notify,
+			struct pm_qos_request, irq_notify);
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	cpumask_clear(&req->cpus_affine);
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+
+	complete(&req->irq_released);
+}
+
+static void pm_qos_irq_notify(struct irq_affinity_notify *notify,
+		const cpumask_t *mask)
+{
+	unsigned long flags;
+	struct pm_qos_request *req = container_of(notify,
+			struct pm_qos_request, irq_notify);
+	struct pm_qos_constraints *c =
+		pm_qos_array[req->pm_qos_class]->constraints;
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	cpumask_copy(&req->cpus_affine, mask);
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+
+	pm_qos_update_target(c, req, PM_QOS_UPDATE_REQ, req->node.prio);
+}
+
 /**
  * pm_qos_add_request - inserts new qos request into the list
  * @req: pointer to a preallocated handle
@@ -445,6 +479,34 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		}
 		break;
 
+	case PM_QOS_REQ_AFFINE_IRQ:
+		if (irq_can_set_affinity(req->irq)) {
+			int ret = 0;
+			struct irq_desc *desc = irq_to_desc(req->irq);
+			struct cpumask *mask = desc->irq_data.affinity;
+
+			/* Get the current affinity */
+			cpumask_copy(&req->cpus_affine, mask);
+			req->irq_notify.irq = req->irq;
+			req->irq_notify.notify = pm_qos_irq_notify;
+			req->irq_notify.release = pm_qos_irq_release;
+
+			ret = irq_set_affinity_notifier(req->irq,
+					&req->irq_notify);
+			if (ret) {
+				WARN(1, KERN_ERR "IRQ affinity notify set failed\n");
+				req->type = PM_QOS_REQ_ALL_CORES;
+				cpumask_setall(&req->cpus_affine);
+			}
+		} else {
+			req->type = PM_QOS_REQ_ALL_CORES;
+			cpumask_setall(&req->cpus_affine);
+			WARN(1, KERN_ERR "IRQ-%d not set for request with affinity flag\n",
+					req->irq);
+		}
+		init_completion(&req->irq_released);
+		break;
+
 	default:
 		WARN(1, KERN_ERR "Unknown request type %d\n", req->type);
 		/* fall through */
@@ -526,11 +588,14 @@ void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
  */
 void pm_qos_remove_request(struct pm_qos_request *req)
 {
-
 	if (!req) /*guard against callers passing in null */
 		return;
 		/* silent return to keep pcm code cleaner */
 
+	/* Remove ourselves from the irq notification */
+	if (req->type == PM_QOS_REQ_AFFINE_IRQ)
+		irq_release_affinity_notifier(&req->irq_notify);
+
 	if (!pm_qos_request_active(req)) {
 		WARN(1, KERN_ERR "pm_qos_remove_request() called for unknown object\n");
 		return;
@@ -543,6 +608,16 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 			     req, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
 
+	/**
+	 * The 'release' callback of the notifier would not be called unless
+	 * there are no active users of the irq_notify object, i.e, kref count
+	 * is non-zero. This could happen if there is an active 'notify'
+	 * callback happening while the pm_qos_remove request is called. Wait
+	 * until the release callback clears the cpus_affine mask.
+	 */
+	if (req->type == PM_QOS_REQ_AFFINE_IRQ)
+		wait_for_completion(&req->irq_released);
+
 	memset(req, 0, sizeof(*req));
 }
 EXPORT_SYMBOL_GPL(pm_qos_remove_request);
-- 
1.9.1


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

* Re: [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-13 16:01 ` [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request Lina Iyer
@ 2014-08-15 12:37   ` Javi Merino
  2014-08-15 15:06     ` Lina Iyer
  2014-08-18 23:55   ` Kevin Hilman
  2014-08-27 18:01   ` Kevin Hilman
  2 siblings, 1 reply; 15+ messages in thread
From: Javi Merino @ 2014-08-15 12:37 UTC (permalink / raw)
  To: Lina Iyer
  Cc: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw,
	Praveen Chidambaram

Hi Lina, some minor nits,

On Wed, Aug 13, 2014 at 05:01:27PM +0100, Lina Iyer wrote:
> QoS request can be better optimized if the request can be set only for
> the required cpus and not all cpus. This helps save power on other
> cores, while still gauranteeing the quality of service on the desired

                     guaranteeing

> cores.
> 
> Add a new enumeration to specify the PM QoS request type. The enums help
> specify what is the intended target cpu of the request.
> 
> Enhance the QoS constraints data structures to support target value for
> each core. Requests specify if the QoS is applicable to all cores
> (default) or to a selective subset of the cores or to a core(s).
> 
> Idle and interested drivers can request a PM QoS value for a constraint
> across all cpus, or a specific cpu or a set of cpus. Separate APIs have
> been added to request for individual cpu or a cpumask.  The default
> behaviour of PM QoS is maintained i.e, requests that do not specify a
> type of the request will continue to be effected on all cores.
> 
> The userspace sysfs interface does not support setting cpumask of a PM
> QoS request.
> 
> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
> ---
>  Documentation/power/pm_qos_interface.txt |  16 +++++
>  include/linux/pm_qos.h                   |  13 ++++
>  kernel/power/qos.c                       | 102 +++++++++++++++++++++++++++++++
>  3 files changed, 131 insertions(+)
> 
[...]
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index d0b9c0f..27f84a2 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -65,6 +65,8 @@ static BLOCKING_NOTIFIER_HEAD(cpu_dma_lat_notifier);
>  static struct pm_qos_constraints cpu_dma_constraints = {
>  	.list = PLIST_HEAD_INIT(cpu_dma_constraints.list),
>  	.target_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
> +	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
> +				PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE },
>  	.default_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
>  	.no_constraint_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
>  	.type = PM_QOS_MIN,
> @@ -79,6 +81,8 @@ static BLOCKING_NOTIFIER_HEAD(network_lat_notifier);
>  static struct pm_qos_constraints network_lat_constraints = {
>  	.list = PLIST_HEAD_INIT(network_lat_constraints.list),
>  	.target_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
> +	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
> +				PM_QOS_NETWORK_LAT_DEFAULT_VALUE },
>  	.default_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
>  	.no_constraint_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
>  	.type = PM_QOS_MIN,
> @@ -94,6 +98,8 @@ static BLOCKING_NOTIFIER_HEAD(network_throughput_notifier);
>  static struct pm_qos_constraints network_tput_constraints = {
>  	.list = PLIST_HEAD_INIT(network_tput_constraints.list),
>  	.target_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
> +	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
> +				PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE },
>  	.default_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
>  	.no_constraint_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
>  	.type = PM_QOS_MAX,
> @@ -157,6 +163,43 @@ static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
>  	c->target_value = value;
>  }
>  
> +static inline void pm_qos_set_value_for_cpus(struct pm_qos_constraints *c)
> +{
> +	struct pm_qos_request *req = NULL;
> +	int cpu;
> +	s32 *qos_val;
> +
> +	qos_val = kcalloc(NR_CPUS, sizeof(*qos_val), GFP_KERNEL);
> +	if (!qos_val) {
> +		WARN("%s: No memory for PM QoS\n", __func__);
> +		return;
> +	}
> +
> +	for_each_possible_cpu(cpu)
> +		qos_val[cpu] = c->default_value;
> +
> +	plist_for_each_entry(req, &c->list, node) {
> +		for_each_cpu(cpu, &req->cpus_affine) {
> +			switch (c->type) {
> +			case PM_QOS_MIN:
> +				if (qos_val[cpu] > req->node.prio)
> +					qos_val[cpu] = req->node.prio;
> +				break;
> +			case PM_QOS_MAX:
> +				if (req->node.prio > qos_val[cpu])
> +					qos_val[cpu] = req->node.prio;
> +				break;
> +			default:
> +				BUG();
> +				break;
> +			}
> +		}
> +	}
> +
> +	for_each_possible_cpu(cpu)
> +		c->target_per_cpu[cpu] = qos_val[cpu];
> +}
> +
>  /**
>   * pm_qos_update_target - manages the constraints list and calls the notifiers
>   *  if needed
> @@ -206,6 +249,7 @@ int pm_qos_update_target(struct pm_qos_constraints *c,
>  
>  	curr_value = pm_qos_get_value(c);
>  	pm_qos_set_value(c, curr_value);
> +	pm_qos_set_value_for_cpus(c);
>  
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);
>  
> @@ -298,6 +342,44 @@ int pm_qos_request(int pm_qos_class)
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_request);
>  
> +int pm_qos_request_for_cpu(int pm_qos_class, int cpu)
> +{
> +	return pm_qos_array[pm_qos_class]->constraints->target_per_cpu[cpu];
> +}
> +EXPORT_SYMBOL(pm_qos_request_for_cpu);
> +
> +int pm_qos_request_for_cpumask(int pm_qos_class, struct cpumask *mask)
> +{
> +	unsigned long irqflags;
> +	int cpu;
> +	struct pm_qos_constraints *c = NULL;
> +	int val;
> +
> +	spin_lock_irqsave(&pm_qos_lock, irqflags);
> +	c = pm_qos_array[pm_qos_class]->constraints;
> +	val = c->default_value;
> +
> +	for_each_cpu(cpu, mask) {
> +		switch (c->type) {
> +		case PM_QOS_MIN:
> +			if (c->target_per_cpu[cpu] < val)
> +				val = c->target_per_cpu[cpu];
> +			break;
> +		case PM_QOS_MAX:
> +			if (c->target_per_cpu[cpu] > val)
> +				val = c->target_per_cpu[cpu];
> +			break;
> +		default:
> +			BUG();
> +			break;
> +		}
> +	}
> +	spin_unlock_irqrestore(&pm_qos_lock, irqflags);
> +
> +	return val;
> +}
> +EXPORT_SYMBOL(pm_qos_request_for_cpumask);
> +
>  int pm_qos_request_active(struct pm_qos_request *req)
>  {
>  	return req->pm_qos_class != 0;
> @@ -353,6 +435,24 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		WARN(1, KERN_ERR "pm_qos_add_request() called for already added request\n");
>  		return;
>  	}
> +
> +	switch (req->type) {
> +	case PM_QOS_REQ_AFFINE_CORES:
> +		if (cpumask_empty(&req->cpus_affine)) {
> +			req->type = PM_QOS_REQ_ALL_CORES;
> +			cpumask_setall(&req->cpus_affine);
> +			WARN(1, KERN_ERR "Affine cores not set for request with affinity flag\n");
> +		}
> +		break;
> +
> +	default:
> +		WARN(1, KERN_ERR "Unknown request type %d\n", req->type);
> +		/* fall through */
> +	case PM_QOS_REQ_ALL_CORES:
> +		cpumask_setall(&req->cpus_affine);
> +		break;
> +	}
> +
>  	req->pm_qos_class = pm_qos_class;
>  	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>  	trace_pm_qos_add_request(pm_qos_class, value);
> @@ -426,6 +526,7 @@ void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
>   */
>  void pm_qos_remove_request(struct pm_qos_request *req)
>  {
> +

Unnecessary newline added.

>  	if (!req) /*guard against callers passing in null */
>  		return;
>  		/* silent return to keep pcm code cleaner */
> @@ -441,6 +542,7 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     req, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
> +

ditto.  Cheers,
Javi

>  	memset(req, 0, sizeof(*req));
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_remove_request);
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


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

* Re: [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-15 12:37   ` Javi Merino
@ 2014-08-15 15:06     ` Lina Iyer
  0 siblings, 0 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-15 15:06 UTC (permalink / raw)
  To: Javi Merino
  Cc: daniel.lezcano, khilman, ulf.hansson, linux-pm, tglx, rjw,
	Praveen Chidambaram

On Fri, Aug 15, 2014 at 01:37:32PM +0100, Javi Merino wrote:
>Hi Lina, some minor nits,
>
>On Wed, Aug 13, 2014 at 05:01:27PM +0100, Lina Iyer wrote:
>> QoS request can be better optimized if the request can be set only for
>> the required cpus and not all cpus. This helps save power on other
>> cores, while still gauranteeing the quality of service on the desired
>
>                     guaranteeing
>
I never get that right :[
Will take care of it.
>> cores.
>>
>> Add a new enumeration to specify the PM QoS request type. The enums help
>> specify what is the intended target cpu of the request.
>>
>> Enhance the QoS constraints data structures to support target value for
>> each core. Requests specify if the QoS is applicable to all cores
>> (default) or to a selective subset of the cores or to a core(s).
>>
>> Idle and interested drivers can request a PM QoS value for a constraint
>> across all cpus, or a specific cpu or a set of cpus. Separate APIs have
>> been added to request for individual cpu or a cpumask.  The default
>> behaviour of PM QoS is maintained i.e, requests that do not specify a
>> type of the request will continue to be effected on all cores.
>>
>> The userspace sysfs interface does not support setting cpumask of a PM
>> QoS request.
>>
>> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>> ---
>>  Documentation/power/pm_qos_interface.txt |  16 +++++
>>  include/linux/pm_qos.h                   |  13 ++++
>>  kernel/power/qos.c                       | 102 +++++++++++++++++++++++++++++++
>>  3 files changed, 131 insertions(+)
>>
>[...]
>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>> index d0b9c0f..27f84a2 100644
>> --- a/kernel/power/qos.c
>> +++ b/kernel/power/qos.c
>> @@ -65,6 +65,8 @@ static BLOCKING_NOTIFIER_HEAD(cpu_dma_lat_notifier);
>>  static struct pm_qos_constraints cpu_dma_constraints = {
>>  	.list = PLIST_HEAD_INIT(cpu_dma_constraints.list),
>>  	.target_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
>> +	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
>> +				PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE },
>>  	.default_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
>>  	.no_constraint_value = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE,
>>  	.type = PM_QOS_MIN,
>> @@ -79,6 +81,8 @@ static BLOCKING_NOTIFIER_HEAD(network_lat_notifier);
>>  static struct pm_qos_constraints network_lat_constraints = {
>>  	.list = PLIST_HEAD_INIT(network_lat_constraints.list),
>>  	.target_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
>> +	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
>> +				PM_QOS_NETWORK_LAT_DEFAULT_VALUE },
>>  	.default_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
>>  	.no_constraint_value = PM_QOS_NETWORK_LAT_DEFAULT_VALUE,
>>  	.type = PM_QOS_MIN,
>> @@ -94,6 +98,8 @@ static BLOCKING_NOTIFIER_HEAD(network_throughput_notifier);
>>  static struct pm_qos_constraints network_tput_constraints = {
>>  	.list = PLIST_HEAD_INIT(network_tput_constraints.list),
>>  	.target_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
>> +	.target_per_cpu = { [0 ... (NR_CPUS - 1)] =
>> +				PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE },
>>  	.default_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
>>  	.no_constraint_value = PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE,
>>  	.type = PM_QOS_MAX,
>> @@ -157,6 +163,43 @@ static inline void pm_qos_set_value(struct pm_qos_constraints *c, s32 value)
>>  	c->target_value = value;
>>  }
>>
>> +static inline void pm_qos_set_value_for_cpus(struct pm_qos_constraints *c)
>> +{
>> +	struct pm_qos_request *req = NULL;
>> +	int cpu;
>> +	s32 *qos_val;
>> +
>> +	qos_val = kcalloc(NR_CPUS, sizeof(*qos_val), GFP_KERNEL);
>> +	if (!qos_val) {
>> +		WARN("%s: No memory for PM QoS\n", __func__);
>> +		return;
>> +	}
>> +
>> +	for_each_possible_cpu(cpu)
>> +		qos_val[cpu] = c->default_value;
>> +
>> +	plist_for_each_entry(req, &c->list, node) {
>> +		for_each_cpu(cpu, &req->cpus_affine) {
>> +			switch (c->type) {
>> +			case PM_QOS_MIN:
>> +				if (qos_val[cpu] > req->node.prio)
>> +					qos_val[cpu] = req->node.prio;
>> +				break;
>> +			case PM_QOS_MAX:
>> +				if (req->node.prio > qos_val[cpu])
>> +					qos_val[cpu] = req->node.prio;
>> +				break;
>> +			default:
>> +				BUG();
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>> +	for_each_possible_cpu(cpu)
>> +		c->target_per_cpu[cpu] = qos_val[cpu];
>> +}
>> +
>>  /**
>>   * pm_qos_update_target - manages the constraints list and calls the notifiers
>>   *  if needed
>> @@ -206,6 +249,7 @@ int pm_qos_update_target(struct pm_qos_constraints *c,
>>
>>  	curr_value = pm_qos_get_value(c);
>>  	pm_qos_set_value(c, curr_value);
>> +	pm_qos_set_value_for_cpus(c);
>>
>>  	spin_unlock_irqrestore(&pm_qos_lock, flags);
>>
>> @@ -298,6 +342,44 @@ int pm_qos_request(int pm_qos_class)
>>  }
>>  EXPORT_SYMBOL_GPL(pm_qos_request);
>>
>> +int pm_qos_request_for_cpu(int pm_qos_class, int cpu)
>> +{
>> +	return pm_qos_array[pm_qos_class]->constraints->target_per_cpu[cpu];
>> +}
>> +EXPORT_SYMBOL(pm_qos_request_for_cpu);
>> +
>> +int pm_qos_request_for_cpumask(int pm_qos_class, struct cpumask *mask)
>> +{
>> +	unsigned long irqflags;
>> +	int cpu;
>> +	struct pm_qos_constraints *c = NULL;
>> +	int val;
>> +
>> +	spin_lock_irqsave(&pm_qos_lock, irqflags);
>> +	c = pm_qos_array[pm_qos_class]->constraints;
>> +	val = c->default_value;
>> +
>> +	for_each_cpu(cpu, mask) {
>> +		switch (c->type) {
>> +		case PM_QOS_MIN:
>> +			if (c->target_per_cpu[cpu] < val)
>> +				val = c->target_per_cpu[cpu];
>> +			break;
>> +		case PM_QOS_MAX:
>> +			if (c->target_per_cpu[cpu] > val)
>> +				val = c->target_per_cpu[cpu];
>> +			break;
>> +		default:
>> +			BUG();
>> +			break;
>> +		}
>> +	}
>> +	spin_unlock_irqrestore(&pm_qos_lock, irqflags);
>> +
>> +	return val;
>> +}
>> +EXPORT_SYMBOL(pm_qos_request_for_cpumask);
>> +
>>  int pm_qos_request_active(struct pm_qos_request *req)
>>  {
>>  	return req->pm_qos_class != 0;
>> @@ -353,6 +435,24 @@ void pm_qos_add_request(struct pm_qos_request *req,
>>  		WARN(1, KERN_ERR "pm_qos_add_request() called for already added request\n");
>>  		return;
>>  	}
>> +
>> +	switch (req->type) {
>> +	case PM_QOS_REQ_AFFINE_CORES:
>> +		if (cpumask_empty(&req->cpus_affine)) {
>> +			req->type = PM_QOS_REQ_ALL_CORES;
>> +			cpumask_setall(&req->cpus_affine);
>> +			WARN(1, KERN_ERR "Affine cores not set for request with affinity flag\n");
>> +		}
>> +		break;
>> +
>> +	default:
>> +		WARN(1, KERN_ERR "Unknown request type %d\n", req->type);
>> +		/* fall through */
>> +	case PM_QOS_REQ_ALL_CORES:
>> +		cpumask_setall(&req->cpus_affine);
>> +		break;
>> +	}
>> +
>>  	req->pm_qos_class = pm_qos_class;
>>  	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>>  	trace_pm_qos_add_request(pm_qos_class, value);
>> @@ -426,6 +526,7 @@ void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
>>   */
>>  void pm_qos_remove_request(struct pm_qos_request *req)
>>  {
>> +
>
>Unnecessary newline added.
>
>>  	if (!req) /*guard against callers passing in null */
>>  		return;
>>  		/* silent return to keep pcm code cleaner */
>> @@ -441,6 +542,7 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>>  			     req, PM_QOS_REMOVE_REQ,
>>  			     PM_QOS_DEFAULT_VALUE);
>> +
>
>ditto.  Cheers,
>Javi
>
>>  	memset(req, 0, sizeof(*req));
>>  }
>>  EXPORT_SYMBOL_GPL(pm_qos_remove_request);
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>

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

* Re: [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability.
  2014-08-13 16:01 ` [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability Lina Iyer
@ 2014-08-18 23:38   ` Kevin Hilman
  2014-08-27 17:44   ` Kevin Hilman
  1 sibling, 0 replies; 15+ messages in thread
From: Kevin Hilman @ 2014-08-18 23:38 UTC (permalink / raw)
  To: Lina Iyer
  Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw, Praveen Chidambaram

Lina Iyer <lina.iyer@linaro.org> writes:

> QoS add requests uses a handle to the priority list that is used
> internally to save the request, but this does not extend well. Also,
> dev_pm_qos structure definition seems to use a list object directly.
> The 'derivative' relationship seems to be broken.
>
> Use pm_qos_request objects instead of passing around the protected
> priority list object.
>
> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>

This looks like a good cleanup.

Acked-by: Kevin Hilman <khilman@linaro.org>

Kevin

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

* Re: [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-13 16:01 ` [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request Lina Iyer
  2014-08-15 12:37   ` Javi Merino
@ 2014-08-18 23:55   ` Kevin Hilman
  2014-08-19  0:34     ` Lina Iyer
  2014-08-27 18:01   ` Kevin Hilman
  2 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2014-08-18 23:55 UTC (permalink / raw)
  To: Lina Iyer
  Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw, Praveen Chidambaram

Hi Lina,

Lina Iyer <lina.iyer@linaro.org> writes:

> QoS request can be better optimized if the request can be set only for
> the required cpus and not all cpus. This helps save power on other
> cores, while still gauranteeing the quality of service on the desired
> cores.
>
> Add a new enumeration to specify the PM QoS request type. The enums help
> specify what is the intended target cpu of the request.
>
> Enhance the QoS constraints data structures to support target value for
> each core. Requests specify if the QoS is applicable to all cores
> (default) or to a selective subset of the cores or to a core(s).
>
> Idle and interested drivers can request a PM QoS value for a constraint
> across all cpus, or a specific cpu or a set of cpus. Separate APIs have
> been added to request for individual cpu or a cpumask.  The default
> behaviour of PM QoS is maintained i.e, requests that do not specify a
> type of the request will continue to be effected on all cores.
>
> The userspace sysfs interface does not support setting cpumask of a PM
> QoS request.
>
> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>

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.  

However, I did start to wonder how this will behave in the context of
the hotplug.  For example, what if a constraint is setup with a cpumask,
then one of those CPUs is hotplugged away.  

Kevin

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

* Re: [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification
  2014-08-13 16:01 ` [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification Lina Iyer
@ 2014-08-19  0:04   ` Kevin Hilman
  2014-08-19  0:17     ` Lina Iyer
  0 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2014-08-19  0:04 UTC (permalink / raw)
  To: Lina Iyer; +Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw

Lina Iyer <lina.iyer@linaro.org> writes:

> The current implementation allows only a single notification callback
> whenever the IRQ's SMP affinity is changed. Adding a second notification
> punts the existing notifier function out of registration.
>
> Add a list of notifiers, allowing multiple clients to register for irq
> affinity notifications.

nit: the changelog describes "what", but doesn't answer "why?"  You
address it a bit in the cover letter, but it should probably be spelled out
here since this patch will likely need to be separated from this series
and taken via the IRQ core maintiners.

Kevin

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

* Re: [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification
  2014-08-19  0:04   ` Kevin Hilman
@ 2014-08-19  0:17     ` Lina Iyer
  0 siblings, 0 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-19  0:17 UTC (permalink / raw)
  To: Kevin Hilman; +Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw

On Mon, Aug 18, 2014 at 05:04:22PM -0700, Kevin Hilman wrote:
>Lina Iyer <lina.iyer@linaro.org> writes:
>
>> The current implementation allows only a single notification callback
>> whenever the IRQ's SMP affinity is changed. Adding a second notification
>> punts the existing notifier function out of registration.
>>
>> Add a list of notifiers, allowing multiple clients to register for irq
>> affinity notifications.
>
>nit: the changelog describes "what", but doesn't answer "why?"  You
>address it a bit in the cover letter, but it should probably be spelled out
>here since this patch will likely need to be separated from this series
>and taken via the IRQ core maintiners.
Good point. I will amend it.

>
>Kevin

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

* Re: [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-18 23:55   ` Kevin Hilman
@ 2014-08-19  0:34     ` Lina Iyer
  0 siblings, 0 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-19  0:34 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw, Praveen Chidambaram

On Mon, Aug 18, 2014 at 04:55:41PM -0700, Kevin Hilman wrote:
>Hi Lina,
>
>Lina Iyer <lina.iyer@linaro.org> writes:
>
>> QoS request can be better optimized if the request can be set only for
>> the required cpus and not all cpus. This helps save power on other
>> cores, while still gauranteeing the quality of service on the desired
>> cores.
>>
>> Add a new enumeration to specify the PM QoS request type. The enums help
>> specify what is the intended target cpu of the request.
>>
>> Enhance the QoS constraints data structures to support target value for
>> each core. Requests specify if the QoS is applicable to all cores
>> (default) or to a selective subset of the cores or to a core(s).
>>
>> Idle and interested drivers can request a PM QoS value for a constraint
>> across all cpus, or a specific cpu or a set of cpus. Separate APIs have
>> been added to request for individual cpu or a cpumask.  The default
>> behaviour of PM QoS is maintained i.e, requests that do not specify a
>> type of the request will continue to be effected on all cores.
>>
>> The userspace sysfs interface does not support setting cpumask of a PM
>> QoS request.
>>
>> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>
>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.
>
>However, I did start to wonder how this will behave in the context of
>the hotplug.  For example, what if a constraint is setup with a cpumask,
>then one of those CPUs is hotplugged away.
>
Thanks for bringing it up. I forgot to mention this in the series, but
well, it can be addressed.

When a core is hotplugged, the IRQ migrates to the next online in the
smp_affinity. The QoS code would work in the hotplug case as well, with
a simple change. The current code does not send affinity_notifications
correctly because of a direct call to irq_chip->irq_set_affinity()
instead of calling the generic irq affinity api.

This is the simple change that needs to be made. I will submit a patch
for that.

In arm64/kernel/irq.c 

-       c = irq_data_get_irq_chip(d);
-       if (!c->irq_set_affinity)
-               pr_debug("IRQ%u: unable to set affinity\n", d->irq);
-       else if (c->irq_set_affinity(d, affinity, true) == IRQ_SET_MASK_OK && ret)
-               cpumask_copy(d->affinity, affinity);
-
-       return ret;
+       return __irq_set_affinity_locked(d, affinity) == 0;



>Kevin

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

* Re: [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability.
  2014-08-13 16:01 ` [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability Lina Iyer
  2014-08-18 23:38   ` Kevin Hilman
@ 2014-08-27 17:44   ` Kevin Hilman
  1 sibling, 0 replies; 15+ messages in thread
From: Kevin Hilman @ 2014-08-27 17:44 UTC (permalink / raw)
  To: Lina Iyer
  Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw, Praveen Chidambaram

Lina Iyer <lina.iyer@linaro.org> writes:

> QoS add requests uses a handle to the priority list that is used
> internally to save the request, but this does not extend well. Also,
> dev_pm_qos structure definition seems to use a list object directly.
> The 'derivative' relationship seems to be broken.
>
> Use pm_qos_request objects instead of passing around the protected
> priority list object.
>
> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>

Acked-by: Kevin Hilman <khilman@linaro.org>

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

* Re: [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-13 16:01 ` [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request Lina Iyer
  2014-08-15 12:37   ` Javi Merino
  2014-08-18 23:55   ` Kevin Hilman
@ 2014-08-27 18:01   ` Kevin Hilman
  2014-08-27 20:13     ` Lina Iyer
  2 siblings, 1 reply; 15+ messages in thread
From: Kevin Hilman @ 2014-08-27 18:01 UTC (permalink / raw)
  To: Lina Iyer
  Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw, Praveen Chidambaram

Lina Iyer <lina.iyer@linaro.org> writes:

> QoS request can be better optimized if the request can be set only for
> the required cpus and not all cpus. This helps save power on other
> cores, while still gauranteeing the quality of service on the desired
> cores.
>
> Add a new enumeration to specify the PM QoS request type. The enums help
> specify what is the intended target cpu of the request.
>
> Enhance the QoS constraints data structures to support target value for
> each core. Requests specify if the QoS is applicable to all cores
> (default) or to a selective subset of the cores or to a core(s).
>
> Idle and interested drivers can request a PM QoS value for a constraint
> across all cpus, or a specific cpu or a set of cpus. Separate APIs have
> been added to request for individual cpu or a cpumask.  The default
> behaviour of PM QoS is maintained i.e, requests that do not specify a
> type of the request will continue to be effected on all cores.
>
> The userspace sysfs interface does not support setting cpumask of a PM
> QoS request.
>
> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>

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.

Kevin

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

* Re: [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request
  2014-08-27 18:01   ` Kevin Hilman
@ 2014-08-27 20:13     ` Lina Iyer
  0 siblings, 0 replies; 15+ messages in thread
From: Lina Iyer @ 2014-08-27 20:13 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: daniel.lezcano, ulf.hansson, linux-pm, tglx, rjw, Praveen Chidambaram

On Wed, Aug 27, 2014 at 11:01:40AM -0700, Kevin Hilman wrote:
>Lina Iyer <lina.iyer@linaro.org> writes:
>
>> QoS request can be better optimized if the request can be set only for
>> the required cpus and not all cpus. This helps save power on other
>> cores, while still gauranteeing the quality of service on the desired
>> cores.
>>
>> Add a new enumeration to specify the PM QoS request type. The enums help
>> specify what is the intended target cpu of the request.
>>
>> Enhance the QoS constraints data structures to support target value for
>> each core. Requests specify if the QoS is applicable to all cores
>> (default) or to a selective subset of the cores or to a core(s).
>>
>> Idle and interested drivers can request a PM QoS value for a constraint
>> across all cpus, or a specific cpu or a set of cpus. Separate APIs have
>> been added to request for individual cpu or a cpumask.  The default
>> behaviour of PM QoS is maintained i.e, requests that do not specify a
>> type of the request will continue to be effected on all cores.
>>
>> The userspace sysfs interface does not support setting cpumask of a PM
>> QoS request.
>>
>> Signed-off-by: Praveen Chidambaram <pchidamb@codeaurora.org>
>> Signed-off-by: Lina Iyer <lina.iyer@linaro.org>
>
>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.
>
If you mean dev-pm-qos, then yes. I explored using that. The dev-pm-qos
is an user of the qos framework and holds an pm-qos object, but not in
any other way influences the final value of the QoS constraint other
than specify a request on behalf of the device.  IMHO, What we want is
complementary. When a device specifies a request, we want the request to
be directed at a set of cpus and that is a function of the QoS
framework, hence addressed by these patches to the QoS framework.

>Kevin
>--
>To unsubscribe from this list: send the line "unsubscribe linux-pm" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2014-08-27 20:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-13 16:01 [PATCH v2 0/4] PM QoS: per-cpu PM QoS support Lina Iyer
2014-08-13 16:01 ` [PATCH v2 1/4] QoS: Modify data structures and function arguments for scalability Lina Iyer
2014-08-18 23:38   ` Kevin Hilman
2014-08-27 17:44   ` Kevin Hilman
2014-08-13 16:01 ` [PATCH v2 2/4] QoS: Enhance framework to support per-cpu PM QoS request Lina Iyer
2014-08-15 12:37   ` Javi Merino
2014-08-15 15:06     ` Lina Iyer
2014-08-18 23:55   ` Kevin Hilman
2014-08-19  0:34     ` Lina Iyer
2014-08-27 18:01   ` Kevin Hilman
2014-08-27 20:13     ` Lina Iyer
2014-08-13 16:01 ` [PATCH v2 3/4] irq: Allow multiple clients to register for irq affinity notification Lina Iyer
2014-08-19  0:04   ` Kevin Hilman
2014-08-19  0:17     ` Lina Iyer
2014-08-13 16:01 ` [PATCH v2 4/4] QoS: Enable PM QoS requests to apply only on smp_affinity of an IRQ Lina Iyer

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.