Hi Reinette,
On 6/14/24 13:38, Reinette Chatre wrote:
> Hi Babu,
>
> Subject and changelog mentions how controller name is "passed" but the
> patch does not seem to "pass" anything new.
Sure. Will change the subject to this. Also will update the commit message.
selftests/resctrl: Dynamically detect sysfs controller name of the vendor
>
> On 6/5/24 3:45 PM, Babu Moger wrote:
>> The test detects number of memory controllers by looking at the sysfs
>> file system. Detect the vendor to pass the controller name appropriately.
>> So, that rest of the code will be generic and can be used to support other
>> vendors.
>>
>> Also change the search to look for the full string "uncore_imc_". Replace
>> the sizeof with strlen.
>>
>> Signed-off-by: Babu Moger <babu.moger@xxxxxxx>
>> ---
>> v3: Change the search string to "uncore_imc_".
>>
>> v2: No changes
>> ---
>> tools/testing/selftests/resctrl/resctrl_val.c | 22 ++++++++++++-------
>> 1 file changed, 14 insertions(+), 8 deletions(-)
>>
>> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c
>> b/tools/testing/selftests/resctrl/resctrl_val.c
>> index 2d5e961b3a68..23c0e0a1d845 100644
>> --- a/tools/testing/selftests/resctrl/resctrl_val.c
>> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
>> @@ -10,7 +10,7 @@
>> */
>> #include "resctrl.h"
>> -#define UNCORE_IMC "uncore_imc"
>> +#define UNCORE_IMC "uncore_imc_"
>> #define READ_FILE_NAME "events/cas_count_read"
>> #define WRITE_FILE_NAME "events/cas_count_write"
>> #define DYN_PMU_PATH "/sys/bus/event_source/devices"
>> @@ -206,24 +206,30 @@ static int num_of_mem_controllers(void)
>> char mc_dir[512], *temp;
>> unsigned int count = 0;
>> struct dirent *ep;
>> - int ret;
>> + char *sysfs_name;
>> + int ret, vendor;
>> DIR *dp;
>> + vendor = get_vendor();
>
> get_vendor() is already optimized to only need to do actual detection once.
> I thus do not see a need for a local variable.
Sure.
>
>> + if (vendor == ARCH_INTEL) {
>> + sysfs_name = UNCORE_IMC;
>> + } else {
>> + ksft_perror("Unsupported vendor!\n");
>
> ksft_perror() also adds a "\n" so adding it here is not necessary. Also
> please drop the exclamation.
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;
>> /*
>> * imc counters are named as "uncore_imc_<n>", hence
>> - * increment the pointer to point to <n>. Note that
>> - * sizeof(UNCORE_IMC) would count for null character as
>> - * well and hence the last underscore character in
>> - * uncore_imc'_' need not be counted.
>> + * increment the pointer to point to <n>.
>> */
>> - temp = temp + sizeof(UNCORE_IMC);
>> + temp = temp + strlen(sysfs_name);
>> /*
>> * Some directories under "DYN_PMU_PATH" could have
>
> Reinette
>
--
Thanks
Babu Moger
