[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200527011413.GD149611@google.com>
Date: Tue, 26 May 2020 21:14:13 -0400
From: Joel Fernandes <joel@...lfernandes.org>
To: Ingo Molnar <mingo@...nel.org>
Cc: linux-kernel@...r.kernel.org,
"Paul E. McKenney" <paulmck@...nel.org>,
linux-tip-commits@...r.kernel.org, x86 <x86@...nel.org>
Subject: Re: [PATCH] rcu/performance: Fix kfree_perf_init() build warning on
32-bit kernels
On Tue, May 26, 2020 at 08:27:44PM +0200, Ingo Molnar wrote:
[...]
> ./include/linux/kern_levels.h:5:18: warning: format ‘%lu’ expects argument
> of type ‘long unsigned int’, but argument 2 has type ‘unsigned int’
> [-Wformat=] 5 | #define KERN_SOH "\001" /* ASCII Start Of Header */ |
> ^~~~~~
> ./include/linux/kern_levels.h:9:20: note: in expansion of macro ‘KERN_SOH’
> 9 | #define KERN_ALERT KERN_SOH "1" /* action must be taken immediately */
> | ^~~~~~~~
> ./include/linux/printk.h:295:9: note: in expansion of macro ‘KERN_ALERT’
> 295 | printk(KERN_ALERT pr_fmt(fmt), ##__VA_ARGS__)
> | ^~~~~~~~~~
> kernel/rcu/rcuperf.c:726:2: note: in expansion of macro ‘pr_alert’
> 726 | pr_alert("kfree object size=%lu\n", kfree_mult * sizeof(struct kfree_obj));
> | ^~~~~~~~
> kernel/rcu/rcuperf.c:726:32: note: format string is defined here
> 726 | pr_alert("kfree object size=%lu\n", kfree_mult * sizeof(struct kfree_obj));
> | ~~^
> | |
> | long unsigned int
> | %u
>
>
> The reason for the warning is that both kfree_mult and sizeof() are
> 'int' types on 32-bit kernels, while the format string expects a long.
>
> Instead of casting the type to long or tweaking the format string, the
> most straightforward solution is to upgrade kfree_mult to a long.
> Since this depends on CONFIG_RCU_PERF_TEST
Thanks for fixing it.
> BTW., could we please also rename this code from 'PERF_TEST'/'perf test'
> to 'PERFORMANCE_TEST'/'performance test'? At first glance I always
> mistakenly believe that it's somehow related to perf, while it isn't. =B-)
Would it be better to call it 'RCUPERF_TEST' instead of the
'RCU_PERFORMANCE_TEST' you are proposing? I feel the word 'PERFORMANCE' is
too long. Also, 'rcuperf test' instead of the 'rcu performance test' you are
proposing. I am Ok with doing it however you and Paul want it though, let me
know.
Paul, should I send you a renaming patch for the new performance tests as
well (which I believe should be in the -dev branch).
thanks,
- Joel
>
> Thanks,
>
> Ingo
>
> Signed-off-by: Ingo Molnar <mingo@...nel.org>
>
> kernel/rcu/rcuperf.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c
> index 16dd1e6b7c09..221a0a3810e4 100644
> --- a/kernel/rcu/rcuperf.c
> +++ b/kernel/rcu/rcuperf.c
> @@ -88,7 +88,7 @@ torture_param(bool, shutdown, RCUPERF_SHUTDOWN,
> torture_param(int, verbose, 1, "Enable verbose debugging printk()s");
> torture_param(int, writer_holdoff, 0, "Holdoff (us) between GPs, zero to disable");
> torture_param(int, kfree_rcu_test, 0, "Do we run a kfree_rcu() perf test?");
> -torture_param(int, kfree_mult, 1, "Multiple of kfree_obj size to allocate.");
> +torture_param(long, kfree_mult, 1, "Multiple of kfree_obj size to allocate.");
>
> static char *perf_type = "rcu";
> module_param(perf_type, charp, 0444);
Powered by blists - more mailing lists