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

["Morten Brørup" <mb@smartsharesystems.com>]

> From: lihuisong (C) [mailto: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.

Then it should be OK.
Thanks for the detailed answer.

> >
> > @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)?

I suppose setting latency for service cores should be forbidden too,
so using rte_lcore_is_enabled() to check for ROLE_RTE is correct.

> >
> > 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
> >
> >
> > .