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

Moger, Babu
2024-05-30T16:08:10Z

Hi Reinette,

On 5/9/24 16:11, Reinette Chatre wrote:
> 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?

Sure. Will do.

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

Yes.

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

ok. Sure.

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

Sure.
>
>>
>> /*
>> * Some directories under "DYN_PMU_PATH" could have
>
> Reinette
>

--
Thanks
Babu Moger