Re: [PATCH v10 1/2] power: introduce PM QoS API on CPU wide

["lihuisong (C)" <lihuisong@huawei.com>]

Hi Stephen,

Can you take a look at this reply so as to send out the next version ASAP?
Thanks.😁

/Huisong
在 2024/10/14 20:19, lihuisong (C) 写道:
> Hi Stephen,
>
>
> 在 2024/10/13 9:10, Stephen Hemminger 写道:
>> On Thu, 12 Sep 2024 10:38:11 +0800
>> Huisong Li <lihuisong@huawei.com> wrote:
>>
>>> +
>>> +PM QoS
>>> +------
>>> +
>>> +The deeper the idle state, the lower the power consumption, but the 
>>> longer
>>> +the resume time. Some service are delay sensitive and very except 
>>> the low
>>> +resume time, like interrupt packet receiving mode.
>>> +
>>> +And the 
>>> "/sys/devices/system/cpu/cpuX/power/pm_qos_resume_latency_us" sysfs
>>> +interface is used to set and get the resume latency limit on the 
>>> cpuX for
>>> +userspace. Each cpuidle governor in Linux select which idle state 
>>> to enter
>>> +based on this CPU resume latency in their idle task.
>>> +
>>> +The per-CPU PM QoS API can be used to set and get the CPU resume 
>>> latency based
>>> +on this sysfs.
>>> +
>>> +The ``rte_power_qos_set_cpu_resume_latency()`` function can control 
>>> the CPU's
>>> +idle state selection in Linux and limit just to enter the 
>>> shallowest idle state
>>> +to low the delay of resuming service after sleeping by setting 
>>> strict resume
>>> +latency (zero value).
>>> +
>>> +The ``rte_power_qos_get_cpu_resume_latency()`` function can get the 
>>> resume
>>> +latency on specified CPU.
>>> +
>> These paragraphs need some editing help. The wording is awkward,
>> it uses passive voice, and does not seemed directed at a user audience.
>> If you need help, a writer or AI might help clarify.
> How about the following description:
> -->
> The "/sys/devices/system/cpu/cpuX/power/pm_qos_resume_latency_us" sysfs
> interface is used to set and get the resume latency limit on the cpuX for
> userspace. Each cpuidle governor in Linux select which idle state to 
> enter
> based on this CPU resume latency in their idle task.
>
> The deeper the idle state, the lower the power consumption, but the 
> longer
> the resume time. Some service are delay sensitive and very except the low
> resume time, like interrupt packet receiving mode.
>
> The ``rte_power_qos_set_cpu_resume_latency()`` and 
> ``rte_power_qos_get_cpu_resume_latency()``
> can set and get the CPU resume latency based on this per-CPU sysfs.
>
> The ``rte_power_qos_set_cpu_resume_latency()`` can control the CPU's
> idle state selection and limit to enter the shallowest idle state
> to low the delay of resuming service by setting strict resume
> latency (zero value) so as to get better performance.
>>
>> It also ends up in the section associated with Intel UnCore.
>> It would be better after the Turbo Boost section.
> Ack
>>
>>
>>
>>> +    ret = open_core_sysfs_file(&f, "w", 
>>> PM_QOS_SYSFILE_RESUME_LATENCY_US, lcore_id);
>>> +    if (ret != 0) {
>>> +        POWER_LOG(ERR, "Failed to open 
>>> "PM_QOS_SYSFILE_RESUME_LATENCY_US, lcore_id);
>>> +        return ret;
>>> +    }
>> The function open_core_sysfs_file() should have been written to 
>> return FILE *
>> and then it could have same attributes as fopen().
> Do you mean we should save this handle for directly using it to get or 
> set this resume latency?
> If it is I understand, we cannot do it like this. Because qos driver 
> in Linux use the "noop_llseek()" as the .llseek API.
>>
>> The message should include the error reason.
>>
>>     if (open_core_sysfs_file(&f, "w", 
>> PM_QOS_SYSFILE_RESUME_LATENCY_US, lcore_id)) {
>>         POWER_LOG(ERR, "Failed to open " 
>> PM_QOS_SYSFILE_RESUME_LATENCY_US ": %s",
>>             lcore_id, strerror(errno));
> Good idea.  Thanks.
>
>>
>> .