| Message ID | 406d1fbe6b480fbe13fd0b8c432cdc517a7bd8c1.1714073751.git.babu.moger@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | selftests/resctrl: Enable MBM and MBA tests on AMD | expand |
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
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 >
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); + } else { + perror("Unsupported Vendor!\n"); + 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; /* * Some directories under "DYN_PMU_PATH" could have
Detect the vendor and pass the sysfs name for the vendor for searching the controller information. Signed-off-by: Babu Moger <babu.moger@amd.com> --- tools/testing/selftests/resctrl/resctrl_val.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)