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 <[email protected]>
>> ---
>>  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

Reply via email to