complain when users abuse the pm_qos API

PATCH In review mark gross 3 participants 15 messages May 29, 2010 – May 30, 2010

Share & tools

mark gross

author

2010-05-29 04:50 UTC #0 lore raw

PATCH 1/1 complain when users abuse the pm_qos API

The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.

--mgross

--Signed-off-by: markgross <[email protected]>

Quoted 1 line

From a0813007ddc7b72cb3da7a533fefba9889aab1d8 Mon Sep 17 00:00:00 2001

From: mgross <mgross@mgross-desktop.(none)>
Date: Fri, 28 May 2010 21:36:06 -0700
Subject: [PATCH] complain when users abuse the pm_qos API by updating a request that
isn't registered yet.

---
 kernel/pm_qos_params.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

kernel/pm_qos_params.c +4 -0

index f42d3f7..8e55bf1 100644
@@ -266,6 +266,10 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
 		spin_unlock_irqrestore(&pm_qos_lock, flags);
 		if (pending_update)
 			update_target(pm_qos_req->pm_qos_class);
+	} else {
+		WARN(true, "pm_qos: updating an unregistered request "
+				"does nothing");
+		dump_stack();
 	}
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);

Signature · 1 line

1.7.0.4

Rafael J. Wysocki

2010-05-29 20:07 UTC #1 lore raw

Quoted 6 lines · mark gross

The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross <[email protected]>

Will there be a big issue if I push this during the next merge window?

Rafael

Quoted 26 lines

From a0813007ddc7b72cb3da7a533fefba9889aab1d8 Mon Sep 17 00:00:00 2001
From: mgross <mgross@mgross-desktop.(none)>
Date: Fri, 28 May 2010 21:36:06 -0700
Subject: [PATCH] complain when users abuse the pm_qos API by updating a request that
isn't registered yet.
---
 kernel/pm_qos_params.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
kernel/pm_qos_params.c +4 -0
index f42d3f7..8e55bf1 100644
@@ -266,6 +266,10 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
 		spin_unlock_irqrestore(&pm_qos_lock, flags);
 		if (pending_update)
 			update_target(pm_qos_req->pm_qos_class);
+	} else {
+		WARN(true, "pm_qos: updating an unregistered request "
+				"does nothing");
+		dump_stack();
 	}
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);

Nigel Cunningham

2010-05-29 23:03 UTC #2 lore raw

Hi.

Quoted 9 lines · Rafael J. Wysocki

Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?

What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

Regards,

Nigel

Nigel Cunningham

2010-05-29 23:03 UTC #3 lore raw

In reply to Rafael J. Wysocki

Hi.

Quoted 9 lines · Rafael J. Wysocki

Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?

What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

Regards,

Nigel

Rafael J. Wysocki

2010-05-30 19:48 UTC #4 lore raw

Quoted 9 lines · Nigel Cunningham

Hi.
Quoted 6 lines · Rafael J. Wysocki
Quoted 5 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

It is an API violation if I understand that correctly.

Rafael

mark gross

author

2010-05-30 20:11 UTC #5 lore raw

Quoted 12 lines · Rafael J. Wysocki

Quoted 11 lines · Nigel Cunningham
Hi.
Quoted 8 lines · Rafael J. Wysocki
Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross<[email protected]>

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?
It is an API violation if I understand that correctly.

Yeah, it is, but now that I'm thinking clearly perhaps a better fix would be to change the prototype of pm_qos_update_request to return something so callers can check for success.

Lets fix the API rather than use this patch. Please dopt apply it.

--mgross

Rafael J. Wysocki

2010-05-30 20:54 UTC #6 lore raw

Quoted 19 lines · mark gross

Quoted 18 lines · Rafael J. Wysocki
Quoted 16 lines · Nigel Cunningham
Hi.
Quoted 9 lines · Rafael J. Wysocki
Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

Quoted 7 lines

Quoted 1 line
It is an API violation if I understand that correctly.
Yeah, it is, but now that I'm thinking clearly perhaps a better fix would be to change the prototype of pm_qos_update_request to return something so callers can check for success.
Lets fix the API rather than use this patch. Please dopt apply it.

Rafael

Rafael J. Wysocki

2010-05-30 20:55 UTC #7 lore raw

In reply to mark gross

Quoted 19 lines · mark gross

Quoted 18 lines · Rafael J. Wysocki
Quoted 16 lines · Nigel Cunningham
Hi.
Quoted 9 lines · Rafael J. Wysocki
Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

Quoted 7 lines

Quoted 1 line
It is an API violation if I understand that correctly.
Yeah, it is, but now that I'm thinking clearly perhaps a better fix would be to change the prototype of pm_qos_update_request to return something so callers can check for success.
Lets fix the API rather than use this patch. Please dopt apply it.

Rafael

mark gross

author

2010-05-30 20:11 UTC #8 lore raw

In reply to Rafael J. Wysocki

Quoted 12 lines · Rafael J. Wysocki

Quoted 11 lines · Nigel Cunningham
Hi.
Quoted 8 lines · Rafael J. Wysocki
Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross<[email protected]>

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?
It is an API violation if I understand that correctly.

Yeah, it is, but now that I'm thinking clearly perhaps a better fix would be to change the prototype of pm_qos_update_request to return something so callers can check for success.

Lets fix the API rather than use this patch. Please dopt apply it.

--mgross

Rafael J. Wysocki

2010-05-30 19:50 UTC #9 lore raw

In reply to Nigel Cunningham

Quoted 9 lines · Nigel Cunningham

Hi.
Quoted 6 lines · Rafael J. Wysocki
Quoted 5 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

It is an API violation if I understand that correctly.

Rafael

mark gross

author

2010-05-30 20:07 UTC #10 lore raw

In reply to Nigel Cunningham

Quoted 9 lines · Nigel Cunningham

Hi.
Quoted 6 lines · Rafael J. Wysocki
Quoted 5 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

If e1000e realy needs the latency set and makes assumptions that its done its part, I would like to let them know that they have not registerd the request they thought they did.

--mgross

Quoted 4 lines

Regards,
Nigel

mark gross

author

2010-05-30 20:07 UTC #11 lore raw

In reply to Nigel Cunningham

Quoted 9 lines · Nigel Cunningham

Hi.
Quoted 6 lines · Rafael J. Wysocki
Quoted 5 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
--Signed-off-by: markgross<[email protected]>
Will there be a big issue if I push this during the next merge window?
What's the point to the patch? That is: why is calling update_request before registering a request such a big problem that it demands a WARN() and dump stack?

If e1000e realy needs the latency set and makes assumptions that its done its part, I would like to let them know that they have not registerd the request they thought they did.

--mgross

Quoted 4 lines

Regards,
Nigel

mark gross

author

2010-05-30 20:08 UTC #12 lore raw

In reply to Rafael J. Wysocki

Quoted 9 lines · Rafael J. Wysocki

Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross <[email protected]>
Will there be a big issue if I push this during the next merge window?

No big issue. it can wait.

--mgross

Quoted 31 lines

Rafael
Quoted 26 lines
From a0813007ddc7b72cb3da7a533fefba9889aab1d8 Mon Sep 17 00:00:00 2001
From: mgross <mgross@mgross-desktop.(none)>
Date: Fri, 28 May 2010 21:36:06 -0700
Subject: [PATCH] complain when users abuse the pm_qos API by updating a request that
isn't registered yet.
---
 kernel/pm_qos_params.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
kernel/pm_qos_params.c +4 -0
index f42d3f7..8e55bf1 100644
@@ -266,6 +266,10 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
 		spin_unlock_irqrestore(&pm_qos_lock, flags);
 		if (pending_update)
 			update_target(pm_qos_req->pm_qos_class);
+	} else {
+		WARN(true, "pm_qos: updating an unregistered request "
+				"does nothing");
+		dump_stack();
 	}
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);

mark gross

author

2010-05-30 20:08 UTC #13 lore raw

In reply to Rafael J. Wysocki

Quoted 9 lines · Rafael J. Wysocki

Quoted 6 lines · mark gross
The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross <[email protected]>
Will there be a big issue if I push this during the next merge window?

No big issue. it can wait.

--mgross

Quoted 31 lines

Rafael
Quoted 26 lines
From a0813007ddc7b72cb3da7a533fefba9889aab1d8 Mon Sep 17 00:00:00 2001
From: mgross <mgross@mgross-desktop.(none)>
Date: Fri, 28 May 2010 21:36:06 -0700
Subject: [PATCH] complain when users abuse the pm_qos API by updating a request that
isn't registered yet.
---
 kernel/pm_qos_params.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
kernel/pm_qos_params.c +4 -0
index f42d3f7..8e55bf1 100644
@@ -266,6 +266,10 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
 		spin_unlock_irqrestore(&pm_qos_lock, flags);
 		if (pending_update)
 			update_target(pm_qos_req->pm_qos_class);
+	} else {
+		WARN(true, "pm_qos: updating an unregistered request "
+				"does nothing");
+		dump_stack();
 	}
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);

Rafael J. Wysocki

2010-05-29 20:08 UTC #14 lore raw

In reply to mark gross

Quoted 6 lines · mark gross

The following patch is to help clean up API abusers of pm_qos where they call update_request before registering a request.
--mgross
--Signed-off-by: markgross <[email protected]>

Will there be a big issue if I push this during the next merge window?

Rafael

Quoted 26 lines

From a0813007ddc7b72cb3da7a533fefba9889aab1d8 Mon Sep 17 00:00:00 2001
From: mgross <mgross@mgross-desktop.(none)>
Date: Fri, 28 May 2010 21:36:06 -0700
Subject: [PATCH] complain when users abuse the pm_qos API by updating a request that
isn't registered yet.
---
 kernel/pm_qos_params.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
kernel/pm_qos_params.c +4 -0
index f42d3f7..8e55bf1 100644
@@ -266,6 +266,10 @@ void pm_qos_update_request(struct pm_qos_request_list *pm_qos_req,
 		spin_unlock_irqrestore(&pm_qos_lock, flags);
 		if (pending_update)
 			update_target(pm_qos_req->pm_qos_class);
+	} else {
+		WARN(true, "pm_qos: updating an unregistered request "
+				"does nothing");
+		dump_stack();
 	}
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);