On 08-Apr-2022, at 12:31 AM, Shuah Khan skhan@linuxfoundation.org wrote:
On 4/7/22 12:40 PM, Athira Rajeev wrote:
The selftest "mqueue/mq_perf_tests.c" use CPU_ALLOC to allocate CPU set. This cpu set is used further in pthread_attr_setaffinity_np and by pthread_create in the code. But in current code, allocated cpu set is not freed. Fix this issue by adding CPU_FREE in the "shutdown" function which is called in most of the error/exit path for the cleanup. Also add CPU_FREE in some of the error paths where shutdown is not called. Fixes: 7820b0715b6f ("tools/selftests: add mq_perf_tests") Signed-off-by: Athira Rajeev atrajeev@linux.vnet.ibm.com
Changelog: From v1 -> v2: Addressed review comment from Shuah Khan to add CPU_FREE in other exit paths where it is needed
Thank you. I have a couple of comments on making the error paths simpler. Please see below.
tools/testing/selftests/mqueue/mq_perf_tests.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c b/tools/testing/selftests/mqueue/mq_perf_tests.c index b019e0b8221c..182434c7898d 100644 --- a/tools/testing/selftests/mqueue/mq_perf_tests.c +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c @@ -180,6 +180,9 @@ void shutdown(int exit_val, char *err_cause, int line_no) if (in_shutdown++) return;
- /* Free the cpu_set allocated using CPU_ALLOC in main function */
- CPU_FREE(cpu_set);
- for (i = 0; i < num_cpus_to_pin; i++) if (cpu_threads[i]) { pthread_kill(cpu_threads[i], SIGUSR1);
@@ -589,6 +592,7 @@ int main(int argc, char *argv[]) cpu_set)) { fprintf(stderr, "Any given CPU may " "only be given once.\n");
CPU_FREE(cpu_set);This could be done in a common error path handling.
exit(1); } else CPU_SET_S(cpus_to_pin[cpu],@@ -607,6 +611,7 @@ int main(int argc, char *argv[]) queue_path = malloc(strlen(option) + 2); if (!queue_path) { perror("malloc()");
CPU_FREE(cpu_set);This could be done in a common error path handling.
exit(1); } queue_path[0] = '/';@@ -619,6 +624,7 @@ int main(int argc, char *argv[]) } if (continuous_mode && num_cpus_to_pin == 0) {
CPU_FREE(cpu_set);This could be done in a common error path handling.
fprintf(stderr, "Must pass at least one CPU to continuous " "mode.\n"); poptPrintUsage(popt_context, stderr, 0);@@ -628,10 +634,12 @@ int main(int argc, char *argv[]) cpus_to_pin[0] = cpus_online - 1; }
- if (getuid() != 0)
- if (getuid() != 0) {
CPU_FREE(cpu_set);- }
Why not move this check before CPU_ALLOC and make this the very first check in main()?
With this change the other places where CPU_FREE is added right before exit(1). Something like this:
err_code: CPU_FREE(cpu_set); exit(code)
Hi Shuah
Thanks for the comments. Addressed these in V3
Athira
max_msgs = fopen(MAX_MSGS, "r+"); max_msgsize = fopen(MAX_MSGSIZE, "r+");
thanks, -- Shuah