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