On Sun, Jun 06, 2010 at 02:39:54PM -0700, mark gross wrote:
> On Sat, Jun 05, 2010 at 12:58:08PM -0500, James Bottomley wrote:
> > A lot of the pm_qos extremal value handling is really duplicating what a
> > priority ordered list does, just in a less efficient fashion. Simply
> > redoing the implementation in terms of a plist gets rid of a lot of this
> > junk (although there are several other strange things that could do with
> > tidying up, like pm_qos_request_list has to carry the pm_qos_class with
> > every node, simply because it doesn't get passed in to
> > pm_qos_update_request even though every caller knows full well what
> > parameter it's updating).
> >
> > I think this redo is a win independent of android, so we should do
> > something like this now.
> >
> > There is one nasty that should probably be fixed in plists not open
> > coded here: plist_first gives the highest priority value, but there's no
> > corresponding API to give the lowest (even though you can get it from
> > the head.nodes_list.prev) ... if the sched people are OK, I'll correct
> > this with the final patch set.
> >
> > James
> >
> > ---
> >
> > kernel/pm_qos_params.c | 152 +++++++++++++++++++++++-------------------------
> > 1 files changed, 73 insertions(+), 79 deletions(-)
> >
> > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > index f42d3f7..241fa79 100644
> > --- a/kernel/pm_qos_params.c
> > +++ b/kernel/pm_qos_params.c
> > @@ -30,6 +30,7 @@
> > /*#define DEBUG*/
snip
> > @@ -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.
>
snip
> > 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.
>
snip
>
I found that e1000e will panic on rmmod because of it attempting to
removing of a pm_qos request that it never added.
This is an ugly patch, but I think its needed for a while to clean out
the abusers, then it can be updated to not be so noisy.
--mgross
--Signed-off-by: mark gross <markgross@xxxxxxxxxxx>
>From fb713f95b83ea3744c31917cfd019bf3e32349b3 Mon Sep 17 00:00:00 2001
From: markgross <markgross@xxxxxxxxxxx>
Date: Tue, 8 Jun 2010 06:22:01 -0700
Subject: [PATCH] check and complain about abuse of the api to avoid panics
---
kernel/pm_qos_params.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index f1d3d23..4bded27 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -212,7 +212,7 @@ void pm_qos_add_request(struct pm_qos_request_list *dep,
int new_value;
if (pm_qos_request_active(dep))
- return;
+ return; /* already in the list */
if (value == PM_QOS_DEFAULT_VALUE)
new_value = o->default_value;
@@ -244,6 +244,11 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
if (!pm_qos_req) /*guard against callers passing in null */
return;
+ if (!pm_qos_request_active(pm_qos_req)) {
+ WARN(true, "pm_qos: update to an unregistered request");
+ dump_stack();
+ return;
+ }
o = pm_qos_array[pm_qos_req->pm_qos_class];
@@ -279,6 +284,11 @@ void pm_qos_remove_request(struct pm_qos_request_list *pm_qos_req)
if (pm_qos_req == NULL)
return;
/* silent return to keep pcm code cleaner */
+ if (!pm_qos_request_active(pm_qos_req)) {
+ WARN(true, "pm_qos: removal an unregistered request");
+ dump_stack();
+ return;
+ }
o = pm_qos_array[pm_qos_req->pm_qos_class];
update_target(o, &pm_qos_req->list, 1);
--
1.6.3.3
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm
