Re: [RFC] pm_qos: reimplement using plists

[James Bottomley <James.Bottomley@suse.de>]

On Sun, 2010-06-06 at 14:39 -0700, mark gross wrote:
> > @@ -251,22 +244,27 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
> >  	unsigned long flags;
> >  	int pending_update = 0;
> >  	s32 temp;
> > +	struct pm_qos_object *o;
> >  
> > -	if (pm_qos_req) { /*guard against callers passing in null */
> > -		spin_lock_irqsave(&pm_qos_lock, flags);
> > -		if (new_value == PM_QOS_DEFAULT_VALUE)
> > -			temp = pm_qos_array[pm_qos_req->pm_qos_class]->default_value;
> > -		else
> > -			temp = new_value;
> > +	if (!pm_qos_req) /*guard against callers passing in null */
> > +		return;
> 
> need a better test to see if the pm_qos_req is in the plist or not as we
> move to a caller allocated design.

This is a guard against callers passing in NULL ... which is probably
unnecessary ... I think oopsing on a NULL deref would be just fine for
that, since it would represent a programming error.

> >  
> > -		if (temp != pm_qos_req->value) {
> > -			pending_update = 1;
> > -			pm_qos_req->value = temp;
> > -		}
> > +	o = pm_qos_array[pm_qos_req->pm_qos_class];
> > +
> > +	if (new_value == PM_QOS_DEFAULT_VALUE)
> > +		temp = o->default_value;
> > +	else
> > +		temp = new_value;
> > +
> > +	if (temp != pm_qos_req->list.prio) {
> > +		pending_update = 1;
> > +		spin_lock_irqsave(&pm_qos_lock, flags);
> > +		plist_del(&pm_qos_req->list, &o->requests);
> > +		plist_node_init(&pm_qos_req->list, temp);
> >  		spin_unlock_irqrestore(&pm_qos_lock, flags);
> > -		if (pending_update)
> > -			update_target(pm_qos_req->pm_qos_class);
> >  	}
> > +	if (pending_update)
> > +		update_target(o, &pm_qos_req->list, 0);
> >  }
> >  EXPORT_SYMBOL_GPL(pm_qos_update_request);
> >  
> > @@ -280,19 +278,15 @@ EXPORT_SYMBOL_GPL(pm_qos_update_request);
> >   */
> >  void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req)
> >  {
> > -	unsigned long flags;
> > -	int qos_class;
> > +	struct pm_qos_object *o;
> >  
> >  	if (pm_qos_req == NULL)
> >  		return;
> >  		/* silent return to keep pcm code cleaner */
> 
> need a way to tell if the request is in the list or not so we don't
> crater removing a plist node that isn't in the list.

It won't: plist_del is idempotent, like list_del.  Of course, such a
programming error represents a bug and probably *should* be reported.

James