Re: adding handles to pm_qos?

["Ai Li" <aili@codeaurora.org>]

> Oh.
> 
> This will not scale with the aggregation logic very well at all
> if
> pm_qos update requirement gets hit per transaction through a
> driver code
> path, then I think some thought on the scalability is needed and
> perhaps
> a change to the aggregation design for such uses.
> 
> Do you have a patch for the handle implementation I could look
> at?

Here is the patch that I use with the test code.  There are three new
functions:

void *pm_qos_get(int qos, char *name);
int pm_qos_put(void *handle);
int pm_qos_update_requirement_direct(void *handle, s32 new_value);

In the test, I wanted to keep the existing interface intact so I
could compare them at the same time.  For the formal code submission,
a different approach may work better.


--- include/linux/pm_qos_params.h.orig
+++ include/linux/pm_qos_params.h
@@ -16,7 +16,11 @@
 
 int pm_qos_add_requirement(int qos, char *name, s32 value);
 int pm_qos_update_requirement(int qos, char *name, s32 new_value);
-void pm_qos_remove_requirement(int qos, char *name);
+int pm_qos_remove_requirement(int qos, char *name);
+
+void *pm_qos_get(int qos, char *name);
+int pm_qos_put(void *handle);
+int pm_qos_update_requirement_direct(void *handle, s32 new_value);
 
 int pm_qos_requirement(int qos);
 
--- kernel/pm_qos_params.c.orig
+++ kernel/pm_qos_params.c
@@ -55,6 +55,8 @@ struct requirement_list {
 		s32 kbps;
 	};
 	char *name;
+	int pm_qos_class;
+	int handle_count;
 };
 
 static s32 max_compare(s32 v1, s32 v2);
@@ -210,7 +212,9 @@ EXPORT_SYMBOL_GPL(pm_qos_requirement);
 int pm_qos_add_requirement(int pm_qos_class, char *name, s32 value)
 {
 	struct requirement_list *dep;
+	struct requirement_list *node;
 	unsigned long flags;
+	int retval;
 
 	dep = kzalloc(sizeof(struct requirement_list), GFP_KERNEL);
 	if (dep) {
@@ -219,10 +223,25 @@ int pm_qos_add_requirement(int pm_qos_class,
char *name, s32 value)
 		else
 			dep->value = value;
 		dep->name = kstrdup(name, GFP_KERNEL);
-		if (!dep->name)
+		if (!dep->name) {
+			retval = -ENOMEM;
 			goto cleanup;
+		}
+		dep->pm_qos_class = pm_qos_class;
 
 		spin_lock_irqsave(&pm_qos_lock, flags);
+		/* make sure the new name is unique */
+		list_for_each_entry(node,
+
&pm_qos_array[pm_qos_class]->requirements.list, list) {
+			if (strcmp(node->name, name) == 0) {
+				spin_unlock_irqrestore(&pm_qos_lock,
flags);
+
+				kfree(dep->name);
+				retval = -EINVAL;
+				goto cleanup;
+			}
+		}
+
 		list_add(&dep->list,
 
&pm_qos_array[pm_qos_class]->requirements.list);
 		spin_unlock_irqrestore(&pm_qos_lock, flags);
@@ -231,9 +250,11 @@ int pm_qos_add_requirement(int pm_qos_class,
char *name, s32 value)
 		return 0;
 	}
 
+	retval = -ENOMEM;
+
 cleanup:
 	kfree(dep);
-	return -ENOMEM;
+	return retval;
 }
 EXPORT_SYMBOL_GPL(pm_qos_add_requirement);
 
@@ -282,8 +303,10 @@ EXPORT_SYMBOL_GPL(pm_qos_update_requirement);
  *
  * Will remove named qos request from pm_qos_class list of
parameters and
  * recompute the current target value for the pm_qos_class.
+ *
+ * If there are outstanding handles to the request, -EBUSY is
returned.
  */
-void pm_qos_remove_requirement(int pm_qos_class, char *name)
+int pm_qos_remove_requirement(int pm_qos_class, char *name)
 {
 	unsigned long flags;
 	struct requirement_list *node;
@@ -293,6 +316,11 @@ void pm_qos_remove_requirement(int pm_qos_class,
char *name)
 	list_for_each_entry(node,
 		&pm_qos_array[pm_qos_class]->requirements.list, list)
{
 		if (strcmp(node->name, name) == 0) {
+			if (node->handle_count > 0) {
+				spin_unlock_irqrestore(&pm_qos_lock,
flags);
+				return -EBUSY;
+			}
+
 			kfree(node->name);
 			list_del(&node->list);
 			kfree(node);
@@ -303,9 +331,83 @@ void pm_qos_remove_requirement(int pm_qos_class,
char *name)
 	spin_unlock_irqrestore(&pm_qos_lock, flags);
 	if (pending_update)
 		update_target(pm_qos_class);
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(pm_qos_remove_requirement);
 
+/* pm_qos_get - returns an opaque handle to the existing qos request
+ * @pm_qos_class: identifies which list of qos request to us
+ * @name: identifies the request
+ *
+ * Will increment the reference count as well.
+ *
+ * If the named request isn't in the list then NULL is returned.
+ */
+void *pm_qos_get(int pm_qos_class, char *name)
+{
+	unsigned long flags;
+	struct requirement_list *node;
+	struct requirement_list *handle = NULL;
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	list_for_each_entry(node,
+		&pm_qos_array[pm_qos_class]->requirements.list, list)
{
+		if (strcmp(node->name, name) == 0) {
+			node->handle_count++;
+			handle = node;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+	return handle;
+}
+EXPORT_SYMBOL_GPL(pm_qos_get);
+
+/* pm_qos_put - release the handle of the existing qos request
+ * @handle: previously acquired through pm_qos_get
+ *
+ * Will decrement the reference count as well. Caller should no
+ * longer use the handle after this call.
+ */
+int pm_qos_put(void *handle)
+{
+	unsigned long flags;
+	struct requirement_list *node =	(struct requirement_list *)
handle;
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	BUG_ON(node->handle_count == 0);
+	node->handle_count--;
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pm_qos_put);
+
+/* pm_qos_update_requirement_direct - modifies an existing qos
request
+ * @handle: previously acquired through pm_qos_get
+ * @value: defines the qos request
+ *
+ * Updates an existing qos requirement for the pm_qos_class of
parameters along
+ * with updating the target pm_qos_class value.
+ */
+int pm_qos_update_requirement_direct(void *handle, s32 new_value)
+{
+	unsigned long flags;
+	struct requirement_list *node = (struct requirement_list *)
handle;
+
+	spin_lock_irqsave(&pm_qos_lock, flags);
+	if (new_value == PM_QOS_DEFAULT_VALUE)
+		node->value =
pm_qos_array[node->pm_qos_class]->default_value;
+	else
+		node->value = new_value;
+	spin_unlock_irqrestore(&pm_qos_lock, flags);
+	update_target(node->pm_qos_class);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pm_qos_update_requirement_direct);
+
 /**
  * pm_qos_add_notifier - sets notification entry for changes to
target value
  * @pm_qos_class: identifies which qos target changes should be
notified.


~Ai