Hi Ilpo,
On 11/20/2023 3:13 AM, Ilpo Järvinen wrote:
measure_cache_vals() does a different thing depending on the test case that called it:
- For CAT, it measures LLC misses through perf.
- For CMT, it measures LLC occupancy through resctrl.
Split these two functionalities into own functions the CAT and CMT tests can call directly. Replace passing the struct resctrl_val_param parameter with the filename because it's more generic and all those functions need out of resctrl_val.
Co-developed-by: Fenghua Yu fenghua.yu@intel.com Signed-off-by: Fenghua Yu fenghua.yu@intel.com Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com
tools/testing/selftests/resctrl/cache.c | 66 ++++++++++++------- tools/testing/selftests/resctrl/resctrl.h | 2 +- tools/testing/selftests/resctrl/resctrl_val.c | 2 +- 3 files changed, 43 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/resctrl/cache.c b/tools/testing/selftests/resctrl/cache.c index 8aa6d67db978..129d1c293518 100644 --- a/tools/testing/selftests/resctrl/cache.c +++ b/tools/testing/selftests/resctrl/cache.c @@ -147,7 +147,7 @@ static int get_llc_occu_resctrl(unsigned long *llc_occupancy)
- Return: 0 on success. non-zero on failure.
*/ -static int print_results_cache(char *filename, int bm_pid, +static int print_results_cache(const char *filename, int bm_pid, unsigned long llc_value) { FILE *fp; @@ -169,35 +169,51 @@ static int print_results_cache(char *filename, int bm_pid, return 0; } -int measure_cache_vals(struct resctrl_val_param *param, int bm_pid) +/*
- perf_event_measure - Measure perf events
- @filename: Filename for writing the results
- @bm_pid: PID that runs the benchmark
- Measures perf events (e.g., cache misses) and writes the results into
- @filename. @bm_pid is written to the results file along with the measured
- value.
- Return: =0 on success. <0 on failure.
I do not think this is accurate. It looks like this function returns the return value of print_results_cache() which returns errno on failure. If this is the case then I think this proves that returning a positive integer on failure should be avoided since it just creates traps.
- */
+static int perf_event_measure(const char *filename, int bm_pid) {
- unsigned long llc_perf_miss = 0, llc_occu_resc = 0, llc_value = 0;
- unsigned long llc_perf_miss = 0; int ret;
- /*
* Measure cache miss from perf.*/- if (!strncmp(param->resctrl_val, CAT_STR, sizeof(CAT_STR))) {
ret = get_llc_perf(&llc_perf_miss);if (ret < 0)return ret;llc_value = llc_perf_miss;- }
- ret = get_llc_perf(&llc_perf_miss);
- if (ret < 0)
return ret;
- /*
* Measure llc occupancy from resctrl.*/- if (!strncmp(param->resctrl_val, CMT_STR, sizeof(CMT_STR))) {
ret = get_llc_occu_resctrl(&llc_occu_resc);if (ret < 0)return ret;llc_value = llc_occu_resc;- }
- ret = print_results_cache(param->filename, bm_pid, llc_value);
- if (ret)
- ret = print_results_cache(filename, bm_pid, llc_perf_miss);
- return ret;
+}
Perhaps print_results_cache() can be made to return negative error and this just be "return print_results_cache(...)" and the function comment be accurate?
+/*
- measure_llc_resctrl - Measure resctrl llc value from resctrl
llc -> LLC
- @filename: Filename for writing the results
- @bm_pid: PID that runs the benchmark
- Measures llc occupancy from resctrl and writes the results into @filename.
llc -> LLC
- @bm_pid is written to the results file along with the measured value.
- Return: =0 on success. <0 on failure.
same issue ?
Reinette