Re: [PATCH] rcu/performance: Fix kfree_perf_init() build warning on 32-bit kernels

[Paul E. McKenney]

On Tue, May 26, 2020 at 08:27:44PM +0200, Ingo Molnar wrote:
> 
> * tip-bot2 for Joel Fernandes (Google) <tip-bot2@xxxxxxxxxxxxx> wrote:
> 
> > The following commit has been merged into the core/rcu branch of tip:
> > 
> > Commit-ID:     f87dc808009ac86c790031627698ef1a34c31e25
> > Gitweb:        https://git.kernel.org/tip/f87dc808009ac86c790031627698ef1a34c31e25
> > Author:        Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
> > AuthorDate:    Mon, 16 Mar 2020 12:32:26 -04:00
> > Committer:     Paul E. McKenney <paulmck@xxxxxxxxxx>
> > CommitterDate: Mon, 27 Apr 2020 11:02:50 -07:00
> > 
> > rcuperf: Add ability to increase object allocation size
> > 
> > This allows us to increase memory pressure dynamically using a new
> > rcuperf boot command line parameter called 'rcumult'.
> > 
> > Signed-off-by: Joel Fernandes (Google) <joel@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>
> > ---
> >  kernel/rcu/rcuperf.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c
> > index a4a8d09..16dd1e6 100644
> > --- a/kernel/rcu/rcuperf.c
> > +++ b/kernel/rcu/rcuperf.c
> > @@ -88,6 +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.");
> >  
> >  static char *perf_type = "rcu";
> >  module_param(perf_type, charp, 0444);
> > @@ -635,7 +636,7 @@ kfree_perf_thread(void *arg)
> >  		}
> >  
> >  		for (i = 0; i < kfree_alloc_num; i++) {
> > -			alloc_ptr = kmalloc(sizeof(struct kfree_obj), GFP_KERNEL);
> > +			alloc_ptr = kmalloc(kfree_mult * sizeof(struct kfree_obj), GFP_KERNEL);
> >  			if (!alloc_ptr)
> >  				return -ENOMEM;
> >  
> > @@ -722,6 +723,8 @@ kfree_perf_init(void)
> >  		schedule_timeout_uninterruptible(1);
> >  	}
> >  
> > +	pr_alert("kfree object size=%lu\n", kfree_mult * sizeof(struct kfree_obj));
> 
> There's a new build warning on certain 32-bit kernel builds due to 
> this commit:
> 
> In file included from ./include/linux/printk.h:7,
>                  from ./include/linux/kernel.h:15,
>                  from kernel/rcu/rcuperf.c:13:
> kernel/rcu/rcuperf.c: In function âkfree_perf_initâ:
> ./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

Makes sense, and I have queued the patch below, which I am assuming
that you want in the upcoming merge window.  If you don't tell me
otherwise, I will send you an urgent pull request later today.
Or, if you just put it directly into -tip yourself:

Reviewed-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

Either way, please let me know.

> 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-)

Fair enough, especially given that perf was there first and is also way
more heavily used.  ;-)

But I am guessing that this one is OK for the v5.9 merge window.
Either way, I will update as you say.

							Thanx, Paul

------------------------------------------------------------------------

commit 2fbc7d67a2ed108e3ac63296670fecb3a42fddd0
Author: Ingo Molnar <mingo@xxxxxxxxxx>
Date:   Tue May 26 12:10:01 2020 -0700

    rcuperf: Fix kfree_mult to match printk() format
    
    This commit changes the type of kfree_mult from int to long in order
    to match the printk() format on 32-bit systems.
    
    Reported-by: kbuild test robot <lkp@xxxxxxxxx>
    Signed-off-by: Ingo Molnar <mingo@xxxxxxxxxx>
    Signed-off-by: Paul E. McKenney <paulmck@xxxxxxxxxx>

diff --git a/kernel/rcu/rcuperf.c b/kernel/rcu/rcuperf.c
index d906ca9..fb3a1f0 100644
--- a/kernel/rcu/rcuperf.c
+++ b/kernel/rcu/rcuperf.c
@@ -93,7 +93,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);