Re: adding handles to pm_qos?

[640E9920 <640e9920@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 9079 bytes --]

On Tue, Nov 17, 2009 at 06:06:56PM -0700, Ai Li wrote:
> Date: Tue, 17 Nov 2009 18:06:56 -0700
> From: Ai Li <aili@codeaurora.org>
> To: mgross@linux.intel.com
> Cc: linux-pm@lists.linux-foundation.org
> Subject: Re: [linux-pm] adding handles to pm_qos?
> 
> > Thanks, I'll look at it over the next few days.
> > 
> > --mgross
> > 
> > > 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.
> 
> 
> Here is an alternate way of adding handles to pm_qos_params that I
> was alluding to.  This approach may be more preferable because it
> does not bloat the API and the handles become an integral part of
> pm_qos_params.  In my previous patch, handles are kind of bolted onto
> pm_qos_params and it needs separate calls (pm_qos_get, pm_qos_put) to
> acquire and release the handle.  In this patch,
> pm_qos_add_requirement and pm_qos_remove_requirement automatically
> take care of the handles.
> 
> 
> --- include/linux/pm_qos_params.h.orig
> +++ include/linux/pm_qos_params.h
> @@ -14,9 +14,11 @@
>  #define PM_QOS_NUM_CLASSES 4
>  #define PM_QOS_DEFAULT_VALUE -1
>  
> -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);
> +struct requirement_list;
> +
> +struct requirement_list *pm_qos_add_requirement(int qos, char *name,
> s32 value);
> +int pm_qos_update_requirement(struct requirement_list *qos, s32
> new_value);
> +void pm_qos_remove_requirement(struct requirement_list *qos);
>  
>  int pm_qos_requirement(int qos);
>  
> --- kernel/pm_qos_params.c.orig
> +++ kernel/pm_qos_params.c
> @@ -49,6 +49,7 @@
>   */
>  struct requirement_list {
>  	struct list_head list;
> +	int pm_qos_class;
>  	union {
>  		s32 value;
>  		s32 usec;
> @@ -207,13 +208,16 @@ EXPORT_SYMBOL_GPL(pm_qos_requirement);
>   * performance characteristics.  It recomputes the aggregate QoS
> expectations
>   * for the pm_qos_class of parameters.
>   */
> -int pm_qos_add_requirement(int pm_qos_class, char *name, s32 value)
> +struct requirement_list *pm_qos_add_requirement(int pm_qos_class,
> char *name,

looks like your email client buggered the your patch.

mgross@mgross-laptop:~/marks/Linux/linux-2.6$ patch -p0 < /home/mgross/pm_qos_handle.patch
patching file include/linux/pm_qos_params.h
patch: **** malformed patch at line 43: s32 value);

Please resend 

--mgross



> +		s32 value)
>  {
>  	struct requirement_list *dep;
>  	unsigned long flags;
>  
>  	dep = kzalloc(sizeof(struct requirement_list), GFP_KERNEL);
>  	if (dep) {
> +		dep->pm_qos_class = pm_qos_class;
> +
>  		if (value == PM_QOS_DEFAULT_VALUE)
>  			dep->value =
> pm_qos_array[pm_qos_class]->default_value;
>  		else
> @@ -228,48 +232,37 @@ int pm_qos_add_requirement(int pm_qos_class,
> char *name, s32 value)
>  		spin_unlock_irqrestore(&pm_qos_lock, flags);
>  		update_target(pm_qos_class);
>  
> -		return 0;
> +		return dep;
>  	}
>  
>  cleanup:
>  	kfree(dep);
> -	return -ENOMEM;
> +	return ERR_PTR(-ENOMEM);
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_add_requirement);
>  
>  /**
>   * pm_qos_update_requirement - modifies an existing qos request
> - * @pm_qos_class: identifies which list of qos request to us
> - * @name: identifies the request
> + * @qos: identifies the qos request to us
>   * @value: defines the qos request
>   *
> - * Updates an existing qos requirement for the pm_qos_class of
> parameters along
> + * Updates an existing qos requirement along
>   * with updating the target pm_qos_class value.
> - *
> - * If the named request isn't in the list then no change is made.
>   */
> -int pm_qos_update_requirement(int pm_qos_class, char *name, s32
> new_value)
> +int pm_qos_update_requirement(struct requirement_list *qos, s32
> new_value)
>  {
>  	unsigned long flags;
> -	struct requirement_list *node;
>  	int pending_update = 0;
>  
>  	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) {
> -			if (new_value == PM_QOS_DEFAULT_VALUE)
> -				node->value =
> -
> pm_qos_array[pm_qos_class]->default_value;
> -			else
> -				node->value = new_value;
> -			pending_update = 1;
> -			break;
> -		}
> -	}
> +	if (new_value == PM_QOS_DEFAULT_VALUE)
> +		qos->value =
> pm_qos_array[qos->pm_qos_class]->default_value;
> +	else
> +		qos->value = new_value;
> +	pending_update = 1;
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);
>  	if (pending_update)
> -		update_target(pm_qos_class);
> +		update_target(qos->pm_qos_class);
>  
>  	return 0;
>  }
> @@ -277,32 +270,26 @@ EXPORT_SYMBOL_GPL(pm_qos_update_requirement);
>  
>  /**
>   * pm_qos_remove_requirement - modifies an existing qos request
> - * @pm_qos_class: identifies which list of qos request to us
> + * @qos: identifies the qos request to us
>   * @name: identifies the request
>   *
> - * Will remove named qos request from pm_qos_class list of
> parameters and
> + * Will remove qos request from pm_qos_class list and
>   * recompute the current target value for the pm_qos_class.
>   */
> -void pm_qos_remove_requirement(int pm_qos_class, char *name)
> +void pm_qos_remove_requirement(struct requirement_list *qos)
>  {
>  	unsigned long flags;
> -	struct requirement_list *node;
>  	int pending_update = 0;
>  
>  	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) {
> -			kfree(node->name);
> -			list_del(&node->list);
> -			kfree(node);
> -			pending_update = 1;
> -			break;
> -		}
> -	}
> +	list_del(&qos->list);
> +	pending_update = 1;
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);
>  	if (pending_update)
> -		update_target(pm_qos_class);
> +		update_target(qos->pm_qos_class);
> +
> +	kfree(qos->name);
> +	kfree(qos);
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_remove_requirement);
>  
> @@ -345,37 +332,42 @@ int pm_qos_remove_notifier(int pm_qos_class,
> struct notifier_block *notifier)
>  EXPORT_SYMBOL_GPL(pm_qos_remove_notifier);
>  
>  #define PID_NAME_LEN sizeof("process_1234567890")
> -static char name[PID_NAME_LEN];
>  
>  static int pm_qos_power_open(struct inode *inode, struct file *filp)
>  {
>  	int ret;
>  	long pm_qos_class;
> +	char name[PID_NAME_LEN];
> +	struct requirement_list *qos;
>  
>  	lock_kernel();
>  	pm_qos_class = find_pm_qos_object_by_minor(iminor(inode));
> -	if (pm_qos_class >= 0) {
> -		filp->private_data = (void *)pm_qos_class;
> -		sprintf(name, "process_%d", current->pid);
> -		ret = pm_qos_add_requirement(pm_qos_class, name,
> -					PM_QOS_DEFAULT_VALUE);
> -		if (ret >= 0) {
> -			unlock_kernel();
> -			return 0;
> -		}
> +	if (pm_qos_class < 0) {
> +		ret = -EPERM;
> +		goto power_open_exit;
>  	}
> -	unlock_kernel();
>  
> -	return -EPERM;
> +	sprintf(name, "process_%d", current->pid);
> +	qos = pm_qos_add_requirement(pm_qos_class, name,
> PM_QOS_DEFAULT_VALUE);
> +	if (IS_ERR(qos)) {
> +		ret = PTR_ERR(qos);
> +		goto power_open_exit;
> +	}
> +
> +	filp->private_data = qos;
> +	ret = 0;
> +
> +power_open_exit:
> +	unlock_kernel();
> +	return ret;
>  }
>  
>  static int pm_qos_power_release(struct inode *inode, struct file
> *filp)
>  {
> -	int pm_qos_class;
> +	struct requirement_list *qos;
>  
> -	pm_qos_class = (long)filp->private_data;
> -	sprintf(name, "process_%d", current->pid);
> -	pm_qos_remove_requirement(pm_qos_class, name);
> +	qos = (struct requirement_list *)filp->private_data;
> +	pm_qos_remove_requirement(qos);
>  
>  	return 0;
>  }
> @@ -384,15 +376,15 @@ static ssize_t pm_qos_power_write(struct file
> *filp, const char __user *buf,
>  		size_t count, loff_t *f_pos)
>  {
>  	s32 value;
> -	int pm_qos_class;
> +	struct requirement_list *qos;
>  
> -	pm_qos_class = (long)filp->private_data;
>  	if (count != sizeof(s32))
>  		return -EINVAL;
>  	if (copy_from_user(&value, buf, sizeof(s32)))
>  		return -EFAULT;
> -	sprintf(name, "process_%d", current->pid);
> -	pm_qos_update_requirement(pm_qos_class, name, value);
> +
> +	qos = (struct requirement_list *)filp->private_data;
> +	pm_qos_update_requirement(qos, value);
>  
>  	return  sizeof(s32);
>  }
> 
> 
> ~Ai
> 
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/linux-pm

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]