Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Rafael J. Wysocki]

On Sat, Nov 4, 2017 at 12:30 PM, Rafael J. Wysocki  wrote:
> On Sat, Nov 4, 2017 at 3:28 AM, Ramesh Thomas  wrote:
>> On 2017-11-03 at 09:39:08 -0700, Reinette Chatre wrote:
>>> Hi Rafael,
>>>
>>> I started to test this but found myself triggering one of the warnings:
>>>
>>> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
>>> > --- linux-pm.orig/include/linux/pm_qos.h
>>> > +++ linux-pm/include/linux/pm_qos.h
>>> > @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>>> > PM_QOS_FLAGS_ALL,
>>> >  };
>>> >
>>> > -#define PM_QOS_DEFAULT_VALUE -1
>>> > +#define PM_QOS_DEFAULT_VALUE   (-1)
>>>
>>> PM_QOS_DEFAULT_VALUE is -1 ...
>>>
>>>
>>> > ===
>>> > --- linux-pm.orig/drivers/base/power/qos.c
>>> > +++ linux-pm/drivers/base/power/qos.c
>>> > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>>> >
>>> > switch(req->type) {
>>> > case DEV_PM_QOS_RESUME_LATENCY:
>>> > +   if (WARN_ON(value < 0))
>>> > +   value = 0;
>>> > +
>>>
>>> ... causing me to hit this WARN_ON because apply_constraint() is called by 
>>> __dev_pm_qos_remove_request() with the value parameter set to 
>>> PM_QOS_DEFAULT_VALUE.
>>
>> That value does not get used if action is PM_QOS_REMOVE_REQ. May be just pass
>> 0 or PM_QOS_RESUME_LATENCY_DEFAULT_VALUE everywhere apply_constraint is 
>> called
>> with PM_QOS_REMOVE_REQ action.
>
> I think it's better to pass the "no constraint" value as that should
> not reorder it to the top of the list.

Actually, no.  The value is ignored if action is PM_QOS_REMOVE_REQ, so
it is better to simply check the action under the WARN_ON() too.

Thanks,
Rafael

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Rafael J. Wysocki]

On Sat, Nov 4, 2017 at 12:30 PM, Rafael J. Wysocki  wrote:
> On Sat, Nov 4, 2017 at 3:28 AM, Ramesh Thomas  wrote:
>> On 2017-11-03 at 09:39:08 -0700, Reinette Chatre wrote:
>>> Hi Rafael,
>>>
>>> I started to test this but found myself triggering one of the warnings:
>>>
>>> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
>>> > --- linux-pm.orig/include/linux/pm_qos.h
>>> > +++ linux-pm/include/linux/pm_qos.h
>>> > @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>>> > PM_QOS_FLAGS_ALL,
>>> >  };
>>> >
>>> > -#define PM_QOS_DEFAULT_VALUE -1
>>> > +#define PM_QOS_DEFAULT_VALUE   (-1)
>>>
>>> PM_QOS_DEFAULT_VALUE is -1 ...
>>>
>>>
>>> > ===
>>> > --- linux-pm.orig/drivers/base/power/qos.c
>>> > +++ linux-pm/drivers/base/power/qos.c
>>> > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>>> >
>>> > switch(req->type) {
>>> > case DEV_PM_QOS_RESUME_LATENCY:
>>> > +   if (WARN_ON(value < 0))
>>> > +   value = 0;
>>> > +
>>>
>>> ... causing me to hit this WARN_ON because apply_constraint() is called by 
>>> __dev_pm_qos_remove_request() with the value parameter set to 
>>> PM_QOS_DEFAULT_VALUE.
>>
>> That value does not get used if action is PM_QOS_REMOVE_REQ. May be just pass
>> 0 or PM_QOS_RESUME_LATENCY_DEFAULT_VALUE everywhere apply_constraint is 
>> called
>> with PM_QOS_REMOVE_REQ action.
>
> I think it's better to pass the "no constraint" value as that should
> not reorder it to the top of the list.

Actually, no.  The value is ignored if action is PM_QOS_REMOVE_REQ, so
it is better to simply check the action under the WARN_ON() too.

Thanks,
Rafael

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Rafael J. Wysocki]

On Sat, Nov 4, 2017 at 3:28 AM, Ramesh Thomas  wrote:
> On 2017-11-03 at 09:39:08 -0700, Reinette Chatre wrote:
>> Hi Rafael,
>>
>> I started to test this but found myself triggering one of the warnings:
>>
>> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
>> > --- linux-pm.orig/include/linux/pm_qos.h
>> > +++ linux-pm/include/linux/pm_qos.h
>> > @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>> > PM_QOS_FLAGS_ALL,
>> >  };
>> >
>> > -#define PM_QOS_DEFAULT_VALUE -1
>> > +#define PM_QOS_DEFAULT_VALUE   (-1)
>>
>> PM_QOS_DEFAULT_VALUE is -1 ...
>>
>>
>> > ===
>> > --- linux-pm.orig/drivers/base/power/qos.c
>> > +++ linux-pm/drivers/base/power/qos.c
>> > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>> >
>> > switch(req->type) {
>> > case DEV_PM_QOS_RESUME_LATENCY:
>> > +   if (WARN_ON(value < 0))
>> > +   value = 0;
>> > +
>>
>> ... causing me to hit this WARN_ON because apply_constraint() is called by 
>> __dev_pm_qos_remove_request() with the value parameter set to 
>> PM_QOS_DEFAULT_VALUE.
>
> That value does not get used if action is PM_QOS_REMOVE_REQ. May be just pass
> 0 or PM_QOS_RESUME_LATENCY_DEFAULT_VALUE everywhere apply_constraint is called
> with PM_QOS_REMOVE_REQ action.

I think it's better to pass the "no constraint" value as that should
not reorder it to the top of the list.

Thanks,
Rafael

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Rafael J. Wysocki]

On Sat, Nov 4, 2017 at 3:28 AM, Ramesh Thomas  wrote:
> On 2017-11-03 at 09:39:08 -0700, Reinette Chatre wrote:
>> Hi Rafael,
>>
>> I started to test this but found myself triggering one of the warnings:
>>
>> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
>> > --- linux-pm.orig/include/linux/pm_qos.h
>> > +++ linux-pm/include/linux/pm_qos.h
>> > @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>> > PM_QOS_FLAGS_ALL,
>> >  };
>> >
>> > -#define PM_QOS_DEFAULT_VALUE -1
>> > +#define PM_QOS_DEFAULT_VALUE   (-1)
>>
>> PM_QOS_DEFAULT_VALUE is -1 ...
>>
>>
>> > ===
>> > --- linux-pm.orig/drivers/base/power/qos.c
>> > +++ linux-pm/drivers/base/power/qos.c
>> > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>> >
>> > switch(req->type) {
>> > case DEV_PM_QOS_RESUME_LATENCY:
>> > +   if (WARN_ON(value < 0))
>> > +   value = 0;
>> > +
>>
>> ... causing me to hit this WARN_ON because apply_constraint() is called by 
>> __dev_pm_qos_remove_request() with the value parameter set to 
>> PM_QOS_DEFAULT_VALUE.
>
> That value does not get used if action is PM_QOS_REMOVE_REQ. May be just pass
> 0 or PM_QOS_RESUME_LATENCY_DEFAULT_VALUE everywhere apply_constraint is called
> with PM_QOS_REMOVE_REQ action.

I think it's better to pass the "no constraint" value as that should
not reorder it to the top of the list.

Thanks,
Rafael

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Rafael J. Wysocki]

On Fri, Nov 3, 2017 at 5:39 PM, Reinette Chatre
 wrote:
> Hi Rafael,
>
> I started to test this but found myself triggering one of the warnings:
>
> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
>> --- linux-pm.orig/include/linux/pm_qos.h
>> +++ linux-pm/include/linux/pm_qos.h
>> @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>>   PM_QOS_FLAGS_ALL,
>>  };
>>
>> -#define PM_QOS_DEFAULT_VALUE -1
>> +#define PM_QOS_DEFAULT_VALUE (-1)
>
> PM_QOS_DEFAULT_VALUE is -1 ...
>
>
>> ===
>> --- linux-pm.orig/drivers/base/power/qos.c
>> +++ linux-pm/drivers/base/power/qos.c
>> @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>>
>>   switch(req->type) {
>>   case DEV_PM_QOS_RESUME_LATENCY:
>> + if (WARN_ON(value < 0))
>> + value = 0;
>> +
>
> ... causing me to hit this WARN_ON because apply_constraint() is called by 
> __dev_pm_qos_remove_request() with the value parameter set to 
> PM_QOS_DEFAULT_VALUE.

Thanks for the report!

I've added it to catch bugs in the future, but it has caught a bug
right away. :-)

That actually is a bug in the existing code that needs to be fixed.

Thanks,
Rafael

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Rafael J. Wysocki]

On Fri, Nov 3, 2017 at 5:39 PM, Reinette Chatre
 wrote:
> Hi Rafael,
>
> I started to test this but found myself triggering one of the warnings:
>
> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
>> --- linux-pm.orig/include/linux/pm_qos.h
>> +++ linux-pm/include/linux/pm_qos.h
>> @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>>   PM_QOS_FLAGS_ALL,
>>  };
>>
>> -#define PM_QOS_DEFAULT_VALUE -1
>> +#define PM_QOS_DEFAULT_VALUE (-1)
>
> PM_QOS_DEFAULT_VALUE is -1 ...
>
>
>> ===
>> --- linux-pm.orig/drivers/base/power/qos.c
>> +++ linux-pm/drivers/base/power/qos.c
>> @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>>
>>   switch(req->type) {
>>   case DEV_PM_QOS_RESUME_LATENCY:
>> + if (WARN_ON(value < 0))
>> + value = 0;
>> +
>
> ... causing me to hit this WARN_ON because apply_constraint() is called by 
> __dev_pm_qos_remove_request() with the value parameter set to 
> PM_QOS_DEFAULT_VALUE.

Thanks for the report!

I've added it to catch bugs in the future, but it has caught a bug
right away. :-)

That actually is a bug in the existing code that needs to be fixed.

Thanks,
Rafael

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Ramesh Thomas]

On 2017-11-03 at 12:50:15 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> The special value of 0 for device resume latency PM QoS means
> "no restriction", but there are two problems with that.
> 
> First, device resume latency PM QoS requests with 0 as the
> value are always put in front of requests with positive
> values in the priority lists used internally by the PM QoS
> framework, causing 0 to be chosen as an effective constraint
> value.  However, that 0 is then interpreted as "no restriction"
> effectively overriding the other requests with specific
> restrictions which is incorrect.
> 
> Second, the users of device resume latency PM QoS have no
> way to specify that *any* resume latency at all should be
> avoided, which is an artificial limitation in general.
> 
> To address these issues, modify device resume latency PM QoS to
> use S32_MAX as the "no constraint" value and 0 as the "no
> latency at all" one and rework its users (the cpuidle menu
> governor, the genpd QoS governor and the runtime PM framework)
> to follow these changes.
> 
> Also add a special "n/a" value to the corresponding user space I/F
> to allow user space to indicate that it cannot accept any resume
> latencies at all for the given device.
> 
> Fixes: 85dc0b8a4019 (PM / QoS: Make it possible to expose PM QoS latency 
> constraints)
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197323
> Reported-by: Reinette Chatre 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  Documentation/ABI/testing/sysfs-devices-power |4 +++-
>  drivers/base/cpu.c|3 ++-
>  drivers/base/power/domain.c   |2 +-
>  drivers/base/power/domain_governor.c  |   26 
> ++
>  drivers/base/power/qos.c  |5 -
>  drivers/base/power/runtime.c  |2 +-
>  drivers/base/power/sysfs.c|   25 
> +
>  drivers/cpuidle/governors/menu.c  |4 ++--
>  include/linux/pm_qos.h|   26 
> ++
>  9 files changed, 62 insertions(+), 35 deletions(-)
> 
> Index: linux-pm/drivers/base/power/sysfs.c
> ===
> --- linux-pm.orig/drivers/base/power/sysfs.c
> +++ linux-pm/drivers/base/power/sysfs.c
> @@ -218,7 +218,14 @@ static ssize_t pm_qos_resume_latency_sho
> struct device_attribute *attr,
> char *buf)
>  {
> - return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev));
> + s32 value = dev_pm_qos_requested_resume_latency(dev);
> +
> + if (value == 0)
> + return sprintf(buf, "n/a\n");
> + else if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
> + value = 0;
> +
> + return sprintf(buf, "%d\n", value);
>  }
>  
>  static ssize_t pm_qos_resume_latency_store(struct device *dev,
> @@ -228,11 +235,21 @@ static ssize_t pm_qos_resume_latency_sto
>   s32 value;
>   int ret;
>  
> - if (kstrtos32(buf, 0, ))
> - return -EINVAL;
> + if (!kstrtos32(buf, 0, )) {
> + /*
> +  * Prevent users from writing negative or "no constraint" values
> +  * directly.
> +  */
> + if (value < 0 || value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
> + return -EINVAL;
>  
> - if (value < 0)
> + if (value == 0)
> + value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> + } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) {
> + value = 0;
> + } else {
>   return -EINVAL;
> + }
>  
>   ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req,
>   value);
> Index: linux-pm/include/linux/pm_qos.h
> ===
> --- linux-pm.orig/include/linux/pm_qos.h
> +++ linux-pm/include/linux/pm_qos.h
> @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>   PM_QOS_FLAGS_ALL,
>  };
>  
> -#define PM_QOS_DEFAULT_VALUE -1
> +#define PM_QOS_DEFAULT_VALUE (-1)
> +#define PM_QOS_LATENCY_ANY   S32_MAX
> +#define PM_QOS_LATENCY_ANY_NS((s64)PM_QOS_LATENCY_ANY * 
> NSEC_PER_USEC)
>  
>  #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
>  #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
>  #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE  0
>  #define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE0
> -#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE  0
> +#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE  PM_QOS_LATENCY_ANY
> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT  PM_QOS_LATENCY_ANY
> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS   PM_QOS_LATENCY_ANY_NS
>  #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE 

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Ramesh Thomas]

On 2017-11-03 at 12:50:15 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki 
> 
> The special value of 0 for device resume latency PM QoS means
> "no restriction", but there are two problems with that.
> 
> First, device resume latency PM QoS requests with 0 as the
> value are always put in front of requests with positive
> values in the priority lists used internally by the PM QoS
> framework, causing 0 to be chosen as an effective constraint
> value.  However, that 0 is then interpreted as "no restriction"
> effectively overriding the other requests with specific
> restrictions which is incorrect.
> 
> Second, the users of device resume latency PM QoS have no
> way to specify that *any* resume latency at all should be
> avoided, which is an artificial limitation in general.
> 
> To address these issues, modify device resume latency PM QoS to
> use S32_MAX as the "no constraint" value and 0 as the "no
> latency at all" one and rework its users (the cpuidle menu
> governor, the genpd QoS governor and the runtime PM framework)
> to follow these changes.
> 
> Also add a special "n/a" value to the corresponding user space I/F
> to allow user space to indicate that it cannot accept any resume
> latencies at all for the given device.
> 
> Fixes: 85dc0b8a4019 (PM / QoS: Make it possible to expose PM QoS latency 
> constraints)
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=197323
> Reported-by: Reinette Chatre 
> Signed-off-by: Rafael J. Wysocki 
> ---
>  Documentation/ABI/testing/sysfs-devices-power |4 +++-
>  drivers/base/cpu.c|3 ++-
>  drivers/base/power/domain.c   |2 +-
>  drivers/base/power/domain_governor.c  |   26 
> ++
>  drivers/base/power/qos.c  |5 -
>  drivers/base/power/runtime.c  |2 +-
>  drivers/base/power/sysfs.c|   25 
> +
>  drivers/cpuidle/governors/menu.c  |4 ++--
>  include/linux/pm_qos.h|   26 
> ++
>  9 files changed, 62 insertions(+), 35 deletions(-)
> 
> Index: linux-pm/drivers/base/power/sysfs.c
> ===
> --- linux-pm.orig/drivers/base/power/sysfs.c
> +++ linux-pm/drivers/base/power/sysfs.c
> @@ -218,7 +218,14 @@ static ssize_t pm_qos_resume_latency_sho
> struct device_attribute *attr,
> char *buf)
>  {
> - return sprintf(buf, "%d\n", dev_pm_qos_requested_resume_latency(dev));
> + s32 value = dev_pm_qos_requested_resume_latency(dev);
> +
> + if (value == 0)
> + return sprintf(buf, "n/a\n");
> + else if (value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
> + value = 0;
> +
> + return sprintf(buf, "%d\n", value);
>  }
>  
>  static ssize_t pm_qos_resume_latency_store(struct device *dev,
> @@ -228,11 +235,21 @@ static ssize_t pm_qos_resume_latency_sto
>   s32 value;
>   int ret;
>  
> - if (kstrtos32(buf, 0, ))
> - return -EINVAL;
> + if (!kstrtos32(buf, 0, )) {
> + /*
> +  * Prevent users from writing negative or "no constraint" values
> +  * directly.
> +  */
> + if (value < 0 || value == PM_QOS_RESUME_LATENCY_NO_CONSTRAINT)
> + return -EINVAL;
>  
> - if (value < 0)
> + if (value == 0)
> + value = PM_QOS_RESUME_LATENCY_NO_CONSTRAINT;
> + } else if (!strcmp(buf, "n/a") || !strcmp(buf, "n/a\n")) {
> + value = 0;
> + } else {
>   return -EINVAL;
> + }
>  
>   ret = dev_pm_qos_update_request(dev->power.qos->resume_latency_req,
>   value);
> Index: linux-pm/include/linux/pm_qos.h
> ===
> --- linux-pm.orig/include/linux/pm_qos.h
> +++ linux-pm/include/linux/pm_qos.h
> @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>   PM_QOS_FLAGS_ALL,
>  };
>  
> -#define PM_QOS_DEFAULT_VALUE -1
> +#define PM_QOS_DEFAULT_VALUE (-1)
> +#define PM_QOS_LATENCY_ANY   S32_MAX
> +#define PM_QOS_LATENCY_ANY_NS((s64)PM_QOS_LATENCY_ANY * 
> NSEC_PER_USEC)
>  
>  #define PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
>  #define PM_QOS_NETWORK_LAT_DEFAULT_VALUE (2000 * USEC_PER_SEC)
>  #define PM_QOS_NETWORK_THROUGHPUT_DEFAULT_VALUE  0
>  #define PM_QOS_MEMORY_BANDWIDTH_DEFAULT_VALUE0
> -#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE  0
> +#define PM_QOS_RESUME_LATENCY_DEFAULT_VALUE  PM_QOS_LATENCY_ANY
> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT  PM_QOS_LATENCY_ANY
> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT_NS   PM_QOS_LATENCY_ANY_NS
>  #define PM_QOS_LATENCY_TOLERANCE_DEFAULT_VALUE   0
>  #define PM_QOS_LATENCY_TOLERANCE_NO_CONSTRAINT   (-1)
> -#define 

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Ramesh Thomas]

On 2017-11-03 at 09:39:08 -0700, Reinette Chatre wrote:
> Hi Rafael,
> 
> I started to test this but found myself triggering one of the warnings:
> 
> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
> > --- linux-pm.orig/include/linux/pm_qos.h
> > +++ linux-pm/include/linux/pm_qos.h
> > @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
> > PM_QOS_FLAGS_ALL,
> >  };
> >  
> > -#define PM_QOS_DEFAULT_VALUE -1
> > +#define PM_QOS_DEFAULT_VALUE   (-1)
> 
> PM_QOS_DEFAULT_VALUE is -1 ...
> 
> 
> > ===
> > --- linux-pm.orig/drivers/base/power/qos.c
> > +++ linux-pm/drivers/base/power/qos.c
> > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
> >  
> > switch(req->type) {
> > case DEV_PM_QOS_RESUME_LATENCY:
> > +   if (WARN_ON(value < 0))
> > +   value = 0;
> > +
> 
> ... causing me to hit this WARN_ON because apply_constraint() is called by 
> __dev_pm_qos_remove_request() with the value parameter set to 
> PM_QOS_DEFAULT_VALUE.

That value does not get used if action is PM_QOS_REMOVE_REQ. May be just pass
0 or PM_QOS_RESUME_LATENCY_DEFAULT_VALUE everywhere apply_constraint is called
with PM_QOS_REMOVE_REQ action.

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Ramesh Thomas]

On 2017-11-03 at 09:39:08 -0700, Reinette Chatre wrote:
> Hi Rafael,
> 
> I started to test this but found myself triggering one of the warnings:
> 
> On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
> > --- linux-pm.orig/include/linux/pm_qos.h
> > +++ linux-pm/include/linux/pm_qos.h
> > @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
> > PM_QOS_FLAGS_ALL,
> >  };
> >  
> > -#define PM_QOS_DEFAULT_VALUE -1
> > +#define PM_QOS_DEFAULT_VALUE   (-1)
> 
> PM_QOS_DEFAULT_VALUE is -1 ...
> 
> 
> > ===
> > --- linux-pm.orig/drivers/base/power/qos.c
> > +++ linux-pm/drivers/base/power/qos.c
> > @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
> >  
> > switch(req->type) {
> > case DEV_PM_QOS_RESUME_LATENCY:
> > +   if (WARN_ON(value < 0))
> > +   value = 0;
> > +
> 
> ... causing me to hit this WARN_ON because apply_constraint() is called by 
> __dev_pm_qos_remove_request() with the value parameter set to 
> PM_QOS_DEFAULT_VALUE.

That value does not get used if action is PM_QOS_REMOVE_REQ. May be just pass
0 or PM_QOS_RESUME_LATENCY_DEFAULT_VALUE everywhere apply_constraint is called
with PM_QOS_REMOVE_REQ action.

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Reinette Chatre]

Hi Rafael,

I started to test this but found myself triggering one of the warnings:

On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
> --- linux-pm.orig/include/linux/pm_qos.h
> +++ linux-pm/include/linux/pm_qos.h
> @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>   PM_QOS_FLAGS_ALL,
>  };
>  
> -#define PM_QOS_DEFAULT_VALUE -1
> +#define PM_QOS_DEFAULT_VALUE (-1)

PM_QOS_DEFAULT_VALUE is -1 ...


> ===
> --- linux-pm.orig/drivers/base/power/qos.c
> +++ linux-pm/drivers/base/power/qos.c
> @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>  
>   switch(req->type) {
>   case DEV_PM_QOS_RESUME_LATENCY:
> + if (WARN_ON(value < 0))
> + value = 0;
> +

... causing me to hit this WARN_ON because apply_constraint() is called by 
__dev_pm_qos_remove_request() with the value parameter set to 
PM_QOS_DEFAULT_VALUE.

Reinette

Re: [RFT][PATCH v2 2/2] PM / QoS: Fix device resume latency framework

[Reinette Chatre]

Hi Rafael,

I started to test this but found myself triggering one of the warnings:

On 11/3/2017 4:50 AM, Rafael J. Wysocki wrote:
> --- linux-pm.orig/include/linux/pm_qos.h
> +++ linux-pm/include/linux/pm_qos.h
> @@ -28,16 +28,19 @@ enum pm_qos_flags_status {
>   PM_QOS_FLAGS_ALL,
>  };
>  
> -#define PM_QOS_DEFAULT_VALUE -1
> +#define PM_QOS_DEFAULT_VALUE (-1)

PM_QOS_DEFAULT_VALUE is -1 ...


> ===
> --- linux-pm.orig/drivers/base/power/qos.c
> +++ linux-pm/drivers/base/power/qos.c
> @@ -139,6 +139,9 @@ static int apply_constraint(struct dev_p
>  
>   switch(req->type) {
>   case DEV_PM_QOS_RESUME_LATENCY:
> + if (WARN_ON(value < 0))
> + value = 0;
> +

... causing me to hit this WARN_ON because apply_constraint() is called by 
__dev_pm_qos_remove_request() with the value parameter set to 
PM_QOS_DEFAULT_VALUE.

Reinette