linux-kernel - Re: [PATCH v2 24/26] selftests/resctrl: Rename resource ID to domain ID

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1297008e-b31c-4833-b446-fcd4036d1e64@intel.com>
Date:   Tue, 28 Nov 2023 14:22:26 -0800
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        <linux-kselftest@...r.kernel.org>, Shuah Khan <shuah@...nel.org>,
        "Shaopeng Tan" <tan.shaopeng@...fujitsu.com>,
        Maciej Wieczór-Retman 
        <maciej.wieczor-retman@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>
CC:     <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 24/26] selftests/resctrl: Rename resource ID to domain
 ID

Hi Ilpo,

On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
> Kernel-side calls the instances of a resource domains.
> 
> Change the resource_id naming in the selftest code to domain_id to
> match the kernel side better.
> 
> Suggested-by: Maciej Wieczór-Retman <maciej.wieczor-retman@...el.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---
>  tools/testing/selftests/resctrl/resctrl.h     |  2 +-
>  tools/testing/selftests/resctrl/resctrl_val.c | 30 +++++++++----------
>  tools/testing/selftests/resctrl/resctrlfs.c   | 18 +++++------
>  3 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
> index ed7a69578daf..280ab6b6bd07 100644
> --- a/tools/testing/selftests/resctrl/resctrl.h
> +++ b/tools/testing/selftests/resctrl/resctrl.h
> @@ -124,7 +124,7 @@ extern char llc_occup_path[1024];
>  int get_vendor(void);
>  bool check_resctrlfs_support(void);
>  int filter_dmesg(void);
> -int get_resource_id(int cpu_no, int *resource_id);
> +int get_domain_id(int cpu_no, int *domain_id);
>  int mount_resctrlfs(void);
>  int umount_resctrlfs(void);
>  int validate_bw_report_request(char *bw_report);
> diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
> index 4e0c356b7f18..f278f5c9195c 100644
> --- a/tools/testing/selftests/resctrl/resctrl_val.c
> +++ b/tools/testing/selftests/resctrl/resctrl_val.c
> @@ -387,20 +387,20 @@ static int get_mem_bw_imc(int cpu_no, char *bw_report, float *bw_imc)
>  	return 0;
>  }
>  
> -void set_mbm_path(const char *ctrlgrp, const char *mongrp, int resource_id)
> +void set_mbm_path(const char *ctrlgrp, const char *mongrp, int domain_id)
>  {
>  	if (ctrlgrp && mongrp)
>  		sprintf(mbm_total_path, CON_MON_MBM_LOCAL_BYTES_PATH,
> -			RESCTRL_PATH, ctrlgrp, mongrp, resource_id);
> +			RESCTRL_PATH, ctrlgrp, mongrp, domain_id);
>  	else if (!ctrlgrp && mongrp)
>  		sprintf(mbm_total_path, MON_MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
> -			mongrp, resource_id);
> +			mongrp, domain_id);
>  	else if (ctrlgrp && !mongrp)
>  		sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
> -			ctrlgrp, resource_id);
> +			ctrlgrp, domain_id);
>  	else if (!ctrlgrp && !mongrp)
>  		sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH, RESCTRL_PATH,
> -			resource_id);
> +			domain_id);
>  }
>  
>  /*
> @@ -413,23 +413,23 @@ void set_mbm_path(const char *ctrlgrp, const char *mongrp, int resource_id)
>  static void initialize_mem_bw_resctrl(const char *ctrlgrp, const char *mongrp,
>  				      int cpu_no, char *resctrl_val)
>  {
> -	int resource_id;
> +	int domain_id;
>  
> -	if (get_resource_id(cpu_no, &resource_id) < 0) {
> -		perror("Could not get resource_id");
> +	if (get_domain_id(cpu_no, &domain_id) < 0) {
> +		perror("Could not get domain ID");

I do not think perror() is appropriate here. It is too far away
from failing system call (for which perror() was already called
anyway).

>  		return;
>  	}
>  
>  	if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)))
> -		set_mbm_path(ctrlgrp, mongrp, resource_id);
> +		set_mbm_path(ctrlgrp, mongrp, domain_id);
>  
>  	if (!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
>  		if (ctrlgrp)
>  			sprintf(mbm_total_path, CON_MBM_LOCAL_BYTES_PATH,
> -				RESCTRL_PATH, ctrlgrp, resource_id);
> +				RESCTRL_PATH, ctrlgrp, domain_id);
>  		else
>  			sprintf(mbm_total_path, MBM_LOCAL_BYTES_PATH,
> -				RESCTRL_PATH, resource_id);
> +				RESCTRL_PATH, domain_id);
>  	}
>  }
>  
> @@ -582,15 +582,15 @@ static void set_cmt_path(const char *ctrlgrp, const char *mongrp, char sock_num)
>  static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
>  					int cpu_no, char *resctrl_val)
>  {
> -	int resource_id;
> +	int domain_id;
>  
> -	if (get_resource_id(cpu_no, &resource_id) < 0) {
> -		perror("# Unable to resource_id");
> +	if (get_domain_id(cpu_no, &domain_id) < 0) {
> +		perror("# Could not get domain ID");

here also

>  		return;
>  	}
>  
>  	if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR)))
> -		set_cmt_path(ctrlgrp, mongrp, resource_id);
> +		set_cmt_path(ctrlgrp, mongrp, domain_id);
>  }
>  
>  static int measure_vals(const struct user_params *uparams,

Reinette