Hi all,
This series does a number of cleanups into resctrl_val() and
generalizes it by removing test name specific handling from the
function.
v7:
- Truly use "bound to", not bounded to.
- Fix separator to use 3 dashes
v6:
- Adjust closing/rollback of the IMC perf
- Move the comment in measure_vals() to function level
- Capitalize MBM
- binded to -> bound to
- Language tweak into kerneldoc
- Removed stale paragraph from commit message
v5:
- Open mem bw file only once and use rewind().
- Add \n to mem bw file read to allow reading fresh values from the file.
- Return 0 if create_grp() is given NULL grp_name (matches the original
behavior). Mention this in function's kerneldoc.
- Cast pid_t to int before printing with %d.
- Caps/typo fixes to kerneldoc and commit messages.
- Use imperative tone in commit messages and improve them based on points
that came up during review.
v4:
- Merged close fix into IMC READ+WRITE rework patch
- Add loop to reset imc_counters_config fds to -1 to be able know which
need closing
- Introduce perf_close_imc_mem_bw() to close fds
- Open resctrl mem bw file (twice) beforehand to avoid opening it during
the test
- Remove MBM .mongrp setup
- Remove mongrp from CMT test
v3:
- Rename init functions to <testname>_init()
- Replace for loops with READ+WRITE statements for clarity
- Don't drop Return: entry from perf_open_imc_mem_bw() func comment
- New patch: Fix closing of IMC fds in case of error
- New patch: Make "bandwidth" consistent in comments & prints
- New patch: Simplify mem bandwidth file code
- Remove wrong comment
- Changed grp_name check to return -1 on fail (internal sanity check)
v2:
- Resolved conflicts with kselftest/next
- Spaces -> tabs correction
Ilpo Järvinen (16):
selftests/resctrl: Fix closing IMC fds on error and open-code R+W
instead of loops
selftests/resctrl: Calculate resctrl FS derived mem bw over sleep(1)
only
selftests/resctrl: Make "bandwidth" consistent in comments & prints
selftests/resctrl: Consolidate get_domain_id() into resctrl_val()
selftests/resctrl: Use correct type for pids
selftests/resctrl: Cleanup bm_pid and ppid usage & limit scope
selftests/resctrl: Rename measure_vals() to measure_mem_bw_vals() &
document
selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM
tests
selftests/resctrl: Add ->measure() callback to resctrl_val_param
selftests/resctrl: Add ->init() callback into resctrl_val_param
selftests/resctrl: Simplify bandwidth report type handling
selftests/resctrl: Make some strings passed to resctrlfs functions
const
selftests/resctrl: Convert ctrlgrp & mongrp to pointers
selftests/resctrl: Remove mongrp from MBA test
selftests/resctrl: Remove mongrp from CMT test
selftests/resctrl: Remove test name comparing from
write_bm_pid_to_resctrl()
tools/testing/selftests/resctrl/cache.c | 10 +-
tools/testing/selftests/resctrl/cat_test.c | 5 +-
tools/testing/selftests/resctrl/cmt_test.c | 22 +-
tools/testing/selftests/resctrl/mba_test.c | 26 +-
tools/testing/selftests/resctrl/mbm_test.c | 26 +-
tools/testing/selftests/resctrl/resctrl.h | 49 ++-
tools/testing/selftests/resctrl/resctrl_val.c | 371 ++++++++----------
tools/testing/selftests/resctrl/resctrlfs.c | 67 ++--
8 files changed, 291 insertions(+), 285 deletions(-)
--
2.39.2
'bm_pid' and 'ppid' are global variables. As they are used by different
processes and in signal handler, they cannot be entirely converted into
local variables.
The scope of those variables can still be reduced into resctrl_val.c
only. As PARENT_EXIT() macro is using 'ppid', make it a function in
resctrl_val.c and pass ppid to it as an argument because it is easier
to understand than using the global variable directly.
Pass 'bm_pid' into measure_vals() instead of relying on the global
variable which helps to make the call signatures of measure_vals() and
measure_llc_resctrl() more similar to each other.
Signed-off-by: Ilpo Järvinen <[email protected]>
Tested-by: Babu Moger <[email protected]>
Reviewed-by: Reinette Chatre <[email protected]>
---
tools/testing/selftests/resctrl/resctrl.h | 9 --------
tools/testing/selftests/resctrl/resctrl_val.c | 23 ++++++++++++-------
2 files changed, 15 insertions(+), 17 deletions(-)
diff --git a/tools/testing/selftests/resctrl/resctrl.h b/tools/testing/selftests/resctrl/resctrl.h
index e6f221236c79..e4b6dc672ecc 100644
--- a/tools/testing/selftests/resctrl/resctrl.h
+++ b/tools/testing/selftests/resctrl/resctrl.h
@@ -43,13 +43,6 @@
#define DEFAULT_SPAN (250 * MB)
-#define PARENT_EXIT() \
- do { \
- kill(ppid, SIGKILL); \
- umount_resctrlfs(); \
- exit(EXIT_FAILURE); \
- } while (0)
-
/*
* user_params: User supplied parameters
* @cpu: CPU number to which the benchmark will be bound to
@@ -127,8 +120,6 @@ struct perf_event_read {
*/
extern volatile int *value_sink;
-extern pid_t bm_pid, ppid;
-
extern char llc_occup_path[1024];
int get_vendor(void);
diff --git a/tools/testing/selftests/resctrl/resctrl_val.c b/tools/testing/selftests/resctrl/resctrl_val.c
index 5704fa3ba202..3cf671cbb7a2 100644
--- a/tools/testing/selftests/resctrl/resctrl_val.c
+++ b/tools/testing/selftests/resctrl/resctrl_val.c
@@ -499,7 +499,7 @@ static int get_mem_bw_resctrl(FILE *fp, unsigned long *mbm_total)
return 0;
}
-pid_t bm_pid, ppid;
+static pid_t bm_pid, ppid;
void ctrlc_handler(int signum, siginfo_t *info, void *ptr)
{
@@ -557,6 +557,13 @@ void signal_handler_unregister(void)
}
}
+static void parent_exit(pid_t ppid)
+{
+ kill(ppid, SIGKILL);
+ umount_resctrlfs();
+ exit(EXIT_FAILURE);
+}
+
/*
* print_results_bw: the memory bandwidth results are stored in a file
* @filename: file that stores the results
@@ -631,7 +638,7 @@ static void initialize_llc_occu_resctrl(const char *ctrlgrp, const char *mongrp,
* 1 sec to measure the data.
*/
static int measure_vals(const struct user_params *uparams,
- struct resctrl_val_param *param)
+ struct resctrl_val_param *param, pid_t bm_pid)
{
unsigned long bw_resc, bw_resc_start, bw_resc_end;
FILE *mem_bw_fp;
@@ -700,7 +707,7 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext)
fp = freopen("/dev/null", "w", stdout);
if (!fp) {
ksft_perror("Unable to direct benchmark status to /dev/null");
- PARENT_EXIT();
+ parent_exit(ppid);
}
if (strcmp(benchmark_cmd[0], "fill_buf") == 0) {
@@ -714,7 +721,7 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext)
once = false;
} else {
ksft_print_msg("Invalid once parameter\n");
- PARENT_EXIT();
+ parent_exit(ppid);
}
if (run_fill_buf(span, memflush, operation, once))
@@ -728,7 +735,7 @@ static void run_benchmark(int signum, siginfo_t *info, void *ucontext)
fclose(stdout);
ksft_print_msg("Unable to run specified benchmark\n");
- PARENT_EXIT();
+ parent_exit(ppid);
}
/*
@@ -807,7 +814,7 @@ int resctrl_val(const struct resctrl_test *test,
/* Register for "SIGUSR1" signal from parent */
if (sigaction(SIGUSR1, &sigact, NULL)) {
ksft_perror("Can't register child for signal");
- PARENT_EXIT();
+ parent_exit(ppid);
}
/* Tell parent that child is ready */
@@ -825,7 +832,7 @@ int resctrl_val(const struct resctrl_test *test,
sigsuspend(&sigact.sa_mask);
ksft_perror("Child is done");
- PARENT_EXIT();
+ parent_exit(ppid);
}
ksft_print_msg("Benchmark PID: %d\n", (int)bm_pid);
@@ -895,7 +902,7 @@ int resctrl_val(const struct resctrl_test *test,
if (!strncmp(resctrl_val, MBM_STR, sizeof(MBM_STR)) ||
!strncmp(resctrl_val, MBA_STR, sizeof(MBA_STR))) {
- ret = measure_vals(uparams, param);
+ ret = measure_vals(uparams, param, bm_pid);
if (ret)
break;
} else if (!strncmp(resctrl_val, CMT_STR, sizeof(CMT_STR))) {
--
2.39.2
Hi Shuah,
Could you please consider this series for inclusion?
Thank you very much.
Reinette
On 6/10/24 8:14 AM, Ilpo Järvinen wrote:
> Hi all,
>
> This series does a number of cleanups into resctrl_val() and
> generalizes it by removing test name specific handling from the
> function.
>
> v7:
> - Truly use "bound to", not bounded to.
> - Fix separator to use 3 dashes
>
> v6:
> - Adjust closing/rollback of the IMC perf
> - Move the comment in measure_vals() to function level
> - Capitalize MBM
> - binded to -> bound to
> - Language tweak into kerneldoc
> - Removed stale paragraph from commit message
>
> v5:
> - Open mem bw file only once and use rewind().
> - Add \n to mem bw file read to allow reading fresh values from the file.
> - Return 0 if create_grp() is given NULL grp_name (matches the original
> behavior). Mention this in function's kerneldoc.
> - Cast pid_t to int before printing with %d.
> - Caps/typo fixes to kerneldoc and commit messages.
> - Use imperative tone in commit messages and improve them based on points
> that came up during review.
>
> v4:
> - Merged close fix into IMC READ+WRITE rework patch
> - Add loop to reset imc_counters_config fds to -1 to be able know which
> need closing
> - Introduce perf_close_imc_mem_bw() to close fds
> - Open resctrl mem bw file (twice) beforehand to avoid opening it during
> the test
> - Remove MBM .mongrp setup
> - Remove mongrp from CMT test
>
> v3:
> - Rename init functions to <testname>_init()
> - Replace for loops with READ+WRITE statements for clarity
> - Don't drop Return: entry from perf_open_imc_mem_bw() func comment
> - New patch: Fix closing of IMC fds in case of error
> - New patch: Make "bandwidth" consistent in comments & prints
> - New patch: Simplify mem bandwidth file code
> - Remove wrong comment
> - Changed grp_name check to return -1 on fail (internal sanity check)
>
> v2:
> - Resolved conflicts with kselftest/next
> - Spaces -> tabs correction
>
>
> Ilpo Järvinen (16):
> selftests/resctrl: Fix closing IMC fds on error and open-code R+W
> instead of loops
> selftests/resctrl: Calculate resctrl FS derived mem bw over sleep(1)
> only
> selftests/resctrl: Make "bandwidth" consistent in comments & prints
> selftests/resctrl: Consolidate get_domain_id() into resctrl_val()
> selftests/resctrl: Use correct type for pids
> selftests/resctrl: Cleanup bm_pid and ppid usage & limit scope
> selftests/resctrl: Rename measure_vals() to measure_mem_bw_vals() &
> document
> selftests/resctrl: Simplify mem bandwidth file code for MBA & MBM
> tests
> selftests/resctrl: Add ->measure() callback to resctrl_val_param
> selftests/resctrl: Add ->init() callback into resctrl_val_param
> selftests/resctrl: Simplify bandwidth report type handling
> selftests/resctrl: Make some strings passed to resctrlfs functions
> const
> selftests/resctrl: Convert ctrlgrp & mongrp to pointers
> selftests/resctrl: Remove mongrp from MBA test
> selftests/resctrl: Remove mongrp from CMT test
> selftests/resctrl: Remove test name comparing from
> write_bm_pid_to_resctrl()
>
> tools/testing/selftests/resctrl/cache.c | 10 +-
> tools/testing/selftests/resctrl/cat_test.c | 5 +-
> tools/testing/selftests/resctrl/cmt_test.c | 22 +-
> tools/testing/selftests/resctrl/mba_test.c | 26 +-
> tools/testing/selftests/resctrl/mbm_test.c | 26 +-
> tools/testing/selftests/resctrl/resctrl.h | 49 ++-
> tools/testing/selftests/resctrl/resctrl_val.c | 371 ++++++++----------
> tools/testing/selftests/resctrl/resctrlfs.c | 67 ++--
> 8 files changed, 291 insertions(+), 285 deletions(-)
>