Hi Yifan, On 3/3/26 8:03 PM, Yifan Wu wrote: > Currently, the resctrl_val() function sets CPU affinity and writes > the parent process's PID into the control and monitoring groups in the > resctrl file system before forking the benchmark child process. This > causes several issues: > > 1. Writing the parent process's PID into the resctrl control group > unnecessarily affects the behavior of the parent process, while the > control group should only apply to the benchmark child process. > > 2. Writing the parent process's PID into the resctrl monitoring group > introduces the parent process's activity into the bandwidth measurement, > affecting the accuracy of the monitoring, which should only monitor the > benchmark child process. > > 3. Since only the child process is monitored, the CPU affinity should > also be set only on the child process to ensure that the PMU (Performance > Monitoring Unit) can count memory bandwidth from the benchmark process.
A child process inherits its parent's CPU affinity mask, no? > > 4. When the parent and child processes are scheduled on the same CPU, > the parent process's activity may interfere with the monitoring of > the child process. This is particularly problematic in some ARM MPAM Which tests are encountering issues? For MBM and MBA I do not think this matters since the tests just compare perf and resctrl numbers and perf will already contain all bandwidth associated with the PMU so isolation here may require to potentially take all traffic off a socket. CMT tests may see some impact since it compares cache occupancy to the size of the buffer being read. I do have a pending series that aims to address some issues with the CMT test: https://lore.kernel.org/linux-patches/[email protected]/ > implementations, where memory bandwidth monitoring real-time values. When > the child process is preempted off the CPU, this results in inaccurate > monitoring. This motivation is not clear to me. Could you please elaborate how inaccurate this monitoring gets? The child process may be preempted but it should not be moved from the CPU in its affinity mask. > > This commit moves the CPU affinity and resctrl FS setup to the child > process after fork(), ensuring these settings only affect the benchmark > process, thereby maintaining measurement accuracy and making the > implementation more portable across platforms. > > Signed-off-by: Yifan Wu <[email protected]> > --- > tools/testing/selftests/resctrl/resctrl_val.c | 68 +++++++++++-------- > 1 file changed, 39 insertions(+), 29 deletions(-) > > diff --git a/tools/testing/selftests/resctrl/resctrl_val.c > b/tools/testing/selftests/resctrl/resctrl_val.c > index 7c08e936572d..85ac96c7cb8f 100644 > --- a/tools/testing/selftests/resctrl/resctrl_val.c > +++ b/tools/testing/selftests/resctrl/resctrl_val.c > @@ -545,7 +545,6 @@ int resctrl_val(const struct resctrl_test *test, > cpu_set_t old_affinity; > int domain_id; > int ret = 0; > - pid_t ppid; > > if (strcmp(param->filename, "") == 0) > sprintf(param->filename, "stdio"); > @@ -556,22 +555,10 @@ int resctrl_val(const struct resctrl_test *test, > return ret; > } > > - ppid = getpid(); > - > - /* Taskset test to specified CPU. */ > - ret = taskset_benchmark(ppid, uparams->cpu, &old_affinity); > - if (ret) > - return ret; > - > - /* Write test to specified control & monitoring group in resctrl FS. */ > - ret = write_bm_pid_to_resctrl(ppid, param->ctrlgrp, param->mongrp); > - if (ret) > - goto reset_affinity; > - > if (param->init) { > ret = param->init(param, domain_id); write_bm_pid_to_resctrl() does more than just write the task ID to the tasks file, it also creates the resource groups in its parameters. By moving it after the param->init() callback it prevents any test specific initialization from being done in the test's resource groups. > if (ret) > - goto reset_affinity; > + return ret; > } > > /* > @@ -586,10 +573,8 @@ int resctrl_val(const struct resctrl_test *test, > if (param->fill_buf) { > buf = alloc_buffer(param->fill_buf->buf_size, > param->fill_buf->memflush); This is the buffer on which the workload will operate and by having different CPU affinity between parent and child the buffer may be created in a separate domain. Consider for example if I apply just this patch on a test system and try out the MBM test it now fails: 1..1 # Starting MBM test ... # Mounting resctrl to "/sys/fs/resctrl" # Benchmark PID: 6127 # Writing benchmark parameters to resctrl FS # Write schema "MB:0=100" to resctrl FS # Checking for pass/fail # Fail: Check MBM diff within 15% # avg_diff_per: 100% # Span (MB): 1280 # avg_bw_imc: 50 # avg_bw_resc: 0 not ok 1 MBM: test What happened here is that the buffer was created by the parent in one domain but the child's affinity was set to another domain. The test assumes that it can count local memory and all of those counts are zero for resctrl since the memory is not local. Similarly low numbers from perf for same reason. > - if (!buf) { > - ret = -ENOMEM; > - goto reset_affinity; > - } > + if (!buf) > + return -ENOMEM; > } > While isolating just the workload to its own resource group may be reasonable it is not clear to me what problem this patch aims to solve. Reinette
