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

From: Reinette Chatre
Date: Tue Nov 28 2023 - 17:23:13 EST


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@xxxxxxxxx>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
> ---
> 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