Re: [PATCH v2 2/4] selftests/resctrl: Pass sysfs controller name of the vendor

Reinette Chatre
2024-05-09T21:11:24Z

Hi Babu,

On 4/25/2024 1:17 PM, Babu Moger wrote:
> Detect the vendor and pass the sysfs name for the vendor for searching
> the controller information.

Could you please write a proper changelog?

>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> tools/testing/selftests/resctrl/resctrl_val.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index a30cfcff605f..e3b09128ec3d 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -224,14 +224,24 @@ static int num_of_mem_controllers(void)
> {
> char imc_dir[512], *temp;
> unsigned int count = 0;
> + int ret, vendor, size;
> struct dirent *ep;
> - int ret;
> + char *sysfs_name;
> DIR *dp;
>
> + vendor = get_vendor();
> + if (vendor == ARCH_INTEL) {
> + sysfs_name = UNCORE_IMC;
> + size = sizeof(UNCORE_IMC);

Why is separate size needed? Can strlen() just be used when needed?

> + } else {
> + perror("Unsupported Vendor!\n");

ksft_perror()?

In the message, "Vendor" need not start with capital. It may also
help to print the vendor value in this unlikely case.

> + return -1;
> + }
> +
> dp = opendir(DYN_PMU_PATH);
> if (dp) {
> while ((ep = readdir(dp))) {
> - temp = strstr(ep->d_name, UNCORE_IMC);
> + temp = strstr(ep->d_name, sysfs_name);
> if (!temp)
> continue;
>
> @@ -242,7 +252,7 @@ static int num_of_mem_controllers(void)
> * well and hence the last underscore character in
> * uncore_imc'_' need not be counted.
> */
> - temp = temp + sizeof(UNCORE_IMC);
> + temp = temp + size;

strlen()? (Keeping in mind the adjustment for the "_" character).

>
> /*
> * Some directories under "DYN_PMU_PATH" could have

Reinette