diff mbox series

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

Message ID 406d1fbe6b480fbe13fd0b8c432cdc517a7bd8c1.1714073751.git.babu.moger@amd.com
State New
Headers show
Series selftests/resctrl: Enable MBM and MBA tests on AMD | expand

Commit Message

Moger, Babu April 25, 2024, 8:17 p.m. UTC
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(-)

Comments

Reinette Chatre May 9, 2024, 9:11 p.m. UTC | #1
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
Moger, Babu May 30, 2024, 4:08 p.m. UTC | #2
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 mbox series

Patch

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