Re: [PATCH v4 0/4] PM / QoS: Restore DEV_PM_QOS_MIN/MAX_FREQUENCY

[Leonard Crestez <leonard.crestez@nxp.com>]

On 2019-11-29 1:34 PM, Rafael J. Wysocki wrote:
> On Tuesday, November 26, 2019 4:17:09 PM CET Leonard Crestez wrote:
>> Support for frequency limits in dev_pm_qos was removed when cpufreq was
>> switched to freq_qos, this series attempts to restore it by
>> reimplementing on top of freq_qos.
>>
>> Discussion about removal is here:
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-pm%2FVI1PR04MB7023DF47D046AEADB4E051EBEE680%40VI1PR04MB7023.eurprd04.prod.outlook.com%2FT%2F%23u&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=LYIRtXYe8qPz1G%2F0ADYpPhbhviv1pkk7%2B%2BQ1dX1DQR8%3D&reserved=0
>>
>> The cpufreq core switched away because it needs contraints at the level
>> of a "cpufreq_policy" which cover multiple cpus so dev_pm_qos coupling
>> to struct device was not useful. Cpufreq could only use dev_pm_qos by
>> implementing an additional layer of aggregation anyway.
>>
>> However in the devfreq subsystem scaling is always performed on a per-device
>> basis so dev_pm_qos is a very good match. Support for dev_pm_qos in devfreq
>> core is here (latest version, no dependencies outside this series):
>>
>> 	https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11252409%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=YodRx0IRVsjQXYA5X7UEosn%2FatO%2BWREfotwWrley2DQ%3D&reserved=0
>>
>> That series is RFC mostly because it needs these PM core patches.
>> Earlier versions got entangled in some locking cleanups but those are
>> not strictly necessary to get dev_pm_qos functionality.
>>
>> In theory if freq_qos is extended to handle conflicting min/max values then
>> this sharing would be valuable. Right now freq_qos just ties two unrelated
>> pm_qos aggregations for min and max freq.
>>
>> ---
>> This is implemented by embeding a freq_qos_request inside dev_pm_qos_request:
>> the data field was already an union in order to deal with flag requests.
>>
>> The internal freq_qos_apply is exported so that it can be called from
>> dev_pm_qos apply_constraints.
>>
>> The dev_pm_qos_constraints_destroy function has no obvious equivalent in
>> freq_qos and the whole approach of "removing requests" is somewhat dubios:
>> request objects should be owned by consumers and the list of qos requests
>> will most likely be empty when the target device is deleted. Series follows
>> current pattern for dev_pm_qos.
>>
>> First two patches can be applied separately.
>>
>> Changes since v3:
>> * Fix s/QOS/QoS in patch 2 title
>> * Improves comments in kunit test
>> * Fix assertions after freq_qos_remove_request
>> * Remove (c) from NXP copyright header
>> * Wrap long lines in qos.c to be under 80 chars. This fixes checkpatch but the
>> rule is already broken by code in the files.
>> * Collect reviews
>> Link to v3: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11260627%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=e%2B5SGU%2Bx4UjxlY292ejMO1kDewc3MmFvCpf0SDB0K4U%3D&reserved=0
>>
>> Changes since v2:
>> * #define PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE FREQ_QOS_MAX_DEFAULT_VALUE
>> * #define FREQ_QOS_MAX_DEFAULT_VALUE S32_MAX (in new patch)
>> * Add initial kunit test for freq_qos, validating the MAX_DEFAULT_VALUE found
>> by Matthias and another recent fix. Testing this should be easier!
>> Link to v2: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11250413%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=vyz%2FN2x7OZPCSx4Md8yQkOSjPtfNUvW6%2FHtG0bTG1xU%3D&reserved=0
>>
>> Changes since v1:
>> * Don't rename or EXPORT_SYMBOL_GPL the freq_qos_apply function; just
>> drop the static marker.
>> Link to v1: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fcover%2F11212887%2F&data=02%7C01%7Cleonard.crestez%40nxp.com%7C4531c54354d54442d71808d774c02866%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637106240968746397&sdata=SjcSQmMRZu0z3ATWygBpD8mRToCZT%2FBgQ7U3IRpMUB0%3D&reserved=0
>>
>> Leonard Crestez (4):
>>    PM / QoS: Initial kunit test
>>    PM / QoS: Redefine FREQ_QOS_MAX_DEFAULT_VALUE to S32_MAX
>>    PM / QoS: Reorder pm_qos/freq_qos/dev_pm_qos structs
>>    PM / QoS: Restore DEV_PM_QOS_MIN/MAX_FREQUENCY
>>
>>   drivers/base/Kconfig          |   4 ++
>>   drivers/base/power/Makefile   |   1 +
>>   drivers/base/power/qos-test.c | 117 ++++++++++++++++++++++++++++++++++
>>   drivers/base/power/qos.c      |  73 +++++++++++++++++++--
>>   include/linux/pm_qos.h        |  86 ++++++++++++++-----------
>>   kernel/power/qos.c            |   4 +-
>>   6 files changed, 242 insertions(+), 43 deletions(-)
>>   create mode 100644 drivers/base/power/qos-test.c
>>
>>
> 
> I have applied the whole series as 5.5 material, but I have reordered the fix
> (patch [2/4]) before the rest of it and marked it for -stable.

Thanks!

Devfreq maintainers, do you think the devfreq parts could be queued for 
5.5 as well? I'm not sure about the mechanics involved in this since 
devfreq qos depends at build time on this dev_pm_qos series.

Latest version is here: https://patchwork.kernel.org/cover/11252415/

It's RFC because it didn't compile against unpatched linux-next but it's 
otherwise a reduced version of a series that went through review and 
testing.

--
Regards,
Leonard