2015-04-20 14:00:46

by Dov Levenglick

[permalink] [raw]
Subject: Re: [RFC PATCH 1/4] PM / QoS: Add pm_qos_cancel_request_lazy() that doesn't sleep

> Add pm_qos_cancel_request_lazy() which is convenient for
> contexts that may not sleep.
>
> Signed-off-by: Adrian Hunter <[email protected]>
> ---
> include/linux/pm_qos.h | 2 ++
> kernel/power/qos.c | 20 ++++++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
> index 7b3ae0c..f44d353 100644
> --- a/include/linux/pm_qos.h
> +++ b/include/linux/pm_qos.h
> @@ -126,6 +126,8 @@ 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_cancel_request_lazy(struct pm_qos_request *req,
> + unsigned int timeout_us);
> void pm_qos_remove_request(struct pm_qos_request *req);
>

I think that this could be acheived using existing API if
pm_qos_update_request_timeout() were to be called with the existing
timeout value.
Since reading the existing timeout value is missing - and I think would be
a useful feature to have for other use-cases - do you agree with such an
approach?

> int pm_qos_request(int pm_qos_class);
> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
> index 97b0df7..ac131cb 100644
> --- a/kernel/power/qos.c
> +++ b/kernel/power/qos.c
> @@ -517,6 +517,26 @@ void pm_qos_update_request_timeout(struct
> pm_qos_request *req, s32 new_value,
> }
>
> /**
> + * pm_qos_cancel_request_lazy - cancels an existing qos request lazily.
> + * @req : handle to list element holding a pm_qos request to use
> + * @timeout_us: the delay before cancelling this qos request in usecs.
> + *
> + * After timeout_us, this qos request is cancelled.
> + */
> +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
> + unsigned int timeout_us)
> +{
> + if (!req)
> + return;
> + if (WARN(!pm_qos_request_active(req),
> + "%s called for unknown object.", __func__))
> + return;
> +
> + schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
> +}
> +EXPORT_SYMBOL_GPL(pm_qos_cancel_request_lazy);
> +
> +/**
> * pm_qos_remove_request - modifies an existing qos request
> * @req: handle to request list element
> *
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


2015-04-21 08:29:05

by Adrian Hunter

[permalink] [raw]
Subject: Re: [RFC PATCH 1/4] PM / QoS: Add pm_qos_cancel_request_lazy() that doesn't sleep

On 20/04/15 17:00, Dov Levenglick wrote:
>> Add pm_qos_cancel_request_lazy() which is convenient for
>> contexts that may not sleep.
>>
>> Signed-off-by: Adrian Hunter <[email protected]>
>> ---
>> include/linux/pm_qos.h | 2 ++
>> kernel/power/qos.c | 20 ++++++++++++++++++++
>> 2 files changed, 22 insertions(+)
>>
>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>> index 7b3ae0c..f44d353 100644
>> --- a/include/linux/pm_qos.h
>> +++ b/include/linux/pm_qos.h
>> @@ -126,6 +126,8 @@ 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_cancel_request_lazy(struct pm_qos_request *req,
>> + unsigned int timeout_us);
>> void pm_qos_remove_request(struct pm_qos_request *req);
>>
>
> I think that this could be acheived using existing API if
> pm_qos_update_request_timeout() were to be called with the existing
> timeout value.

I don't follow what you mean. There is no existing timeout value.
Did you mean existing request value? There is still the difference wrt
cancel_delayed_work_sync.

> Since reading the existing timeout value is missing - and I think would be
> a useful feature to have for other use-cases - do you agree with such an
> approach?
>
>> int pm_qos_request(int pm_qos_class);
>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>> index 97b0df7..ac131cb 100644
>> --- a/kernel/power/qos.c
>> +++ b/kernel/power/qos.c
>> @@ -517,6 +517,26 @@ void pm_qos_update_request_timeout(struct
>> pm_qos_request *req, s32 new_value,
>> }
>>
>> /**
>> + * pm_qos_cancel_request_lazy - cancels an existing qos request lazily.
>> + * @req : handle to list element holding a pm_qos request to use
>> + * @timeout_us: the delay before cancelling this qos request in usecs.
>> + *
>> + * After timeout_us, this qos request is cancelled.
>> + */
>> +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
>> + unsigned int timeout_us)
>> +{
>> + if (!req)
>> + return;
>> + if (WARN(!pm_qos_request_active(req),
>> + "%s called for unknown object.", __func__))
>> + return;
>> +
>> + schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
>> +}
>> +EXPORT_SYMBOL_GPL(pm_qos_cancel_request_lazy);
>> +
>> +/**
>> * pm_qos_remove_request - modifies an existing qos request
>> * @req: handle to request list element
>> *
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
>

2015-04-21 10:18:32

by Dov Levenglick

[permalink] [raw]
Subject: Re: [RFC PATCH 1/4] PM / QoS: Add pm_qos_cancel_request_lazy() that doesn't sleep

> On 20/04/15 17:00, Dov Levenglick wrote:
>>> Add pm_qos_cancel_request_lazy() which is convenient for
>>> contexts that may not sleep.
>>>
>>> Signed-off-by: Adrian Hunter <[email protected]>
>>> ---
>>> include/linux/pm_qos.h | 2 ++
>>> kernel/power/qos.c | 20 ++++++++++++++++++++
>>> 2 files changed, 22 insertions(+)
>>>
>>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>>> index 7b3ae0c..f44d353 100644
>>> --- a/include/linux/pm_qos.h
>>> +++ b/include/linux/pm_qos.h
>>> @@ -126,6 +126,8 @@ 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_cancel_request_lazy(struct pm_qos_request *req,
>>> + unsigned int timeout_us);
>>> void pm_qos_remove_request(struct pm_qos_request *req);
>>>
>>
>> I think that this could be acheived using existing API if
>> pm_qos_update_request_timeout() were to be called with the existing
>> timeout value.
>
> I don't follow what you mean. There is no existing timeout value.
> Did you mean existing request value? There is still the difference wrt
> cancel_delayed_work_sync.

I did mean the existing request value. Thanks.
There is also the cancel_delayed_work_sync, however I think that that
should be called in any case in order to cancel any other pending timeout
changes.

>
>> Since reading the existing timeout value is missing - and I think would
> be
>> a useful feature to have for other use-cases - do you agree with such an
>> approach?
>>
>>> int pm_qos_request(int pm_qos_class);
>>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>>> index 97b0df7..ac131cb 100644
>>> --- a/kernel/power/qos.c
>>> +++ b/kernel/power/qos.c
>>> @@ -517,6 +517,26 @@ void pm_qos_update_request_timeout(struct
>>> pm_qos_request *req, s32 new_value,
>>> }
>>>
>>> /**
>>> + * pm_qos_cancel_request_lazy - cancels an existing qos request
> lazily.
>>> + * @req : handle to list element holding a pm_qos request to use
>>> + * @timeout_us: the delay before cancelling this qos request in usecs.
>>> + *
>>> + * After timeout_us, this qos request is cancelled.
>>> + */
>>> +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
>>> + unsigned int timeout_us)
>>> +{
>>> + if (!req)
>>> + return;
>>> + if (WARN(!pm_qos_request_active(req),
>>> + "%s called for unknown object.", __func__))
>>> + return;
>>> +
>>> + schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
>>> +}
>>> +EXPORT_SYMBOL_GPL(pm_qos_cancel_request_lazy);
>>> +
>>> +/**
>>> * pm_qos_remove_request - modifies an existing qos request
>>> * @req: handle to request list element
>>> *
>>> --
>>> 1.9.1
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>> the body of a message to [email protected]
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> Forum,
>> a Linux Foundation Collaborative Project
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>


QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

2015-04-21 10:27:58

by Adrian Hunter

[permalink] [raw]
Subject: Re: [RFC PATCH 1/4] PM / QoS: Add pm_qos_cancel_request_lazy() that doesn't sleep

On 21/04/15 13:18, Dov Levenglick wrote:
>> On 20/04/15 17:00, Dov Levenglick wrote:
>>>> Add pm_qos_cancel_request_lazy() which is convenient for
>>>> contexts that may not sleep.
>>>>
>>>> Signed-off-by: Adrian Hunter <[email protected]>
>>>> ---
>>>> include/linux/pm_qos.h | 2 ++
>>>> kernel/power/qos.c | 20 ++++++++++++++++++++
>>>> 2 files changed, 22 insertions(+)
>>>>
>>>> diff --git a/include/linux/pm_qos.h b/include/linux/pm_qos.h
>>>> index 7b3ae0c..f44d353 100644
>>>> --- a/include/linux/pm_qos.h
>>>> +++ b/include/linux/pm_qos.h
>>>> @@ -126,6 +126,8 @@ 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_cancel_request_lazy(struct pm_qos_request *req,
>>>> + unsigned int timeout_us);
>>>> void pm_qos_remove_request(struct pm_qos_request *req);
>>>>
>>>
>>> I think that this could be acheived using existing API if
>>> pm_qos_update_request_timeout() were to be called with the existing
>>> timeout value.
>>
>> I don't follow what you mean. There is no existing timeout value.
>> Did you mean existing request value? There is still the difference wrt
>> cancel_delayed_work_sync.
>
> I did mean the existing request value. Thanks.
> There is also the cancel_delayed_work_sync, however I think that that
> should be called in any case in order to cancel any other pending timeout
> changes.

That might sleep which defeats one of the reasons for creating
pm_qos_cancel_request_lazy().

>
>>
>>> Since reading the existing timeout value is missing - and I think would
>> be
>>> a useful feature to have for other use-cases - do you agree with such an
>>> approach?
>>>
>>>> int pm_qos_request(int pm_qos_class);
>>>> diff --git a/kernel/power/qos.c b/kernel/power/qos.c
>>>> index 97b0df7..ac131cb 100644
>>>> --- a/kernel/power/qos.c
>>>> +++ b/kernel/power/qos.c
>>>> @@ -517,6 +517,26 @@ void pm_qos_update_request_timeout(struct
>>>> pm_qos_request *req, s32 new_value,
>>>> }
>>>>
>>>> /**
>>>> + * pm_qos_cancel_request_lazy - cancels an existing qos request
>> lazily.
>>>> + * @req : handle to list element holding a pm_qos request to use
>>>> + * @timeout_us: the delay before cancelling this qos request in usecs.
>>>> + *
>>>> + * After timeout_us, this qos request is cancelled.
>>>> + */
>>>> +void pm_qos_cancel_request_lazy(struct pm_qos_request *req,
>>>> + unsigned int timeout_us)
>>>> +{
>>>> + if (!req)
>>>> + return;
>>>> + if (WARN(!pm_qos_request_active(req),
>>>> + "%s called for unknown object.", __func__))
>>>> + return;
>>>> +
>>>> + schedule_delayed_work(&req->work, usecs_to_jiffies(timeout_us));
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(pm_qos_cancel_request_lazy);
>>>> +
>>>> +/**
>>>> * pm_qos_remove_request - modifies an existing qos request
>>>> * @req: handle to request list element
>>>> *
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>>>> the body of a message to [email protected]
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>>
>>>
>>>
>>> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
>>> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
>> Forum,
>>> a Linux Foundation Collaborative Project
>>>
>>>
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
>
> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
>
>