Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] selftests/cgroup: hog_cpus_timed() never calls pthread_attr_destroy() --
commit 3e0ad2c93c22b3e844f853e154c385c491c1c5ca Author: Eva Kurchatova <[email protected]> selftests/cgroup: Pin the hogs of test_cpucg_weight_overprovisioned This patch pins each hog thread in test_cpucg_weight_overprovisioned() to one CPU. The threads of each cgroup are spread one per allowed CPU, so the test checks the cgroup weights and not the load balancer's placement. > diff --git a/tools/testing/selftests/cgroup/test_cpu.c > b/tools/testing/selftests/cgroup/test_cpu.c > --- a/tools/testing/selftests/cgroup/test_cpu.c > +++ b/tools/testing/selftests/cgroup/test_cpu.c [ ... ] > @@ -136,7 +138,13 @@ static int hog_cpus_timed(const char *cgroup, void *arg) > struct timespec ts_run = param->ts; > struct timespec ts_remaining = ts_run; > struct timespec ts_start; > - int i, ret; > + pthread_attr_t attr; > + cpu_set_t cpus, one; > + int i, ret, cpu = -1; > + /* Unpinned, as before, where the mask does not fit in a cpu_set_t */ > + bool pin = param->pin && !sched_getaffinity(0, sizeof(cpus), &cpus); > + > + pthread_attr_init(&attr); [Severity: Low] Is the pthread_attr_t initialized here ever released? hog_cpus_timed() calls pthread_attr_init(&attr) unconditionally, but none of its exit paths call pthread_attr_destroy(&attr). This covers the final "return 0", the early return after a failed pthread_attr_setaffinity_np(), the return after a failed pthread_create(), and the clock_gettime()/nanosleep() failure returns. With pin set, glibc's pthread_attr_setaffinity_np() allocates a copy of the cpuset inside attr, and pthread_attr_destroy() is what frees it. The return value of pthread_attr_init() is also not checked. The impact is small, since hog_cpus_timed() runs in a short-lived child forked by cg_run_nowait() and the memory is reclaimed on exit. Would it be worth adding a pthread_attr_destroy(&attr) on the exit paths? > @@ -145,7 +153,18 @@ static int hog_cpus_timed(const char *cgroup, void *arg) > for (i = 0; i < param->nprocs; i++) { > pthread_t tid; > > - ret = pthread_create(&tid, NULL, &hog_cpu_thread_func, NULL); > + /* One hog per allowed CPU, in turn */ > + if (pin) { > + do { > + cpu = (cpu + 1) % CPU_SETSIZE; > + } while (!CPU_ISSET(cpu, &cpus)); > + CPU_ZERO(&one); > + CPU_SET(cpu, &one); > + ret = pthread_attr_setaffinity_np(&attr, sizeof(one), > &one); > + if (ret != 0) > + return ret; > + } > + ret = pthread_create(&tid, &attr, &hog_cpu_thread_func, NULL); > if (ret != 0) > return ret; > } [ ... ] -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=1 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
