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

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

Hi Morten,

Thanks for your review.


在 2024/6/14 16:04, Morten Brørup 写道:
>> +#define PM_QOS_SYSFILE_RESUME_LATENCY_US	\
>> +	"/sys/devices/system/cpu/cpu%u/power/pm_qos_resume_latency_us"
> Is it OK to access this path using the lcore_id as CPU parameter to open_core_sysfs_file(), or must it be mapped through rte_lcore_to_cpu_id(lcore_id) first?
The cpu_id getting by rte_lcore_to_cpu_id() is from 
lcore_config[lcore_id].core_id which is from 
"/sys/devices/system/cpu/cpuX/topology/core_id" file, please see the 
function eal_cpu_core_id().
So I think the number in above "cpuX" must be the lcore_id in DPDK.
And the similar interface in power lib also directly use the locore_id.
>
> @David, do you know?
>
>> +
>> +int
>> +rte_power_qos_set_cpu_resume_latency(uint16_t lcore_id, int latency)
>> +{
>> +	char buf[BUFSIZ] = {0};
>> +	FILE *f;
>> +	int ret;
>> +
>> +	if (lcore_id >= RTE_MAX_LCORE) {
>> +		POWER_LOG(ERR, "Lcore id %u can not exceeds %u",
>> +			  lcore_id, RTE_MAX_LCORE - 1U);
>> +		return -EINVAL;
>> +	}
> The lcore_id could be a registered non-EAL thread.
> You should probably fail in that case.
right, how about use rte_lcore_is_enabled(locore_id)?
>
> Same comment for rte_power_qos_get_cpu_resume_latency().
>
>
>> +#define PM_QOS_STRICT_LATENCY_VALUE             0
>> +#define PM_QOS_RESUME_LATENCY_NO_CONSTRAINT    ((int)(UINT32_MAX >> 1))
> These definitions are in the public header file, and thus should be RTE_POWER_ prefixed and have comments describing them.
Ack
>
>
> .