[RFC PATCH] PM / QoS: add pm_qos_update_request_timeout API

[RFC PATCH] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

The new API, pm_qos_update_request_timeout() is to provide a timeout
with pm_qos_update_request.

For example, pm_qos_update_request_timeout(req, 100, 1000), means that
QoS request on req with value 100 will be active for 1000 jiffies.
After 1000 jiffies, the QoS request thru req is rolled back to the
request status when pm_qos_update_request_timeout() was called. If there
were another pm_qos_update_request(req, x) during the 1000 jiffies, this
new request with value x will override as this is another request on the
same req handle. A new request on the same req handle will always
override the previous request whether it is the conventional request or
it is the new timeout request.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
---
 include/linux/pm_qos.h |    5 ++++
 kernel/power/qos.c     |   54 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 1 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index f8ccb7b..1c29f52 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -8,6 +8,7 @@
 #include <linux/notifier.h>
 #include <linux/miscdevice.h>
 #include <linux/device.h>
+#include <linux/workqueue.h>
 
 #define PM_QOS_RESERVED 0
 #define PM_QOS_CPU_DMA_LATENCY 1
@@ -29,6 +30,8 @@
 struct pm_qos_request {
 	struct plist_node node;
 	int pm_qos_class;
+	s32 value; /* the back-up value for pm_qos_update_request_timeout */
+	struct delayed_work work; /* for pm_qos_update_request_timeout */
 };
 
 struct dev_pm_qos_request {
@@ -74,6 +77,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
 			s32 value);
 void pm_qos_update_request(struct pm_qos_request *req,
 			   s32 new_value);
+void pm_qos_update_request_timeout(struct pm_qos_request *req,
+				   s32 new_value, unsigned long timeout);
 void pm_qos_remove_request(struct pm_qos_request *req);
 
 int pm_qos_request(int pm_qos_class);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index b15e0b7..acfa433 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
 EXPORT_SYMBOL_GPL(pm_qos_request_active);
 
 /**
+ * pm_qos_timeout - the timeout handler of pm_qos_update_request_timeout
+ * @work: work struct for the delayed work (timeout)
+ *
+ * This cancels the timeout request and rolls the request value back.
+ */
+static void pm_qos_timeout(struct work_struct *work)
+{
+	struct pm_qos_request *req = container_of(to_delayed_work(work),
+						  struct pm_qos_request,
+						  work);
+
+	pm_qos_update_request(req, req->value);
+}
+
+/**
  * pm_qos_add_request - inserts new qos request into the list
  * @req: pointer to a preallocated handle
  * @pm_qos_class: identifies which list of qos request to use
@@ -282,6 +297,8 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		return;
 	}
 	req->pm_qos_class = pm_qos_class;
+	req->value = value;
+	INIT_DELAYED_WORK(&req->work, pm_qos_timeout);
 	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
 			     &req->node, PM_QOS_ADD_REQ, value);
 }
@@ -308,14 +325,46 @@ void pm_qos_update_request(struct pm_qos_request *req,
 		return;
 	}
 
-	if (new_value != req->node.prio)
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
+	req->value = new_value;
+	if (new_value != req->node.prio) {
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
 			&req->node, PM_QOS_UPDATE_REQ, new_value);
+	}
 }
 EXPORT_SYMBOL_GPL(pm_qos_update_request);
 
 /**
+ * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
+ * @req : handle to list element holding a pm_qos request to use
+ * @new_value: defines the temporal qos request
+ * @timeout: the effective duration of this qos request in jiffies
+ *
+ * After timeout, this qos request is cancelled automatically.
+ */
+void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
+				   unsigned long timeout)
+{
+	if (!req)
+		return;
+	if (WARN(!pm_qos_request_active(req),
+		 "pm_qos_update_request_timeout() called for unknown object."))
+		return;
+
+	if (new_value != req->node.prio) {
+		pm_qos_update_target(
+			pm_qos_array[req->pm_qos_class]->constraints,
+			&req->node, PM_QOS_UPDATE_REQ, new_value);
+		if (delayed_work_pending(&req->work))
+			cancel_delayed_work_sync(&req->work);
+		schedule_delayed_work(&req->work, timeout);
+	}
+}
+
+/**
  * pm_qos_remove_request - modifies an existing qos request
  * @req: handle to request list element
  *
@@ -334,6 +383,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
 			     &req->node, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
-- 
1.7.4.1

Re: [RFC PATCH] PM / QoS: add pm_qos_update_request_timeout API

[Rafael J. Wysocki]

Hi,

On Tuesday, February 14, 2012, MyungJoo Ham wrote:
> The new API, pm_qos_update_request_timeout() is to provide a timeout
> with pm_qos_update_request.

I need to know what the other people in the CC list think about that.  It would
definitely help if you described a use case.

> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> QoS request on req with value 100 will be active for 1000 jiffies.

Could that be a different time unit instead of jiffies?

> After 1000 jiffies, the QoS request thru req is rolled back to the
> request status when pm_qos_update_request_timeout() was called. If there
> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> new request with value x will override as this is another request on the
> same req handle. A new request on the same req handle will always
> override the previous request whether it is the conventional request or
> it is the new timeout request.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
>  include/linux/pm_qos.h |    5 ++++
>  kernel/power/qos.c     |   54 +++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 58 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index f8ccb7b..1c29f52 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -8,6 +8,7 @@
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  #include <linux/device.h>
> +#include <linux/workqueue.h>
>  
>  #define PM_QOS_RESERVED 0
>  #define PM_QOS_CPU_DMA_LATENCY 1
> @@ -29,6 +30,8 @@
>  struct pm_qos_request {
>  	struct plist_node node;
>  	int pm_qos_class;
> +	s32 value; /* the back-up value for pm_qos_update_request_timeout */

Well, what about "saved_value"?

> +	struct delayed_work work; /* for pm_qos_update_request_timeout */
>  };
>  
>  struct dev_pm_qos_request {
> @@ -74,6 +77,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>  			s32 value);
>  void pm_qos_update_request(struct pm_qos_request *req,
>  			   s32 new_value);
> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> +				   s32 new_value, unsigned long timeout);
>  void pm_qos_remove_request(struct pm_qos_request *req);
>  
>  int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index b15e0b7..acfa433 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>  
>  /**
> + * pm_qos_timeout - the timeout handler of pm_qos_update_request_timeout
> + * @work: work struct for the delayed work (timeout)
> + *
> + * This cancels the timeout request and rolls the request value back.
> + */
> +static void pm_qos_timeout(struct work_struct *work)

I'd call it pm_qos_work_fn(), so that it's clear what it is.

> +{
> +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> +						  struct pm_qos_request,
> +						  work);
> +
> +	pm_qos_update_request(req, req->value);
> +}
> +
> +/**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @req: pointer to a preallocated handle
>   * @pm_qos_class: identifies which list of qos request to use
> @@ -282,6 +297,8 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		return;
>  	}
>  	req->pm_qos_class = pm_qos_class;
> +	req->value = value;
> +	INIT_DELAYED_WORK(&req->work, pm_qos_timeout);
>  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_ADD_REQ, value);
>  }
> @@ -308,14 +325,46 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  		return;
>  	}
>  
> -	if (new_value != req->node.prio)
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +

That may result in setting the target value to an unwanted one temporarily.
Namely, if pm_qos_timeout() is already running, it will switch back to the
"original" target value before the new one is set.  The PM QoS users may see
the "restored" value and use it in their decision making.  Do we care?

> +	req->value = new_value;
> +	if (new_value != req->node.prio) {
>  		pm_qos_update_target(
>  			pm_qos_array[req->pm_qos_class]->constraints,
>  			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +	}

Adding brances here doesn't belong to this patch.

>  }
>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>  
>  /**
> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @new_value: defines the temporal qos request
> + * @timeout: the effective duration of this qos request in jiffies
> + *
> + * After timeout, this qos request is cancelled automatically.
> + */
> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> +				   unsigned long timeout)
> +{
> +	if (!req)
> +		return;
> +	if (WARN(!pm_qos_request_active(req),
> +		 "pm_qos_update_request_timeout() called for unknown object."))

Please use __func__ in messages like this.

> +		return;
> +
> +	if (new_value != req->node.prio) {
> +		pm_qos_update_target(
> +			pm_qos_array[req->pm_qos_class]->constraints,
> +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +		if (delayed_work_pending(&req->work))
> +			cancel_delayed_work_sync(&req->work);

It seems that if pm_qos_timeout() has been started already, it will overwrite
the value that we've just set, or am I missing something?

> +		schedule_delayed_work(&req->work, timeout);
> +	}
> +}
> +
> +/**
>   * pm_qos_remove_request - modifies an existing qos request
>   * @req: handle to request list element
>   *
> @@ -334,6 +383,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
> 

Thanks,
Rafael

Re: [RFC PATCH] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

2012/2/15 Rafael J. Wysocki <rjw@sisk.pl>:
> Hi,
>
> On Tuesday, February 14, 2012, MyungJoo Ham wrote:
>> The new API, pm_qos_update_request_timeout() is to provide a timeout
>> with pm_qos_update_request.
>
> I need to know what the other people in the CC list think about that.  It would
> definitely help if you described a use case.

Use case (current implementation in local repositories) w/ user inputs
(touch screen)

Run DVFS mechanism (both CPUfreq and Devfreq) or devices faster when a
user starts an input.
(default: polling every 100ms. with user inputs: polling every 10ms)
or (depending on projects)
(default: 200MHz ~ 1.5GHz. with user inputs: 800MHz ~ 1.5GHz)

Run DVFS mechanism faster (shorter polling interval) when the
userspace expects a sudden and temporal utilization changes or
fluctuations soon (but the utilization may and may not rise much, so
DVFS mechanism needs to react faster, not to increase frequency
unconditionally.): a new application is being loaded or a bulky task
is being loaded.


>
>> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
>> QoS request on req with value 100 will be active for 1000 jiffies.
>
> Could that be a different time unit instead of jiffies?

Um.. yes.. I guess usec would fit here.

>
[]
>>
>>  #define PM_QOS_RESERVED 0
>>  #define PM_QOS_CPU_DMA_LATENCY 1
>> @@ -29,6 +30,8 @@
>>  struct pm_qos_request {
>>       struct plist_node node;
>>       int pm_qos_class;
>> +     s32 value; /* the back-up value for pm_qos_update_request_timeout */
>
> Well, what about "saved_value"?

Yes.. it looks better.

>
>>  /**
>> + * pm_qos_timeout - the timeout handler of pm_qos_update_request_timeout
>> + * @work: work struct for the delayed work (timeout)
>> + *
>> + * This cancels the timeout request and rolls the request value back.
>> + */
>> +static void pm_qos_timeout(struct work_struct *work)
>
> I'd call it pm_qos_work_fn(), so that it's clear what it is.

Ok.. I'll modify it.. How about pm_qos_timeout_work_fn()?

>
[]
>>
>> -     if (new_value != req->node.prio)
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>
> That may result in setting the target value to an unwanted one temporarily.
> Namely, if pm_qos_timeout() is already running, it will switch back to the
> "original" target value before the new one is set.  The PM QoS users may see
> the "restored" value and use it in their decision making.  Do we care?

If pm_qos_timeout is already running at this point, QoS-value will be
restored and PM-QoS users (handlers) will react based on the restored
value. And then, these PM-QoS users (handlers) will again react to the
new_value below.

Having one additional and unnecessary PM-QoS users/handers' side
reaction is surely an overhead; however, it happens only if
pm_qos_timeout is already running at the point of this function call.
It pm_qos_timeout is awaiting for "timeout", then it will simply be
canceled.

If we do cancel_delayed_work rather than cancel_delayed_work_sync
here, we then may have serialization problem; the value set by
pm_qos_update_request() is overriden by pm_qos_timeout().

The purpose of modification here is to guarantee that the
pm_qos_timeout(), which is roll-back capability of
pm_qos_update_request_timeout(), won't override QoS request values set
by another pm_qos_update_request() calls.

Or, we may need to add mutexes in request-updating functions and let
the functions serialized.

>
>> +     req->value = new_value;
>> +     if (new_value != req->node.prio) {
>>               pm_qos_update_target(
>>                       pm_qos_array[req->pm_qos_class]->constraints,
>>                       &req->node, PM_QOS_UPDATE_REQ, new_value);
>> +     }
>
> Adding brances here doesn't belong to this patch.

Oops.. I'll remove these braces.

>
>>  }
>>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>>
>>  /**
>> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
>> + * @req : handle to list element holding a pm_qos request to use
>> + * @new_value: defines the temporal qos request
>> + * @timeout: the effective duration of this qos request in jiffies
>> + *
>> + * After timeout, this qos request is cancelled automatically.
>> + */
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
>> +                                unsigned long timeout)
>> +{
>> +     if (!req)
>> +             return;
>> +     if (WARN(!pm_qos_request_active(req),
>> +              "pm_qos_update_request_timeout() called for unknown object."))
>
> Please use __func__ in messages like this.

Oh.. yes, I'll. In fact, because it is WARN(), the function name
itself won't be necessary.

>
>> +             return;
>> +
>> +     if (new_value != req->node.prio) {
>> +             pm_qos_update_target(
>> +                     pm_qos_array[req->pm_qos_class]->constraints,
>> +                     &req->node, PM_QOS_UPDATE_REQ, new_value);
>> +             if (delayed_work_pending(&req->work))
>> +                     cancel_delayed_work_sync(&req->work);
>
> It seems that if pm_qos_timeout() has been started already, it will overwrite
> the value that we've just set, or am I missing something?

No, you are not.

I should've put cancel_delayed_work_sync before pm_qos_update_target().



>
>> +             schedule_delayed_work(&req->work, timeout);
>> +     }
>> +}
>> +
>> +/**
>>   * pm_qos_remove_request - modifies an existing qos request
>>   * @req: handle to request list element
>>   *
>> @@ -334,6 +383,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>>               return;
>>       }
>>
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>>       pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_REMOVE_REQ,
>>                            PM_QOS_DEFAULT_VALUE);
>>
>
> Thanks,
> Rafael

Thank you so much!


Cheers!
MyungJoo

-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [RFC PATCH] PM / QoS: add pm_qos_update_request_timeout API

[mark gross]

On Tue, Feb 14, 2012 at 02:16:54PM +0900, MyungJoo Ham wrote:
> The new API, pm_qos_update_request_timeout() is to provide a timeout
> with pm_qos_update_request.
> 
> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> QoS request on req with value 100 will be active for 1000 jiffies.
> After 1000 jiffies, the QoS request thru req is rolled back to the
> request status when pm_qos_update_request_timeout() was called. If there
> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> new request with value x will override as this is another request on the
> same req handle. A new request on the same req handle will always
> override the previous request whether it is the conventional request or
> it is the new timeout request.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
>  include/linux/pm_qos.h |    5 ++++
>  kernel/power/qos.c     |   54 +++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 58 insertions(+), 1 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index f8ccb7b..1c29f52 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -8,6 +8,7 @@
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  #include <linux/device.h>
> +#include <linux/workqueue.h>
>  
>  #define PM_QOS_RESERVED 0
>  #define PM_QOS_CPU_DMA_LATENCY 1
> @@ -29,6 +30,8 @@
>  struct pm_qos_request {
>  	struct plist_node node;
>  	int pm_qos_class;
> +	s32 value; /* the back-up value for pm_qos_update_request_timeout */
Why can't we just drop back to the qos_class default? or auto remove
the time out request from the list? 

> +	struct delayed_work work; /* for pm_qos_update_request_timeout */
>  };
>  
>  struct dev_pm_qos_request {
> @@ -74,6 +77,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>  			s32 value);
>  void pm_qos_update_request(struct pm_qos_request *req,
>  			   s32 new_value);
> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> +				   s32 new_value, unsigned long timeout);
Don't you need 2 values?  one for the request and one for the time out
request target?

I tend to not like time out api's.  They are race conditions waiting to
happen.

The caller could have its own timer that can simply go off and update the
requested qos.

I understand that you need to bump performance up for a short time to get good
interactivity but I don't know if it should be generalized or if this is
a good enough of a generalization.

--mark

>  void pm_qos_remove_request(struct pm_qos_request *req);
>  
>  int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index b15e0b7..acfa433 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>  
>  /**
> + * pm_qos_timeout - the timeout handler of pm_qos_update_request_timeout
> + * @work: work struct for the delayed work (timeout)
> + *
> + * This cancels the timeout request and rolls the request value back.
> + */
> +static void pm_qos_timeout(struct work_struct *work)
> +{
> +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> +						  struct pm_qos_request,
> +						  work);
> +
> +	pm_qos_update_request(req, req->value);
> +}
> +
> +/**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @req: pointer to a preallocated handle
>   * @pm_qos_class: identifies which list of qos request to use
> @@ -282,6 +297,8 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		return;
>  	}
>  	req->pm_qos_class = pm_qos_class;
> +	req->value = value;
> +	INIT_DELAYED_WORK(&req->work, pm_qos_timeout);
>  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_ADD_REQ, value);
>  }
> @@ -308,14 +325,46 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  		return;
>  	}
>  
> -	if (new_value != req->node.prio)
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
> +	req->value = new_value;
> +	if (new_value != req->node.prio) {
>  		pm_qos_update_target(
>  			pm_qos_array[req->pm_qos_class]->constraints,
>  			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +	}
>  }
>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>  
>  /**
> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @new_value: defines the temporal qos request
> + * @timeout: the effective duration of this qos request in jiffies
> + *
> + * After timeout, this qos request is cancelled automatically.
> + */
> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> +				   unsigned long timeout)
> +{
> +	if (!req)
> +		return;
> +	if (WARN(!pm_qos_request_active(req),
> +		 "pm_qos_update_request_timeout() called for unknown object."))
> +		return;
> +
> +	if (new_value != req->node.prio) {
> +		pm_qos_update_target(
> +			pm_qos_array[req->pm_qos_class]->constraints,
> +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +		if (delayed_work_pending(&req->work))
> +			cancel_delayed_work_sync(&req->work);
> +		schedule_delayed_work(&req->work, timeout);
> +	}
> +}
> +
> +/**
>   * pm_qos_remove_request - modifies an existing qos request
>   * @req: handle to request list element
>   *
> @@ -334,6 +383,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
> -- 
> 1.7.4.1
> 

Re: [RFC PATCH] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

On Mon, Feb 20, 2012 at 6:14 AM, mark gross <markgross@thegnar.org> wrote:
> On Tue, Feb 14, 2012 at 02:16:54PM +0900, MyungJoo Ham wrote:
>>
>>  #define PM_QOS_RESERVED 0
>>  #define PM_QOS_CPU_DMA_LATENCY 1
>> @@ -29,6 +30,8 @@
>>  struct pm_qos_request {
>>       struct plist_node node;
>>       int pm_qos_class;
>> +     s32 value; /* the back-up value for pm_qos_update_request_timeout */
> Why can't we just drop back to the qos_class default? or auto remove
> the time out request from the list?

It is to fall back to previously set pm_qos request after a timeout.

If the user did pm_qos_update_request(X) and then
pm_qos_update_request_timeout(Y), then it seems fair to return back to
X after the timeout.

With a single req struct, I intended to allow both non-timeout API and
timeout API at the same time.

>
>> +     struct delayed_work work; /* for pm_qos_update_request_timeout */
>>  };
>>
>>  struct dev_pm_qos_request {
>> @@ -74,6 +77,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>>                       s32 value);
>>  void pm_qos_update_request(struct pm_qos_request *req,
>>                          s32 new_value);
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
>> +                                s32 new_value, unsigned long timeout);
> Don't you need 2 values?  one for the request and one for the time out
> request target?

Ah.... you want pm_qos_update_request_timeout to timeout the recent
QoS request on the same req object.

I intended to allow adding both timeout request and non-time request
on the same req object.


However, it appears that the approach you've imagined is better as the
PM QoS framework has been allowing one request through one req object.

Then, we won't need the backup value described above.

I'll change the semmantics of this API in the next iteration.

>
> I tend to not like time out api's.  They are race conditions waiting to
> happen.
>
> The caller could have its own timer that can simply go off and update the
> requested qos.
>

Yes, we have been doing so and resulted in creating time-out work here
and there all over the places repeatedly. And I hoped this would clear
them off.


> I understand that you need to bump performance up for a short time to get good
> interactivity but I don't know if it should be generalized or if this is
> a good enough of a generalization.
>
> --mark
>

Hm... I thought many mobile devices would need or have been already
using this feature (frequency-up with timeout for input events), but I
guess we'll need more opinions on this.

Thanks so much for the comments.


Cheers!
MyungJoo.

>>  void pm_qos_remove_request(struct pm_qos_request *req);
>>
>>  int pm_qos_request(int pm_qos_class);
>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>> index b15e0b7..acfa433 100644
>> --- a/kernel/power/qos.c
>> +++ b/kernel/power/qos.c
>> @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>>
>>  /**
>> + * pm_qos_timeout - the timeout handler of pm_qos_update_request_timeout
>> + * @work: work struct for the delayed work (timeout)
>> + *
>> + * This cancels the timeout request and rolls the request value back.
>> + */
>> +static void pm_qos_timeout(struct work_struct *work)
>> +{
>> +     struct pm_qos_request *req = container_of(to_delayed_work(work),
>> +                                               struct pm_qos_request,
>> +                                               work);
>> +
>> +     pm_qos_update_request(req, req->value);
>> +}
>> +
>> +/**
>>   * pm_qos_add_request - inserts new qos request into the list
>>   * @req: pointer to a preallocated handle
>>   * @pm_qos_class: identifies which list of qos request to use
>> @@ -282,6 +297,8 @@ void pm_qos_add_request(struct pm_qos_request *req,
>>               return;
>>       }
>>       req->pm_qos_class = pm_qos_class;
>> +     req->value = value;
>> +     INIT_DELAYED_WORK(&req->work, pm_qos_timeout);
>>       pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_ADD_REQ, value);
>>  }
>> @@ -308,14 +325,46 @@ void pm_qos_update_request(struct pm_qos_request *req,
>>               return;
>>       }
>>
>> -     if (new_value != req->node.prio)
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>> +     req->value = new_value;
>> +     if (new_value != req->node.prio) {
>>               pm_qos_update_target(
>>                       pm_qos_array[req->pm_qos_class]->constraints,
>>                       &req->node, PM_QOS_UPDATE_REQ, new_value);
>> +     }
>>  }
>>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>>
>>  /**
>> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
>> + * @req : handle to list element holding a pm_qos request to use
>> + * @new_value: defines the temporal qos request
>> + * @timeout: the effective duration of this qos request in jiffies
>> + *
>> + * After timeout, this qos request is cancelled automatically.
>> + */
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
>> +                                unsigned long timeout)
>> +{
>> +     if (!req)
>> +             return;
>> +     if (WARN(!pm_qos_request_active(req),
>> +              "pm_qos_update_request_timeout() called for unknown object."))
>> +             return;
>> +
>> +     if (new_value != req->node.prio) {
>> +             pm_qos_update_target(
>> +                     pm_qos_array[req->pm_qos_class]->constraints,
>> +                     &req->node, PM_QOS_UPDATE_REQ, new_value);
>> +             if (delayed_work_pending(&req->work))
>> +                     cancel_delayed_work_sync(&req->work);
>> +             schedule_delayed_work(&req->work, timeout);
>> +     }
>> +}
>> +
>> +/**
>>   * pm_qos_remove_request - modifies an existing qos request
>>   * @req: handle to request list element
>>   *
>> @@ -334,6 +383,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>>               return;
>>       }
>>
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>>       pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_REMOVE_REQ,
>>                            PM_QOS_DEFAULT_VALUE);
>> --
>> 1.7.4.1
>>



-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics

[PATCH v2] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

The new API, pm_qos_update_request_timeout() is to provide a timeout
with pm_qos_update_request.

For example, pm_qos_update_request_timeout(req, 100, 1000), means that
QoS request on req with value 100 will be active for 1000 jiffies.
After 1000 jiffies, the QoS request thru req is rolled back to the
request status when pm_qos_update_request_timeout() was called. If there
were another pm_qos_update_request(req, x) during the 1000 jiffies, this
new request with value x will override as this is another request on the
same req handle. A new request on the same req handle will always
override the previous request whether it is the conventional request or
it is the new timeout request.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

---
Chages from v1(RFC)
- The semmantics of timeout changed. A timeout now means that the
  corresponding qos-req object is being cancelled, not rolling back to
the qos request state at the time of API call. With this changes,
combined with the conventional update_request API, there exists up to
only one qos request per qos-req object.
- The timeout values is expressed in ms (1/1000 sec), not jiffies.
- Style changes, removing unnecessary changes.
- Bugfixes regarding pending timeout work.
---
 include/linux/pm_qos.h |    4 +++
 kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index f8ccb7b..0c13a74 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -8,6 +8,7 @@
 #include <linux/notifier.h>
 #include <linux/miscdevice.h>
 #include <linux/device.h>
+#include <linux/workqueue.h>
 
 #define PM_QOS_RESERVED 0
 #define PM_QOS_CPU_DMA_LATENCY 1
@@ -29,6 +30,7 @@
 struct pm_qos_request {
 	struct plist_node node;
 	int pm_qos_class;
+	struct delayed_work work; /* for pm_qos_update_request_timeout */
 };
 
 struct dev_pm_qos_request {
@@ -74,6 +76,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
 			s32 value);
 void pm_qos_update_request(struct pm_qos_request *req,
 			   s32 new_value);
+void pm_qos_update_request_timeout(struct pm_qos_request *req,
+				   s32 new_value, unsigned long timeout_ms);
 void pm_qos_remove_request(struct pm_qos_request *req);
 
 int pm_qos_request(int pm_qos_class);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index c149af3..c07a91f 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
 EXPORT_SYMBOL_GPL(pm_qos_request_active);
 
 /**
+ * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
+ * @work: work struct for the delayed work (timeout)
+ *
+ * This cancels the timeout request by falling back to the default at timeout.
+ */
+static void pm_qos_work_fn(struct work_struct *work)
+{
+	struct pm_qos_request *req = container_of(to_delayed_work(work),
+						  struct pm_qos_request,
+						  work);
+
+	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
+}
+
+/**
  * pm_qos_add_request - inserts new qos request into the list
  * @req: pointer to a preallocated handle
  * @pm_qos_class: identifies which list of qos request to use
@@ -282,6 +297,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		return;
 	}
 	req->pm_qos_class = pm_qos_class;
+	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
 	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
 			     &req->node, PM_QOS_ADD_REQ, value);
 }
@@ -308,6 +324,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	if (new_value != req->node.prio)
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
@@ -316,6 +335,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
 EXPORT_SYMBOL_GPL(pm_qos_update_request);
 
 /**
+ * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
+ * @req : handle to list element holding a pm_qos request to use
+ * @new_value: defines the temporal qos request
+ * @timeout_ms: the effective duration of this qos request in msecs.
+ *
+ * After timeout_ms, this qos request is cancelled automatically.
+ */
+void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
+				   unsigned long timeout_ms)
+{
+	if (!req)
+		return;
+	if (WARN(!pm_qos_request_active(req),
+		 "%s called for unknown object.", __func__))
+		return;
+
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
+	if (new_value != req->node.prio)
+		pm_qos_update_target(
+			pm_qos_array[req->pm_qos_class]->constraints,
+			&req->node, PM_QOS_UPDATE_REQ, new_value);
+
+	schedule_delayed_work(&req->work, msecs_to_jiffies(timeout_ms));
+}
+
+/**
  * pm_qos_remove_request - modifies an existing qos request
  * @req: handle to request list element
  *
@@ -334,6 +381,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
 			     &req->node, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
-- 
1.7.4.1

[PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

The new API, pm_qos_update_request_timeout() is to provide a timeout
with pm_qos_update_request.

For example, pm_qos_update_request_timeout(req, 100, 1000), means that
QoS request on req with value 100 will be active for 1000 jiffies.
After 1000 jiffies, the QoS request thru req is rolled back to the
request status when pm_qos_update_request_timeout() was called. If there
were another pm_qos_update_request(req, x) during the 1000 jiffies, this
new request with value x will override as this is another request on the
same req handle. A new request on the same req handle will always
override the previous request whether it is the conventional request or
it is the new timeout request.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes from v2
- Rebased to avoid merge conflict
Chages from v1(RFC)
- The semmantics of timeout changed. A timeout now means that the
 corresponding qos-req object is being cancelled, not rolling back to
the qos request state at the time of API call. With this changes,
combined with the conventional update_request API, there exists up to
only one qos request per qos-req object.
- The timeout values is expressed in ms (1/1000 sec), not jiffies.
- Style changes, removing unnecessary changes.
- Bugfixes regarding pending timeout work.

---
 include/linux/pm_qos.h |    4 +++
 kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index 0ee7caa..6f347fe 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -8,6 +8,7 @@
 #include <linux/notifier.h>
 #include <linux/miscdevice.h>
 #include <linux/device.h>
+#include <linux/workqueue.h>
 
 enum {
 	PM_QOS_RESERVED = 0,
@@ -33,6 +34,7 @@ enum {
 struct pm_qos_request {
 	struct plist_node node;
 	int pm_qos_class;
+	struct delayed_work work; /* for pm_qos_update_request_timeout */
 };
 
 struct dev_pm_qos_request {
@@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
 			s32 value);
 void pm_qos_update_request(struct pm_qos_request *req,
 			   s32 new_value);
+void pm_qos_update_request_timeout(struct pm_qos_request *req,
+				   s32 new_value, unsigned long timeout_ms);
 void pm_qos_remove_request(struct pm_qos_request *req);
 
 int pm_qos_request(int pm_qos_class);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index 3e122db..8ac8eca 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
 EXPORT_SYMBOL_GPL(pm_qos_request_active);
 
 /**
+ * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
+ * @work: work struct for the delayed work (timeout)
+ *
+ * This cancels the timeout request by falling back to the default at timeout.
+ */
+static void pm_qos_work_fn(struct work_struct *work)
+{
+	struct pm_qos_request *req = container_of(to_delayed_work(work),
+						  struct pm_qos_request,
+						  work);
+
+	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
+}
+
+/**
  * pm_qos_add_request - inserts new qos request into the list
  * @req: pointer to a preallocated handle
  * @pm_qos_class: identifies which list of qos request to use
@@ -282,6 +297,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		return;
 	}
 	req->pm_qos_class = pm_qos_class;
+	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
 	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
 			     &req->node, PM_QOS_ADD_REQ, value);
 }
@@ -308,6 +324,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	if (new_value != req->node.prio)
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
@@ -316,6 +335,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
 EXPORT_SYMBOL_GPL(pm_qos_update_request);
 
 /**
+ * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
+ * @req : handle to list element holding a pm_qos request to use
+ * @new_value: defines the temporal qos request
+ * @timeout_ms: the effective duration of this qos request in msecs.
+ *
+ * After timeout_ms, this qos request is cancelled automatically.
+ */
+void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
+				   unsigned long timeout_ms)
+{
+	if (!req)
+		return;
+	if (WARN(!pm_qos_request_active(req),
+		 "%s called for unknown object.", __func__))
+		return;
+
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
+	if (new_value != req->node.prio)
+		pm_qos_update_target(
+			pm_qos_array[req->pm_qos_class]->constraints,
+			&req->node, PM_QOS_UPDATE_REQ, new_value);
+
+	schedule_delayed_work(&req->work, msecs_to_jiffies(timeout_ms));
+}
+
+/**
  * pm_qos_remove_request - modifies an existing qos request
  * @req: handle to request list element
  *
@@ -334,6 +381,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
 			     &req->node, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
-- 
1.7.4.1

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[mark gross]

I apologize for the lat replay and admit that I was probably wrong to
oppose the idea of time out pm_qos requests.  (last week we bumped into
a need for them and now I get it.)


On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
> The new API, pm_qos_update_request_timeout() is to provide a timeout
> with pm_qos_update_request.
> 
> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> QoS request on req with value 100 will be active for 1000 jiffies.
> After 1000 jiffies, the QoS request thru req is rolled back to the
> request status when pm_qos_update_request_timeout() was called. If there
> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> new request with value x will override as this is another request on the
> same req handle. A new request on the same req handle will always
> override the previous request whether it is the conventional request or
> it is the new timeout request.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> Changes from v2
> - Rebased to avoid merge conflict
> Chages from v1(RFC)
> - The semmantics of timeout changed. A timeout now means that the
>  corresponding qos-req object is being cancelled, not rolling back to
> the qos request state at the time of API call. With this changes,
> combined with the conventional update_request API, there exists up to
> only one qos request per qos-req object.
> - The timeout values is expressed in ms (1/1000 sec), not jiffies.
> - Style changes, removing unnecessary changes.
> - Bugfixes regarding pending timeout work.
> 
> ---
>  include/linux/pm_qos.h |    4 +++
>  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 0ee7caa..6f347fe 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -8,6 +8,7 @@
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  #include <linux/device.h>
> +#include <linux/workqueue.h>
>  
>  enum {
>  	PM_QOS_RESERVED = 0,
> @@ -33,6 +34,7 @@ enum {
>  struct pm_qos_request {
>  	struct plist_node node;
>  	int pm_qos_class;
> +	struct delayed_work work; /* for pm_qos_update_request_timeout */
>  };
>  
>  struct dev_pm_qos_request {
> @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>  			s32 value);
>  void pm_qos_update_request(struct pm_qos_request *req,
>  			   s32 new_value);
> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> +				   s32 new_value, unsigned long timeout_ms);
is ms the right units?  could we ever need us?

>  void pm_qos_remove_request(struct pm_qos_request *req);
>  
>  int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 3e122db..8ac8eca 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>  
>  /**
> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
> + * @work: work struct for the delayed work (timeout)
> + *
> + * This cancels the timeout request by falling back to the default at timeout.
> + */
> +static void pm_qos_work_fn(struct work_struct *work)
> +{
> +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> +						  struct pm_qos_request,
> +						  work);
> +
> +	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
> +}
> +
> +/**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @req: pointer to a preallocated handle
>   * @pm_qos_class: identifies which list of qos request to use
> @@ -282,6 +297,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		return;
>  	}
>  	req->pm_qos_class = pm_qos_class;
> +	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_ADD_REQ, value);
>  }
> @@ -308,6 +324,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	if (new_value != req->node.prio)
>  		pm_qos_update_target(
>  			pm_qos_array[req->pm_qos_class]->constraints,
> @@ -316,6 +335,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>  
>  /**
> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @new_value: defines the temporal qos request
> + * @timeout_ms: the effective duration of this qos request in msecs.
> + *
> + * After timeout_ms, this qos request is cancelled automatically.
> + */
> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> +				   unsigned long timeout_ms)
> +{
> +	if (!req)
> +		return;
> +	if (WARN(!pm_qos_request_active(req),
> +		 "%s called for unknown object.", __func__))
> +		return;
> +
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
> +	if (new_value != req->node.prio)
> +		pm_qos_update_target(
> +			pm_qos_array[req->pm_qos_class]->constraints,
> +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +
> +	schedule_delayed_work(&req->work, msecs_to_jiffies(timeout_ms));
> +}
> +
> +/**
>   * pm_qos_remove_request - modifies an existing qos request
>   * @req: handle to request list element
>   *
> @@ -334,6 +381,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
> -- 
> 1.7.4.1
>

Acked-by: Mark Gross <markgross@thegnar.org>

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[mark gross]

On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
> The new API, pm_qos_update_request_timeout() is to provide a timeout
> with pm_qos_update_request.
> 
> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> QoS request on req with value 100 will be active for 1000 jiffies.
> After 1000 jiffies, the QoS request thru req is rolled back to the
> request status when pm_qos_update_request_timeout() was called. If there
> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> new request with value x will override as this is another request on the
> same req handle. A new request on the same req handle will always
> override the previous request whether it is the conventional request or
> it is the new timeout request.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> Changes from v2
> - Rebased to avoid merge conflict
> Chages from v1(RFC)
> - The semmantics of timeout changed. A timeout now means that the
>  corresponding qos-req object is being cancelled, not rolling back to
> the qos request state at the time of API call. With this changes,
> combined with the conventional update_request API, there exists up to
> only one qos request per qos-req object.
> - The timeout values is expressed in ms (1/1000 sec), not jiffies.
> - Style changes, removing unnecessary changes.
> - Bugfixes regarding pending timeout work.
> 
> ---
>  include/linux/pm_qos.h |    4 +++
>  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 0ee7caa..6f347fe 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -8,6 +8,7 @@
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  #include <linux/device.h>
> +#include <linux/workqueue.h>
>  
>  enum {
>  	PM_QOS_RESERVED = 0,
> @@ -33,6 +34,7 @@ enum {
>  struct pm_qos_request {
>  	struct plist_node node;
>  	int pm_qos_class;
> +	struct delayed_work work; /* for pm_qos_update_request_timeout */
>  };
>  
>  struct dev_pm_qos_request {
> @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>  			s32 value);
>  void pm_qos_update_request(struct pm_qos_request *req,
>  			   s32 new_value);
> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> +				   s32 new_value, unsigned long timeout_ms);
>  void pm_qos_remove_request(struct pm_qos_request *req);
>  
>  int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 3e122db..8ac8eca 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>  
>  /**
> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
> + * @work: work struct for the delayed work (timeout)
> + *
> + * This cancels the timeout request by falling back to the default at timeout.
> + */
> +static void pm_qos_work_fn(struct work_struct *work)
> +{
> +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> +						  struct pm_qos_request,
> +						  work);
> +
> +	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
this should be pm_qos_remove_request (req);

--mark

> +}
> +
> +/**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @req: pointer to a preallocated handle
>   * @pm_qos_class: identifies which list of qos request to use
> @@ -282,6 +297,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		return;
>  	}
>  	req->pm_qos_class = pm_qos_class;
> +	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_ADD_REQ, value);
>  }
> @@ -308,6 +324,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	if (new_value != req->node.prio)
>  		pm_qos_update_target(
>  			pm_qos_array[req->pm_qos_class]->constraints,
> @@ -316,6 +335,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>  
>  /**
> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @new_value: defines the temporal qos request
> + * @timeout_ms: the effective duration of this qos request in msecs.
> + *
> + * After timeout_ms, this qos request is cancelled automatically.
> + */
> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> +				   unsigned long timeout_ms)
> +{
> +	if (!req)
> +		return;
> +	if (WARN(!pm_qos_request_active(req),
> +		 "%s called for unknown object.", __func__))
> +		return;
> +
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
> +	if (new_value != req->node.prio)
> +		pm_qos_update_target(
> +			pm_qos_array[req->pm_qos_class]->constraints,
> +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +
> +	schedule_delayed_work(&req->work, msecs_to_jiffies(timeout_ms));
> +}
> +
> +/**
>   * pm_qos_remove_request - modifies an existing qos request
>   * @req: handle to request list element
>   *
> @@ -334,6 +381,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
> -- 
> 1.7.4.1
> 

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[mark gross]

On Sat, Mar 24, 2012 at 09:41:22AM -0700, mark gross wrote:
> On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
> > The new API, pm_qos_update_request_timeout() is to provide a timeout
> > with pm_qos_update_request.
> > 
> > For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> > QoS request on req with value 100 will be active for 1000 jiffies.
> > After 1000 jiffies, the QoS request thru req is rolled back to the
> > request status when pm_qos_update_request_timeout() was called. If there
> > were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> > new request with value x will override as this is another request on the
> > same req handle. A new request on the same req handle will always
> > override the previous request whether it is the conventional request or
> > it is the new timeout request.
> > 
> > Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > Changes from v2
> > - Rebased to avoid merge conflict
> > Chages from v1(RFC)
> > - The semmantics of timeout changed. A timeout now means that the
> >  corresponding qos-req object is being cancelled, not rolling back to
> > the qos request state at the time of API call. With this changes,
> > combined with the conventional update_request API, there exists up to
> > only one qos request per qos-req object.
> > - The timeout values is expressed in ms (1/1000 sec), not jiffies.
> > - Style changes, removing unnecessary changes.
> > - Bugfixes regarding pending timeout work.
> > 
> > ---
> >  include/linux/pm_qos.h |    4 +++
> >  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 54 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> > index 0ee7caa..6f347fe 100644
> > --- a/include/linux/pm_qos.h
> > +++ b/include/linux/pm_qos.h
> > @@ -8,6 +8,7 @@
> >  #include <linux/notifier.h>
> >  #include <linux/miscdevice.h>
> >  #include <linux/device.h>
> > +#include <linux/workqueue.h>
> >  
> >  enum {
> >  	PM_QOS_RESERVED = 0,
> > @@ -33,6 +34,7 @@ enum {
> >  struct pm_qos_request {
> >  	struct plist_node node;
> >  	int pm_qos_class;
> > +	struct delayed_work work; /* for pm_qos_update_request_timeout */
> >  };
> >  
> >  struct dev_pm_qos_request {
> > @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
> >  			s32 value);
> >  void pm_qos_update_request(struct pm_qos_request *req,
> >  			   s32 new_value);
> > +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> > +				   s32 new_value, unsigned long timeout_ms);
> >  void pm_qos_remove_request(struct pm_qos_request *req);
> >  
> >  int pm_qos_request(int pm_qos_class);
> > diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> > index 3e122db..8ac8eca 100644
> > --- a/kernel/power/qos.c
> > +++ b/kernel/power/qos.c
> > @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
> >  EXPORT_SYMBOL_GPL(pm_qos_request_active);
> >  
> >  /**
> > + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
> > + * @work: work struct for the delayed work (timeout)
> > + *
> > + * This cancels the timeout request by falling back to the default at timeout.
> > + */
> > +static void pm_qos_work_fn(struct work_struct *work)
> > +{
> > +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> > +						  struct pm_qos_request,
> > +						  work);
> > +
> > +	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
> this should be pm_qos_remove_request (req);
> 

on second thought your implementation is right.   pm_qos_remove_request
is wrong.

--mark


> 
> > +}
> > +
> > +/**
> >   * pm_qos_add_request - inserts new qos request into the list
> >   * @req: pointer to a preallocated handle
> >   * @pm_qos_class: identifies which list of qos request to use
> > @@ -282,6 +297,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
> >  		return;
> >  	}
> >  	req->pm_qos_class = pm_qos_class;
> > +	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
> >  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
> >  			     &req->node, PM_QOS_ADD_REQ, value);
> >  }
> > @@ -308,6 +324,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
> >  		return;
> >  	}
> >  
> > +	if (delayed_work_pending(&req->work))
> > +		cancel_delayed_work_sync(&req->work);
> > +
> >  	if (new_value != req->node.prio)
> >  		pm_qos_update_target(
> >  			pm_qos_array[req->pm_qos_class]->constraints,
> > @@ -316,6 +335,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
> >  EXPORT_SYMBOL_GPL(pm_qos_update_request);
> >  
> >  /**
> > + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> > + * @req : handle to list element holding a pm_qos request to use
> > + * @new_value: defines the temporal qos request
> > + * @timeout_ms: the effective duration of this qos request in msecs.
> > + *
> > + * After timeout_ms, this qos request is cancelled automatically.
> > + */
> > +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> > +				   unsigned long timeout_ms)
> > +{
> > +	if (!req)
> > +		return;
> > +	if (WARN(!pm_qos_request_active(req),
> > +		 "%s called for unknown object.", __func__))
> > +		return;
> > +
> > +	if (delayed_work_pending(&req->work))
> > +		cancel_delayed_work_sync(&req->work);
> > +
> > +	if (new_value != req->node.prio)
> > +		pm_qos_update_target(
> > +			pm_qos_array[req->pm_qos_class]->constraints,
> > +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> > +
> > +	schedule_delayed_work(&req->work, msecs_to_jiffies(timeout_ms));
> > +}
> > +
> > +/**
> >   * pm_qos_remove_request - modifies an existing qos request
> >   * @req: handle to request list element
> >   *
> > @@ -334,6 +381,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
> >  		return;
> >  	}
> >  
> > +	if (delayed_work_pending(&req->work))
> > +		cancel_delayed_work_sync(&req->work);
> > +
> >  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
> >  			     &req->node, PM_QOS_REMOVE_REQ,
> >  			     PM_QOS_DEFAULT_VALUE);
> > -- 
> > 1.7.4.1
> > 

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

On Sun, Mar 25, 2012 at 1:35 AM, mark gross <markgross@thegnar.org> wrote:
> I apologize for the lat replay and admit that I was probably wrong to
> oppose the idea of time out pm_qos requests.  (last week we bumped into
> a need for them and now I get it.)
>
>
> On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
>> The new API, pm_qos_update_request_timeout() is to provide a timeout
>> with pm_qos_update_request.
>>
>> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
>> QoS request on req with value 100 will be active for 1000 jiffies.
>> After 1000 jiffies, the QoS request thru req is rolled back to the
>> request status when pm_qos_update_request_timeout() was called. If there
>> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
>> new request with value x will override as this is another request on the
>> same req handle. A new request on the same req handle will always
>> override the previous request whether it is the conventional request or
>> it is the new timeout request.
>>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
[]
>> @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>>                       s32 value);
>>  void pm_qos_update_request(struct pm_qos_request *req,
>>                          s32 new_value);
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
>> +                                s32 new_value, unsigned long timeout_ms);
> is ms the right units?  could we ever need us?
>

Because jiffies are used for scheduling tasks, I thought ms should be
fine and having some devices running fast for some msecs longer won't
hurt. However, do you expect scheduling tasks or jiffies may use usecs
later? I don't mind using usecs instead of msecs here; thus, I'll
update this to use usecs. I'll resend patchset soon.


Thanks.

Cheers!
MyungJoo.

>>  void pm_qos_remove_request(struct pm_qos_request *req);
>>
>>  int pm_qos_request(int pm_qos_class);
>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>> index 3e122db..8ac8eca 100644
>> --- a/kernel/power/qos.c
>> +++ b/kernel/power/qos.c
>> @@ -259,6 +259,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>>
>>  /**
>> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
>> + * @work: work struct for the delayed work (timeout)
>> + *
>> + * This cancels the timeout request by falling back to the default at timeout.
>> + */
>> +static void pm_qos_work_fn(struct work_struct *work)
>> +{
>> +     struct pm_qos_request *req = container_of(to_delayed_work(work),
>> +                                               struct pm_qos_request,
>> +                                               work);
>> +
>> +     pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
>> +}
>> +
>> +/**
>>   * pm_qos_add_request - inserts new qos request into the list
>>   * @req: pointer to a preallocated handle
>>   * @pm_qos_class: identifies which list of qos request to use
>> @@ -282,6 +297,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
>>               return;
>>       }
>>       req->pm_qos_class = pm_qos_class;
>> +     INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>>       pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_ADD_REQ, value);
>>  }
>> @@ -308,6 +324,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
>>               return;
>>       }
>>
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>>       if (new_value != req->node.prio)
>>               pm_qos_update_target(
>>                       pm_qos_array[req->pm_qos_class]->constraints,
>> @@ -316,6 +335,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
>>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>>
>>  /**
>> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
>> + * @req : handle to list element holding a pm_qos request to use
>> + * @new_value: defines the temporal qos request
>> + * @timeout_ms: the effective duration of this qos request in msecs.
>> + *
>> + * After timeout_ms, this qos request is cancelled automatically.
>> + */
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
>> +                                unsigned long timeout_ms)
>> +{
>> +     if (!req)
>> +             return;
>> +     if (WARN(!pm_qos_request_active(req),
>> +              "%s called for unknown object.", __func__))
>> +             return;
>> +
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>> +     if (new_value != req->node.prio)
>> +             pm_qos_update_target(
>> +                     pm_qos_array[req->pm_qos_class]->constraints,
>> +                     &req->node, PM_QOS_UPDATE_REQ, new_value);
>> +
>> +     schedule_delayed_work(&req->work, msecs_to_jiffies(timeout_ms));
>> +}
>> +
>> +/**
>>   * pm_qos_remove_request - modifies an existing qos request
>>   * @req: handle to request list element
>>   *
>> @@ -334,6 +381,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>>               return;
>>       }
>>
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>>       pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_REMOVE_REQ,
>>                            PM_QOS_DEFAULT_VALUE);
>> --
>> 1.7.4.1
>>
>
> Acked-by: Mark Gross <markgross@thegnar.org>
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[mark gross]

On Mon, Mar 26, 2012 at 10:41:15AM +0900, MyungJoo Ham wrote:
> On Sun, Mar 25, 2012 at 1:35 AM, mark gross <markgross@thegnar.org> wrote:
> > I apologize for the lat replay and admit that I was probably wrong to
> > oppose the idea of time out pm_qos requests.  (last week we bumped into
> > a need for them and now I get it.)
> >
> >
> > On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
> >> The new API, pm_qos_update_request_timeout() is to provide a timeout
> >> with pm_qos_update_request.
> >>
> >> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> >> QoS request on req with value 100 will be active for 1000 jiffies.
> >> After 1000 jiffies, the QoS request thru req is rolled back to the
> >> request status when pm_qos_update_request_timeout() was called. If there
> >> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> >> new request with value x will override as this is another request on the
> >> same req handle. A new request on the same req handle will always
> >> override the previous request whether it is the conventional request or
> >> it is the new timeout request.
> >>
> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> []
> >> @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
> >>                       s32 value);
> >>  void pm_qos_update_request(struct pm_qos_request *req,
> >>                          s32 new_value);
> >> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> >> +                                s32 new_value, unsigned long timeout_ms);
> > is ms the right units?  could we ever need us?
> >
> 
> Because jiffies are used for scheduling tasks, I thought ms should be
> fine and having some devices running fast for some msecs longer won't
> hurt. However, do you expect scheduling tasks or jiffies may use usecs
> later? I don't mind using usecs instead of msecs here; thus, I'll
> update this to use usecs. I'll resend patchset soon.
> 

I am just asking a question.  I'm not sure if us or ms are the better
units off the top of my head.

--mark

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

On Mon, Mar 26, 2012 at 12:02 PM, mark gross <markgross@thegnar.org> wrote:
> On Mon, Mar 26, 2012 at 10:41:15AM +0900, MyungJoo Ham wrote:
>> On Sun, Mar 25, 2012 at 1:35 AM, mark gross <markgross@thegnar.org> wrote:
>> > I apologize for the lat replay and admit that I was probably wrong to
>> > oppose the idea of time out pm_qos requests.  (last week we bumped into
>> > a need for them and now I get it.)
>> >
>> >
>> > On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
>> >> The new API, pm_qos_update_request_timeout() is to provide a timeout
>> >> with pm_qos_update_request.
>> >>
>> >> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
>> >> QoS request on req with value 100 will be active for 1000 jiffies.
>> >> After 1000 jiffies, the QoS request thru req is rolled back to the
>> >> request status when pm_qos_update_request_timeout() was called. If there
>> >> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
>> >> new request with value x will override as this is another request on the
>> >> same req handle. A new request on the same req handle will always
>> >> override the previous request whether it is the conventional request or
>> >> it is the new timeout request.
>> >>
>> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> []
>> >> @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>> >>                       s32 value);
>> >>  void pm_qos_update_request(struct pm_qos_request *req,
>> >>                          s32 new_value);
>> >> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
>> >> +                                s32 new_value, unsigned long timeout_ms);
>> > is ms the right units?  could we ever need us?
>> >
>>
>> Because jiffies are used for scheduling tasks, I thought ms should be
>> fine and having some devices running fast for some msecs longer won't
>> hurt. However, do you expect scheduling tasks or jiffies may use usecs
>> later? I don't mind using usecs instead of msecs here; thus, I'll
>> update this to use usecs. I'll resend patchset soon.
>>
>
> I am just asking a question.  I'm not sure if us or ms are the better
> units off the top of my head.
>
> --mark
>


For the current structure of Linux (scheduling task, jiffies), I
thought that none of the two is better than the another because the
duration should not be so long and the jiffies are larger than 1 ms.

However, it turns out to be not true: some uses jiffies < 1ms (alpha),
some uses jiffies not cleanly dividable with msecs (omap). Thus, it
appears that usecs is better. Thus, the parameter should be usecs
rather than msecs.



Cheers!
MyungJoo.



-- 
MyungJoo Ham, Ph.D.
System S/W Lab, S/W Center, Samsung Electronics

Re: [PATCH v3] PM / QoS: add pm_qos_update_request_timeout API

[Rafael J. Wysocki]

On Monday, March 26, 2012, MyungJoo Ham wrote:
> On Mon, Mar 26, 2012 at 12:02 PM, mark gross <markgross@thegnar.org> wrote:
> > On Mon, Mar 26, 2012 at 10:41:15AM +0900, MyungJoo Ham wrote:
> >> On Sun, Mar 25, 2012 at 1:35 AM, mark gross <markgross@thegnar.org> wrote:
> >> > I apologize for the lat replay and admit that I was probably wrong to
> >> > oppose the idea of time out pm_qos requests.  (last week we bumped into
> >> > a need for them and now I get it.)
> >> >
> >> >
> >> > On Wed, Mar 07, 2012 at 02:06:18PM +0900, MyungJoo Ham wrote:
> >> >> The new API, pm_qos_update_request_timeout() is to provide a timeout
> >> >> with pm_qos_update_request.
> >> >>
> >> >> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> >> >> QoS request on req with value 100 will be active for 1000 jiffies.
> >> >> After 1000 jiffies, the QoS request thru req is rolled back to the
> >> >> request status when pm_qos_update_request_timeout() was called. If there
> >> >> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> >> >> new request with value x will override as this is another request on the
> >> >> same req handle. A new request on the same req handle will always
> >> >> override the previous request whether it is the conventional request or
> >> >> it is the new timeout request.
> >> >>
> >> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> >> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> []
> >> >> @@ -77,6 +79,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
> >> >>                       s32 value);
> >> >>  void pm_qos_update_request(struct pm_qos_request *req,
> >> >>                          s32 new_value);
> >> >> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> >> >> +                                s32 new_value, unsigned long timeout_ms);
> >> > is ms the right units?  could we ever need us?
> >> >
> >>
> >> Because jiffies are used for scheduling tasks, I thought ms should be
> >> fine and having some devices running fast for some msecs longer won't
> >> hurt. However, do you expect scheduling tasks or jiffies may use usecs
> >> later? I don't mind using usecs instead of msecs here; thus, I'll
> >> update this to use usecs. I'll resend patchset soon.
> >>
> >
> > I am just asking a question.  I'm not sure if us or ms are the better
> > units off the top of my head.
> >
> > --mark
> >
> 
> 
> For the current structure of Linux (scheduling task, jiffies), I
> thought that none of the two is better than the another because the
> duration should not be so long and the jiffies are larger than 1 ms.
> 
> However, it turns out to be not true: some uses jiffies < 1ms (alpha),
> some uses jiffies not cleanly dividable with msecs (omap). Thus, it
> appears that usecs is better. Thus, the parameter should be usecs
> rather than msecs.

Yes, usecs are generally better, although you may consider using nsecs too.

Thanks,
Rafael

[PATCH v4] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

The new API, pm_qos_update_request_timeout() is to provide a timeout
with pm_qos_update_request.

For example, pm_qos_update_request_timeout(req, 100, 1000), means that
QoS request on req with value 100 will be active for 1000 jiffies.
After 1000 jiffies, the QoS request thru req is rolled back to the
request status when pm_qos_update_request_timeout() was called. If there
were another pm_qos_update_request(req, x) during the 1000 jiffies, this
new request with value x will override as this is another request on the
same req handle. A new request on the same req handle will always
override the previous request whether it is the conventional request or
it is the new timeout request.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Mark Gross <markgross@thegnar.org>

--
Changes from v3
- The timeout value is expresses in us (1/1000000 sec), not ms (1/1000 sec).
Changes from v2
- Rebased to avoid merge conflict
Chages from v1(RFC)
- The semmantics of timeout changed. A timeout now means that the
 corresponding qos-req object is being cancelled, not rolling back to
the qos request state at the time of API call. With this changes,
combined with the conventional update_request API, there exists up to
only one qos request per qos-req object.
- The timeout value is expressed in ms (1/1000 sec), not jiffies.
- Style changes, removing unnecessary changes.
- Bugfixes regarding pending timeout work.
---
 include/linux/pm_qos.h |    4 +++
 kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index c8a541e..18689737 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -8,6 +8,7 @@
 #include <linux/notifier.h>
 #include <linux/miscdevice.h>
 #include <linux/device.h>
+#include <linux/workqueue.h>
 
 enum {
 	PM_QOS_RESERVED = 0,
@@ -29,6 +30,7 @@ enum {
 struct pm_qos_request {
 	struct plist_node node;
 	int pm_qos_class;
+	struct delayed_work work; /* for pm_qos_update_request_timeout */
 };
 
 struct dev_pm_qos_request {
@@ -73,6 +75,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
 			s32 value);
 void pm_qos_update_request(struct pm_qos_request *req,
 			   s32 new_value);
+void pm_qos_update_request_timeout(struct pm_qos_request *req,
+				   s32 new_value, unsigned long timeout_us);
 void pm_qos_remove_request(struct pm_qos_request *req);
 
 int pm_qos_request(int pm_qos_class);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index d6d6dbd..6a031e6 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -230,6 +230,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
 EXPORT_SYMBOL_GPL(pm_qos_request_active);
 
 /**
+ * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
+ * @work: work struct for the delayed work (timeout)
+ *
+ * This cancels the timeout request by falling back to the default at timeout.
+ */
+static void pm_qos_work_fn(struct work_struct *work)
+{
+	struct pm_qos_request *req = container_of(to_delayed_work(work),
+						  struct pm_qos_request,
+						  work);
+
+	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
+}
+
+/**
  * pm_qos_add_request - inserts new qos request into the list
  * @req: pointer to a preallocated handle
  * @pm_qos_class: identifies which list of qos request to use
@@ -253,6 +268,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		return;
 	}
 	req->pm_qos_class = pm_qos_class;
+	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
 	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
 			     &req->node, PM_QOS_ADD_REQ, value);
 }
@@ -279,6 +295,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	if (new_value != req->node.prio)
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
@@ -287,6 +306,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
 EXPORT_SYMBOL_GPL(pm_qos_update_request);
 
 /**
+ * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
+ * @req : handle to list element holding a pm_qos request to use
+ * @new_value: defines the temporal qos request
+ * @timeout_us: the effective duration of this qos request in usecs.
+ *
+ * After timeout_us, this qos request is cancelled automatically.
+ */
+void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
+				   unsigned long timeout_us)
+{
+	if (!req)
+		return;
+	if (WARN(!pm_qos_request_active(req),
+		 "%s called for unknown object.", __func__))
+		return;
+
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
+	if (new_value != req->node.prio)
+		pm_qos_update_target(
+			pm_qos_array[req->pm_qos_class]->constraints,
+			&req->node, PM_QOS_UPDATE_REQ, new_value);
+
+	schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
+}
+
+/**
  * pm_qos_remove_request - modifies an existing qos request
  * @req: handle to request list element
  *
@@ -305,6 +352,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
 			     &req->node, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
-- 
1.7.4.1

Re: [PATCH v4] PM / QoS: add pm_qos_update_request_timeout API

[Rafael J. Wysocki]

On Tuesday, March 27, 2012, MyungJoo Ham wrote:
> The new API, pm_qos_update_request_timeout() is to provide a timeout
> with pm_qos_update_request.
> 
> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> QoS request on req with value 100 will be active for 1000 jiffies.
> After 1000 jiffies, the QoS request thru req is rolled back to the
> request status when pm_qos_update_request_timeout() was called. If there
> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> new request with value x will override as this is another request on the
> same req handle.

Can you please update the above part of the changelog to reflect the
fact that microseconds are the new timeout units?

> A new request on the same req handle will always
> override the previous request whether it is the conventional request or
> it is the new timeout request.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Mark Gross <markgross@thegnar.org>

It is kind of too late for the 3.4 merge window, so I'd prefer to keep that
for the v3.5 cycle, unless you urgently need it in v3.4, in which case please
let me know.

Thanks,
Rafael


> --
> Changes from v3
> - The timeout value is expresses in us (1/1000000 sec), not ms (1/1000 sec).
> Changes from v2
> - Rebased to avoid merge conflict
> Chages from v1(RFC)
> - The semmantics of timeout changed. A timeout now means that the
>  corresponding qos-req object is being cancelled, not rolling back to
> the qos request state at the time of API call. With this changes,
> combined with the conventional update_request API, there exists up to
> only one qos request per qos-req object.
> - The timeout value is expressed in ms (1/1000 sec), not jiffies.
> - Style changes, removing unnecessary changes.
> - Bugfixes regarding pending timeout work.
> ---
>  include/linux/pm_qos.h |    4 +++
>  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index c8a541e..18689737 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -8,6 +8,7 @@
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  #include <linux/device.h>
> +#include <linux/workqueue.h>
>  
>  enum {
>  	PM_QOS_RESERVED = 0,
> @@ -29,6 +30,7 @@ enum {
>  struct pm_qos_request {
>  	struct plist_node node;
>  	int pm_qos_class;
> +	struct delayed_work work; /* for pm_qos_update_request_timeout */
>  };
>  
>  struct dev_pm_qos_request {
> @@ -73,6 +75,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>  			s32 value);
>  void pm_qos_update_request(struct pm_qos_request *req,
>  			   s32 new_value);
> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> +				   s32 new_value, unsigned long timeout_us);
>  void pm_qos_remove_request(struct pm_qos_request *req);
>  
>  int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index d6d6dbd..6a031e6 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -230,6 +230,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>  
>  /**
> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
> + * @work: work struct for the delayed work (timeout)
> + *
> + * This cancels the timeout request by falling back to the default at timeout.
> + */
> +static void pm_qos_work_fn(struct work_struct *work)
> +{
> +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> +						  struct pm_qos_request,
> +						  work);
> +
> +	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
> +}
> +
> +/**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @req: pointer to a preallocated handle
>   * @pm_qos_class: identifies which list of qos request to use
> @@ -253,6 +268,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		return;
>  	}
>  	req->pm_qos_class = pm_qos_class;
> +	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_ADD_REQ, value);
>  }
> @@ -279,6 +295,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	if (new_value != req->node.prio)
>  		pm_qos_update_target(
>  			pm_qos_array[req->pm_qos_class]->constraints,
> @@ -287,6 +306,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>  
>  /**
> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @new_value: defines the temporal qos request
> + * @timeout_us: the effective duration of this qos request in usecs.
> + *
> + * After timeout_us, this qos request is cancelled automatically.
> + */
> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> +				   unsigned long timeout_us)
> +{
> +	if (!req)
> +		return;
> +	if (WARN(!pm_qos_request_active(req),
> +		 "%s called for unknown object.", __func__))
> +		return;
> +
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
> +	if (new_value != req->node.prio)
> +		pm_qos_update_target(
> +			pm_qos_array[req->pm_qos_class]->constraints,
> +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +
> +	schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
> +}
> +
> +/**
>   * pm_qos_remove_request - modifies an existing qos request
>   * @req: handle to request list element
>   *
> @@ -305,6 +352,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
> 

[PATCH v4 resend] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

The new API, pm_qos_update_request_timeout() is to provide a timeout
with pm_qos_update_request.

For example, pm_qos_update_request_timeout(req, 100, 1000), means that
QoS request on req with value 100 will be active for 1000 microseconds.
After 1000 microseconds, the QoS request thru req is reset. If there
were another pm_qos_update_request(req, x) during the 1000 us, this
new request with value x will override as this is another request on the
same req handle. A new request on the same req handle will always
override the previous request whether it is the conventional request or
it is the new timeout request.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Mark Gross <markgross@thegnar.org>

--
Resend v4
- Corrected changelog
Changes from v3
- The timeout value is expresses in us (1/1000000 sec), not ms (1/1000 sec).
Changes from v2
- Rebased to avoid merge conflict
Chages from v1(RFC)
- The semmantics of timeout changed. A timeout now means that the
 corresponding qos-req object is being cancelled, not rolling back to
the qos request state at the time of API call. With this changes,
combined with the conventional update_request API, there exists up to
only one qos request per qos-req object.
- The timeout value is expressed in ms (1/1000 sec), not jiffies.
- Style changes, removing unnecessary changes.
- Bugfixes regarding pending timeout work.
---
 include/linux/pm_qos.h |    4 +++
 kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
index c8a541e..18689737 100644
--- a/include/linux/pm_qos.h
+++ b/include/linux/pm_qos.h
@@ -8,6 +8,7 @@
 #include <linux/notifier.h>
 #include <linux/miscdevice.h>
 #include <linux/device.h>
+#include <linux/workqueue.h>
 
 enum {
 	PM_QOS_RESERVED = 0,
@@ -29,6 +30,7 @@ enum {
 struct pm_qos_request {
 	struct plist_node node;
 	int pm_qos_class;
+	struct delayed_work work; /* for pm_qos_update_request_timeout */
 };
 
 struct dev_pm_qos_request {
@@ -73,6 +75,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
 			s32 value);
 void pm_qos_update_request(struct pm_qos_request *req,
 			   s32 new_value);
+void pm_qos_update_request_timeout(struct pm_qos_request *req,
+				   s32 new_value, unsigned long timeout_us);
 void pm_qos_remove_request(struct pm_qos_request *req);
 
 int pm_qos_request(int pm_qos_class);
diff --git a/kernel/power/qos.c b/kernel/power/qos.c
index d6d6dbd..6a031e6 100644
--- a/kernel/power/qos.c
+++ b/kernel/power/qos.c
@@ -230,6 +230,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
 EXPORT_SYMBOL_GPL(pm_qos_request_active);
 
 /**
+ * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
+ * @work: work struct for the delayed work (timeout)
+ *
+ * This cancels the timeout request by falling back to the default at timeout.
+ */
+static void pm_qos_work_fn(struct work_struct *work)
+{
+	struct pm_qos_request *req = container_of(to_delayed_work(work),
+						  struct pm_qos_request,
+						  work);
+
+	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
+}
+
+/**
  * pm_qos_add_request - inserts new qos request into the list
  * @req: pointer to a preallocated handle
  * @pm_qos_class: identifies which list of qos request to use
@@ -253,6 +268,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
 		return;
 	}
 	req->pm_qos_class = pm_qos_class;
+	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
 	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
 			     &req->node, PM_QOS_ADD_REQ, value);
 }
@@ -279,6 +295,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	if (new_value != req->node.prio)
 		pm_qos_update_target(
 			pm_qos_array[req->pm_qos_class]->constraints,
@@ -287,6 +306,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
 EXPORT_SYMBOL_GPL(pm_qos_update_request);
 
 /**
+ * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
+ * @req : handle to list element holding a pm_qos request to use
+ * @new_value: defines the temporal qos request
+ * @timeout_us: the effective duration of this qos request in usecs.
+ *
+ * After timeout_us, this qos request is cancelled automatically.
+ */
+void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
+				   unsigned long timeout_us)
+{
+	if (!req)
+		return;
+	if (WARN(!pm_qos_request_active(req),
+		 "%s called for unknown object.", __func__))
+		return;
+
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
+	if (new_value != req->node.prio)
+		pm_qos_update_target(
+			pm_qos_array[req->pm_qos_class]->constraints,
+			&req->node, PM_QOS_UPDATE_REQ, new_value);
+
+	schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
+}
+
+/**
  * pm_qos_remove_request - modifies an existing qos request
  * @req: handle to request list element
  *
@@ -305,6 +352,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
 		return;
 	}
 
+	if (delayed_work_pending(&req->work))
+		cancel_delayed_work_sync(&req->work);
+
 	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
 			     &req->node, PM_QOS_REMOVE_REQ,
 			     PM_QOS_DEFAULT_VALUE);
-- 
1.7.4.1

Re: [PATCH v4] PM / QoS: add pm_qos_update_request_timeout API

[MyungJoo Ham]

2012/3/28 Rafael J. Wysocki <rjw@sisk.pl>:
> On Tuesday, March 27, 2012, MyungJoo Ham wrote:
>> The new API, pm_qos_update_request_timeout() is to provide a timeout
>> with pm_qos_update_request.
>>
>> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
>> QoS request on req with value 100 will be active for 1000 jiffies.
>> After 1000 jiffies, the QoS request thru req is rolled back to the
>> request status when pm_qos_update_request_timeout() was called. If there
>> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
>> new request with value x will override as this is another request on the
>> same req handle.
>
> Can you please update the above part of the changelog to reflect the
> fact that microseconds are the new timeout units?

Sure, I've just sent another.

>
>> A new request on the same req handle will always
>> override the previous request whether it is the conventional request or
>> it is the new timeout request.
>>
>> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
>> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>> Acked-by: Mark Gross <markgross@thegnar.org>
>
> It is kind of too late for the 3.4 merge window, so I'd prefer to keep that
> for the v3.5 cycle, unless you urgently need it in v3.4, in which case please
> let me know.
>
> Thanks,
> Rafael
>

Could you please put it with 3.4 if possible? We want to use this
feature with 3.4.

Thanks a lot.


Cheers!
MyungJoo.

>
>> --
>> Changes from v3
>> - The timeout value is expresses in us (1/1000000 sec), not ms (1/1000 sec).
>> Changes from v2
>> - Rebased to avoid merge conflict
>> Chages from v1(RFC)
>> - The semmantics of timeout changed. A timeout now means that the
>>  corresponding qos-req object is being cancelled, not rolling back to
>> the qos request state at the time of API call. With this changes,
>> combined with the conventional update_request API, there exists up to
>> only one qos request per qos-req object.
>> - The timeout value is expressed in ms (1/1000 sec), not jiffies.
>> - Style changes, removing unnecessary changes.
>> - Bugfixes regarding pending timeout work.
>> ---
>>  include/linux/pm_qos.h |    4 +++
>>  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 54 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>> index c8a541e..18689737 100644
>> --- a/include/linux/pm_qos.h
>> +++ b/include/linux/pm_qos.h
>> @@ -8,6 +8,7 @@
>>  #include <linux/notifier.h>
>>  #include <linux/miscdevice.h>
>>  #include <linux/device.h>
>> +#include <linux/workqueue.h>
>>
>>  enum {
>>       PM_QOS_RESERVED = 0,
>> @@ -29,6 +30,7 @@ enum {
>>  struct pm_qos_request {
>>       struct plist_node node;
>>       int pm_qos_class;
>> +     struct delayed_work work; /* for pm_qos_update_request_timeout */
>>  };
>>
>>  struct dev_pm_qos_request {
>> @@ -73,6 +75,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>>                       s32 value);
>>  void pm_qos_update_request(struct pm_qos_request *req,
>>                          s32 new_value);
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
>> +                                s32 new_value, unsigned long timeout_us);
>>  void pm_qos_remove_request(struct pm_qos_request *req);
>>
>>  int pm_qos_request(int pm_qos_class);
>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>> index d6d6dbd..6a031e6 100644
>> --- a/kernel/power/qos.c
>> +++ b/kernel/power/qos.c
>> @@ -230,6 +230,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>>
>>  /**
>> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
>> + * @work: work struct for the delayed work (timeout)
>> + *
>> + * This cancels the timeout request by falling back to the default at timeout.
>> + */
>> +static void pm_qos_work_fn(struct work_struct *work)
>> +{
>> +     struct pm_qos_request *req = container_of(to_delayed_work(work),
>> +                                               struct pm_qos_request,
>> +                                               work);
>> +
>> +     pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
>> +}
>> +
>> +/**
>>   * pm_qos_add_request - inserts new qos request into the list
>>   * @req: pointer to a preallocated handle
>>   * @pm_qos_class: identifies which list of qos request to use
>> @@ -253,6 +268,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
>>               return;
>>       }
>>       req->pm_qos_class = pm_qos_class;
>> +     INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>>       pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_ADD_REQ, value);
>>  }
>> @@ -279,6 +295,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
>>               return;
>>       }
>>
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>>       if (new_value != req->node.prio)
>>               pm_qos_update_target(
>>                       pm_qos_array[req->pm_qos_class]->constraints,
>> @@ -287,6 +306,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
>>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>>
>>  /**
>> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
>> + * @req : handle to list element holding a pm_qos request to use
>> + * @new_value: defines the temporal qos request
>> + * @timeout_us: the effective duration of this qos request in usecs.
>> + *
>> + * After timeout_us, this qos request is cancelled automatically.
>> + */
>> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
>> +                                unsigned long timeout_us)
>> +{
>> +     if (!req)
>> +             return;
>> +     if (WARN(!pm_qos_request_active(req),
>> +              "%s called for unknown object.", __func__))
>> +             return;
>> +
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>> +     if (new_value != req->node.prio)
>> +             pm_qos_update_target(
>> +                     pm_qos_array[req->pm_qos_class]->constraints,
>> +                     &req->node, PM_QOS_UPDATE_REQ, new_value);
>> +
>> +     schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
>> +}
>> +
>> +/**
>>   * pm_qos_remove_request - modifies an existing qos request
>>   * @req: handle to request list element
>>   *
>> @@ -305,6 +352,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>>               return;
>>       }
>>
>> +     if (delayed_work_pending(&req->work))
>> +             cancel_delayed_work_sync(&req->work);
>> +
>>       pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>>                            &req->node, PM_QOS_REMOVE_REQ,
>>                            PM_QOS_DEFAULT_VALUE);
>>
>



-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics

Re: [PATCH v4] PM / QoS: add pm_qos_update_request_timeout API

[Rafael J. Wysocki]

On Wednesday, March 28, 2012, MyungJoo Ham wrote:
> 2012/3/28 Rafael J. Wysocki <rjw@sisk.pl>:
> > On Tuesday, March 27, 2012, MyungJoo Ham wrote:
> >> The new API, pm_qos_update_request_timeout() is to provide a timeout
> >> with pm_qos_update_request.
> >>
> >> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> >> QoS request on req with value 100 will be active for 1000 jiffies.
> >> After 1000 jiffies, the QoS request thru req is rolled back to the
> >> request status when pm_qos_update_request_timeout() was called. If there
> >> were another pm_qos_update_request(req, x) during the 1000 jiffies, this
> >> new request with value x will override as this is another request on the
> >> same req handle.
> >
> > Can you please update the above part of the changelog to reflect the
> > fact that microseconds are the new timeout units?
> 
> Sure, I've just sent another.
> 
> >
> >> A new request on the same req handle will always
> >> override the previous request whether it is the conventional request or
> >> it is the new timeout request.
> >>
> >> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> >> Acked-by: Mark Gross <markgross@thegnar.org>
> >
> > It is kind of too late for the 3.4 merge window, so I'd prefer to keep that
> > for the v3.5 cycle, unless you urgently need it in v3.4, in which case please
> > let me know.
> >
> > Thanks,
> > Rafael
> >
> 
> Could you please put it with 3.4 if possible? We want to use this
> feature with 3.4.

Can you please give any details, if possible?

Rafael


> >> --
> >> Changes from v3
> >> - The timeout value is expresses in us (1/1000000 sec), not ms (1/1000 sec).
> >> Changes from v2
> >> - Rebased to avoid merge conflict
> >> Chages from v1(RFC)
> >> - The semmantics of timeout changed. A timeout now means that the
> >>  corresponding qos-req object is being cancelled, not rolling back to
> >> the qos request state at the time of API call. With this changes,
> >> combined with the conventional update_request API, there exists up to
> >> only one qos request per qos-req object.
> >> - The timeout value is expressed in ms (1/1000 sec), not jiffies.
> >> - Style changes, removing unnecessary changes.
> >> - Bugfixes regarding pending timeout work.
> >> ---
> >>  include/linux/pm_qos.h |    4 +++
> >>  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
> >>  2 files changed, 54 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> >> index c8a541e..18689737 100644
> >> --- a/include/linux/pm_qos.h
> >> +++ b/include/linux/pm_qos.h
> >> @@ -8,6 +8,7 @@
> >>  #include <linux/notifier.h>
> >>  #include <linux/miscdevice.h>
> >>  #include <linux/device.h>
> >> +#include <linux/workqueue.h>
> >>
> >>  enum {
> >>       PM_QOS_RESERVED = 0,
> >> @@ -29,6 +30,7 @@ enum {
> >>  struct pm_qos_request {
> >>       struct plist_node node;
> >>       int pm_qos_class;
> >> +     struct delayed_work work; /* for pm_qos_update_request_timeout */
> >>  };
> >>
> >>  struct dev_pm_qos_request {
> >> @@ -73,6 +75,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
> >>                       s32 value);
> >>  void pm_qos_update_request(struct pm_qos_request *req,
> >>                          s32 new_value);
> >> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> >> +                                s32 new_value, unsigned long timeout_us);
> >>  void pm_qos_remove_request(struct pm_qos_request *req);
> >>
> >>  int pm_qos_request(int pm_qos_class);
> >> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> >> index d6d6dbd..6a031e6 100644
> >> --- a/kernel/power/qos.c
> >> +++ b/kernel/power/qos.c
> >> @@ -230,6 +230,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
> >>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
> >>
> >>  /**
> >> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
> >> + * @work: work struct for the delayed work (timeout)
> >> + *
> >> + * This cancels the timeout request by falling back to the default at timeout.
> >> + */
> >> +static void pm_qos_work_fn(struct work_struct *work)
> >> +{
> >> +     struct pm_qos_request *req = container_of(to_delayed_work(work),
> >> +                                               struct pm_qos_request,
> >> +                                               work);
> >> +
> >> +     pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
> >> +}
> >> +
> >> +/**
> >>   * pm_qos_add_request - inserts new qos request into the list
> >>   * @req: pointer to a preallocated handle
> >>   * @pm_qos_class: identifies which list of qos request to use
> >> @@ -253,6 +268,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
> >>               return;
> >>       }
> >>       req->pm_qos_class = pm_qos_class;
> >> +     INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
> >>       pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
> >>                            &req->node, PM_QOS_ADD_REQ, value);
> >>  }
> >> @@ -279,6 +295,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
> >>               return;
> >>       }
> >>
> >> +     if (delayed_work_pending(&req->work))
> >> +             cancel_delayed_work_sync(&req->work);
> >> +
> >>       if (new_value != req->node.prio)
> >>               pm_qos_update_target(
> >>                       pm_qos_array[req->pm_qos_class]->constraints,
> >> @@ -287,6 +306,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
> >>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
> >>
> >>  /**
> >> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> >> + * @req : handle to list element holding a pm_qos request to use
> >> + * @new_value: defines the temporal qos request
> >> + * @timeout_us: the effective duration of this qos request in usecs.
> >> + *
> >> + * After timeout_us, this qos request is cancelled automatically.
> >> + */
> >> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> >> +                                unsigned long timeout_us)
> >> +{
> >> +     if (!req)
> >> +             return;
> >> +     if (WARN(!pm_qos_request_active(req),
> >> +              "%s called for unknown object.", __func__))
> >> +             return;
> >> +
> >> +     if (delayed_work_pending(&req->work))
> >> +             cancel_delayed_work_sync(&req->work);
> >> +
> >> +     if (new_value != req->node.prio)
> >> +             pm_qos_update_target(
> >> +                     pm_qos_array[req->pm_qos_class]->constraints,
> >> +                     &req->node, PM_QOS_UPDATE_REQ, new_value);
> >> +
> >> +     schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
> >> +}
> >> +
> >> +/**
> >>   * pm_qos_remove_request - modifies an existing qos request
> >>   * @req: handle to request list element
> >>   *
> >> @@ -305,6 +352,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
> >>               return;
> >>       }
> >>
> >> +     if (delayed_work_pending(&req->work))
> >> +             cancel_delayed_work_sync(&req->work);
> >> +
> >>       pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
> >>                            &req->node, PM_QOS_REMOVE_REQ,
> >>                            PM_QOS_DEFAULT_VALUE);
> >>
> >
> 
> 
> 
> 

Re: [PATCH v4 resend] PM / QoS: add pm_qos_update_request_timeout API

[Rafael J. Wysocki]

On Wednesday, March 28, 2012, MyungJoo Ham wrote:
> The new API, pm_qos_update_request_timeout() is to provide a timeout
> with pm_qos_update_request.
> 
> For example, pm_qos_update_request_timeout(req, 100, 1000), means that
> QoS request on req with value 100 will be active for 1000 microseconds.
> After 1000 microseconds, the QoS request thru req is reset. If there
> were another pm_qos_update_request(req, x) during the 1000 us, this
> new request with value x will override as this is another request on the
> same req handle. A new request on the same req handle will always
> override the previous request whether it is the conventional request or
> it is the new timeout request.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Mark Gross <markgross@thegnar.org>

Applied to linux-pm/linux-next.

Thanks,
Rafael


> --
> Resend v4
> - Corrected changelog
> Changes from v3
> - The timeout value is expresses in us (1/1000000 sec), not ms (1/1000 sec).
> Changes from v2
> - Rebased to avoid merge conflict
> Chages from v1(RFC)
> - The semmantics of timeout changed. A timeout now means that the
>  corresponding qos-req object is being cancelled, not rolling back to
> the qos request state at the time of API call. With this changes,
> combined with the conventional update_request API, there exists up to
> only one qos request per qos-req object.
> - The timeout value is expressed in ms (1/1000 sec), not jiffies.
> - Style changes, removing unnecessary changes.
> - Bugfixes regarding pending timeout work.
> ---
>  include/linux/pm_qos.h |    4 +++
>  kernel/power/qos.c     |   50 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index c8a541e..18689737 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -8,6 +8,7 @@
>  #include <linux/notifier.h>
>  #include <linux/miscdevice.h>
>  #include <linux/device.h>
> +#include <linux/workqueue.h>
>  
>  enum {
>  	PM_QOS_RESERVED = 0,
> @@ -29,6 +30,7 @@ enum {
>  struct pm_qos_request {
>  	struct plist_node node;
>  	int pm_qos_class;
> +	struct delayed_work work; /* for pm_qos_update_request_timeout */
>  };
>  
>  struct dev_pm_qos_request {
> @@ -73,6 +75,8 @@ void pm_qos_add_request(struct pm_qos_request *req, int pm_qos_class,
>  			s32 value);
>  void pm_qos_update_request(struct pm_qos_request *req,
>  			   s32 new_value);
> +void pm_qos_update_request_timeout(struct pm_qos_request *req,
> +				   s32 new_value, unsigned long timeout_us);
>  void pm_qos_remove_request(struct pm_qos_request *req);
>  
>  int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index d6d6dbd..6a031e6 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -230,6 +230,21 @@ int pm_qos_request_active(struct pm_qos_request *req)
>  EXPORT_SYMBOL_GPL(pm_qos_request_active);
>  
>  /**
> + * pm_qos_work_fn - the timeout handler of pm_qos_update_request_timeout
> + * @work: work struct for the delayed work (timeout)
> + *
> + * This cancels the timeout request by falling back to the default at timeout.
> + */
> +static void pm_qos_work_fn(struct work_struct *work)
> +{
> +	struct pm_qos_request *req = container_of(to_delayed_work(work),
> +						  struct pm_qos_request,
> +						  work);
> +
> +	pm_qos_update_request(req, PM_QOS_DEFAULT_VALUE);
> +}
> +
> +/**
>   * pm_qos_add_request - inserts new qos request into the list
>   * @req: pointer to a preallocated handle
>   * @pm_qos_class: identifies which list of qos request to use
> @@ -253,6 +268,7 @@ void pm_qos_add_request(struct pm_qos_request *req,
>  		return;
>  	}
>  	req->pm_qos_class = pm_qos_class;
> +	INIT_DELAYED_WORK(&req->work, pm_qos_work_fn);
>  	pm_qos_update_target(pm_qos_array[pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_ADD_REQ, value);
>  }
> @@ -279,6 +295,9 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	if (new_value != req->node.prio)
>  		pm_qos_update_target(
>  			pm_qos_array[req->pm_qos_class]->constraints,
> @@ -287,6 +306,34 @@ void pm_qos_update_request(struct pm_qos_request *req,
>  EXPORT_SYMBOL_GPL(pm_qos_update_request);
>  
>  /**
> + * pm_qos_update_request_timeout - modifies an existing qos request temporarily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @new_value: defines the temporal qos request
> + * @timeout_us: the effective duration of this qos request in usecs.
> + *
> + * After timeout_us, this qos request is cancelled automatically.
> + */
> +void pm_qos_update_request_timeout(struct pm_qos_request *req, s32 new_value,
> +				   unsigned long timeout_us)
> +{
> +	if (!req)
> +		return;
> +	if (WARN(!pm_qos_request_active(req),
> +		 "%s called for unknown object.", __func__))
> +		return;
> +
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
> +	if (new_value != req->node.prio)
> +		pm_qos_update_target(
> +			pm_qos_array[req->pm_qos_class]->constraints,
> +			&req->node, PM_QOS_UPDATE_REQ, new_value);
> +
> +	schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
> +}
> +
> +/**
>   * pm_qos_remove_request - modifies an existing qos request
>   * @req: handle to request list element
>   *
> @@ -305,6 +352,9 @@ void pm_qos_remove_request(struct pm_qos_request *req)
>  		return;
>  	}
>  
> +	if (delayed_work_pending(&req->work))
> +		cancel_delayed_work_sync(&req->work);
> +
>  	pm_qos_update_target(pm_qos_array[req->pm_qos_class]->constraints,
>  			     &req->node, PM_QOS_REMOVE_REQ,
>  			     PM_QOS_DEFAULT_VALUE);
>