Thanks, I'll look at it over the next few days.
--mgross
On Fri, Oct 30, 2009 at 07:53:42PM -0600, Ai Li wrote:
> > 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
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
