[LTP] [PATCH] cfs-scheduler: Fixed "make check" errors and warnings.
Samir M
samir@linux.ibm.com
Sun Apr 12 14:35:37 CEST 2026
On 07/04/26 3:24 pm, Petr Vorel wrote:
> Hi Samir,
>
>> Fixed all make check errors and warnings in cfs_bandwidth01.c and
>> hackbench.c to comply with LTP coding style.
> I guess you get that info with make check (which is using checkpatch.pl and
> other code).
>
>> cfs_bandwidth01.c:
>> - Remove initialization of static variable to 0
>> hackbench.c:
> FYI hackbench.c would deserve to rewrite into new C API. With that many of your
> fixes would be not needed because the old code would be deleted.
>
>> - Add SPDX-License-Identifier header
>> - Remove FSF mailing address paragraph
>> - Remove filename from file header
>> - Remove initialization of static variables to 0
>> - Convert zero-length array to C99 flexible array
>> - Modify barf() function to accept variadic arguments
>> - Use __func__ instead of hardcoded function names
>> - Separate assignments from if conditions
>> - Fix pointer declaration spacing
>> - Add blank line after declarations
>> - Fix spacing in macro and struct initialization
>> - Remove unnecessary braces for single statement
>> Both files now pass make check validation with 0 errors and 0 warnings.
>> Signed-off-by: Samir <samir@linux.ibm.com>
>> ---
>> .../sched/cfs-scheduler/cfs_bandwidth01.c | 2 +-
>> .../kernel/sched/cfs-scheduler/hackbench.c | 121 +++++++++---------
>> 2 files changed, 58 insertions(+), 65 deletions(-)
>> diff --git a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> index e52858f8e..8c511f060 100644
>> --- a/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> +++ b/testcases/kernel/sched/cfs-scheduler/cfs_bandwidth01.c
>> @@ -35,7 +35,7 @@
>> static struct tst_cg_group *cg_level2, *cg_level3a, *cg_level3b;
>> static struct tst_cg_group *cg_workers[3];
>> -static int may_have_waiters = 0;
>> +static int may_have_waiters;
> +1
>
>> static void set_cpu_quota(const struct tst_cg_group *const cg,
>> const float quota_percent)
>> diff --git a/testcases/kernel/sched/cfs-scheduler/hackbench.c b/testcases/kernel/sched/cfs-scheduler/hackbench.c
>> index 6f37060aa..6a30e1cc8 100644
>> --- a/testcases/kernel/sched/cfs-scheduler/hackbench.c
>> +++ b/testcases/kernel/sched/cfs-scheduler/hackbench.c
>> @@ -1,51 +1,33 @@
>> -/******************************************************************************/
>> -/* Copyright Rusty Russell, */
>> -/* Copyright Pierre Peiffer */
>> -/* Copyright Zhang, Yanmin, */
>> -/* Copyright Ingo Molnar, */
>> -/* Copyright Arjan van de Ven, */
>> -/* Copyright (c) International Business Machines Corp., 2008 */
>> -/* */
>> -/* This program is free software; you can redistribute it and/or modify */
>> -/* it under the terms of the GNU General Public License as published by */
>> -/* the Free Software Foundation; either version 2 of the License, or */
>> -/* (at your option) any later version. */
>> -/* */
>> -/* This program is distributed in the hope that it will be useful, */
>> -/* but WITHOUT ANY WARRANTY; without even the implied warranty of */
>> -/* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See */
>> -/* the GNU General Public License for more details. */
>> -/* */
>> -/* You should have received a copy of the GNU General Public License */
>> -/* along with this program; if not, write to the Free Software */
>> -/* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA */
>> -/* */
>> -/******************************************************************************/
>> -
>> -/******************************************************************************/
>> -/* */
>> -/* File: hackbench.c */
>> -/* */
>> -/* Description: hackbench tests the Linux scheduler. Test groups of 20 */
>> -/* processes spraying to 20 receivers */
>> -/* */
>> -/* Total Tests: 1 */
>> -/* */
>> -/* Test Name: hackbench01 and hackbench02 */
>> -/* */
>> -/* Test Assertion: */
>> -/* */
>> -/* Author(s): Rusty Russell <rusty@rustcorp.com.au>, */
>> -/* Pierre Peiffer <pierre.peiffer@bull.net>, */
>> -/* Ingo Molnar <mingo@elte.hu>, */
>> -/* Arjan van de Ven <arjan@infradead.org>, */
>> -/* "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>, */
>> -/* Nathan Lynch <ntl@pobox.com> */
>> -/* */
>> -/* History: Included into LTP */
>> -/* - June 26 2008 - Subrata Modak<subrata@linux.vnet.ibm.com>*/
>> -/* */
>> -/******************************************************************************/
>> +// SPDX-License-Identifier: GPL-2.0-or-later
> FYI we add SPDX only to files which has been converted into new LTP API. I'm ok
> to change it, but it would require to add additional cleanup.
>
>> +/*
>> + * Copyright Rusty Russell,
>> + * Copyright Pierre Peiffer
>> + * Copyright Zhang, Yanmin,
>> + * Copyright Ingo Molnar,
>> + * Copyright Arjan van de Ven,
> I know it's not your addition, but I wonder if a copyright without year even
> makes sense.
>
>> + * Copyright (c) International Business Machines Corp., 2008
>> + */
>> +
>> +/*
>> + * Description: hackbench tests the Linux scheduler. Test groups of 20
>> + * processes spraying to 20 receivers
> Missing dot at the end.
>
>> + *
>> + * Total Tests: 1
> Useless, please remove.
>> + *
>> + * Test Name: hackbench01 and hackbench02
> Useless, please remove (it can change anyway).
>> + *
>> + * Test Assertion:
> Useless, please remove.
>> + *
>> + * Author(s): Rusty Russell <rusty@rustcorp.com.au>,
>> + * Pierre Peiffer <pierre.peiffer@bull.net>,
>> + * Ingo Molnar <mingo@elte.hu>,
>> + * Arjan van de Ven <arjan@infradead.org>,
>> + * "Zhang, Yanmin" <yanmin_zhang@linux.intel.com>,
>> + * Nathan Lynch <ntl@pobox.com>
> Hm, how about to add their mails in copyright section and remove this?
>
>> + *
>> + * History: Included into LTP
>> + * - June 26 2008 - Subrata Modak<subrata@linux.vnet.ibm.com>
> I would remove this (we have git history anyway) and move him to copyright
> section.
>
>> + */
>> #include <pthread.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> @@ -58,25 +40,26 @@
>> #include <sys/time.h>
>> #include <sys/poll.h>
>> #include <limits.h>
>> +#include <stdarg.h>
>> -#define SAFE_FREE(p) { if (p) { free(p); (p)=NULL; } }
>> +#define SAFE_FREE(p) { if (p) { free(p); (p) = NULL; } }
>> #define DATASIZE 100
>> static struct sender_context **snd_ctx_tab; /*Table for sender context pointers. */
>> static struct receiver_context **rev_ctx_tab; /*Table for receiver context pointers. */
>> -static int gr_num = 0; /*For group calculation */
>> +static int gr_num; /*For group calculation */
>> static unsigned int loops = 100;
>> /*
>> * 0 means thread mode and others mean process (default)
>> */
>> static unsigned int process_mode = 1;
>> -static int use_pipes = 0;
>> +static int use_pipes;
> +1
>
>> struct sender_context {
>> unsigned int num_fds;
>> int ready_out;
>> int wakefd;
>> - int out_fds[0];
>> + int out_fds[];
>> };
>> struct receiver_context {
>> @@ -86,9 +69,14 @@ struct receiver_context {
>> int wakefd;
>> };
>> -static void barf(const char *msg)
>> +static void barf(const char *fmt, ...)
>> {
>> - fprintf(stderr, "%s (error: %s)\n", msg, strerror(errno));
>> + va_list args;
>> +
>> + va_start(args, fmt);
>> + vfprintf(stderr, fmt, args);
>> + va_end(args);
>> + fprintf(stderr, " (error: %s)\n", strerror(errno));
>> exit(1);
> FYI in new C API we have tst_brk() and tst_res().
>> }
>> @@ -108,18 +96,18 @@ static void fdpair(int fds[2])
>> if (socketpair(AF_UNIX, SOCK_STREAM, 0, fds) == 0)
>> return;
>> }
>> - barf("Creating fdpair");
>> + barf("Creating %s", __func__);
> Adding va_*() just to post the name of the function looks useless for me.
> Please drop this change.
>
>> }
>> /* Block until we're ready to go */
>> static void ready(int ready_out, int wakefd)
>> {
>> char dummy;
>> - struct pollfd pollfd = {.fd = wakefd,.events = POLLIN };
>> + struct pollfd pollfd = {.fd = wakefd, .events = POLLIN };
>> /* Tell them we're ready. */
>> if (write(ready_out, &dummy, 1) != 1)
>> - barf("CLIENT: ready write");
>> + barf("CLIENT: %s write", __func__);
>> /* Wait for "GO" signal */
>> if (poll(&pollfd, 1, -1) != 1)
>> @@ -210,7 +198,8 @@ pthread_t create_worker(void *ctx, void *(*func) (void *))
>> barf("pthread_attr_setstacksize");
>> #endif
>> - if ((err = pthread_create(&childid, &attr, func, ctx)) != 0) {
>> + err = pthread_create(&childid, &attr, func, ctx);
>> + if (err != 0) {
>> fprintf(stderr, "pthread_create failed: %s (%d)\n",
>> strerror(err), err);
>> exit(-1);
>> @@ -235,11 +224,12 @@ void reap_worker(pthread_t id)
>> }
>> /* One group of senders and receivers */
>> -static unsigned int group(pthread_t * pth,
>> +static unsigned int group(pthread_t *pth,
>> unsigned int num_fds, int ready_out, int wakefd)
>> {
>> unsigned int i;
>> struct sender_context *snd_ctx = malloc(sizeof(struct sender_context) + num_fds * sizeof(int));
>> +
>> if (!snd_ctx)
>> barf("malloc()");
>> else
>> @@ -305,8 +295,11 @@ int main(int argc, char *argv[])
>> argv++;
>> }
>> - if (argc >= 2 && (num_groups = atoi(argv[1])) == 0)
>> - print_usage_exit();
>> + if (argc >= 2) {
>> + num_groups = atoi(argv[1]);
>> + if (num_groups == 0)
>> + print_usage_exit();
>> + }
> +1, assigning variables is really better out of if / while,
> although here it's safe, because num_groups is initialized to 10.
>
> Kind regards,
> Petr
>
>> printf("Running with %d*40 (== %d) tasks.\n",
>> num_groups, num_groups * 40);
>> @@ -329,7 +322,7 @@ int main(int argc, char *argv[])
>> snd_ctx_tab = malloc(num_groups * sizeof(void *));
>> rev_ctx_tab = malloc(num_groups * num_fds * sizeof(void *));
>> if (!pth_tab || !snd_ctx_tab || !rev_ctx_tab)
>> - barf("main:malloc()");
>> + barf("%s:malloc()", __func__);
>> fdpair(readyfds);
>> fdpair(wakefds);
>> @@ -363,9 +356,8 @@ int main(int argc, char *argv[])
>> /* free the memory */
>> for (i = 0; i < num_groups; i++) {
>> - for (j = 0; j < num_fds; j++) {
>> + for (j = 0; j < num_fds; j++)
>> SAFE_FREE(rev_ctx_tab[i * num_fds + j])
>> - }
> +1
>> SAFE_FREE(snd_ctx_tab[i]);
>> }
>> SAFE_FREE(pth_tab);
>> @@ -373,3 +365,4 @@ int main(int argc, char *argv[])
>> SAFE_FREE(rev_ctx_tab);
>> exit(0);
>> }
>> +
Hi Petr,
I have incorporated the requested review comments and submitted the
revised version (v2). Thank you for your valuable feedback.
Patch v2:
https://lore.kernel.org/ltp/20260412122842.1074017-1-samir@linux.ibm.com/T/#u
Thanks,
Samir
More information about the ltp
mailing list