* [PATCH] torture-test modules: Remove unnecessary "ret" variables @ 2018-09-22 1:21 Pierce Griffiths 2018-09-22 21:26 ` Paul E. McKenney 0 siblings, 1 reply; 4+ messages in thread From: Pierce Griffiths @ 2018-09-22 1:21 UTC (permalink / raw) Cc: pierceagriffiths, Davidlohr Bueso, Paul E. McKenney, Josh Triplett, linux-kernel Remove return variables (declared as "ret") in cases where, depending on whether a condition evaluates as true, the result of a function call can be immediately returned instead of storing the result in the return variable. When the condition evaluates as false, the constant initially stored in the return variable at declaration is returned instead. Signed-off-by: Pierce Griffiths <pierceagriffiths@gmail.com> --- kernel/torture.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/kernel/torture.c b/kernel/torture.c index 1ac24a826589..f4cec6db7f3c 100644 --- a/kernel/torture.c +++ b/kernel/torture.c @@ -233,16 +233,15 @@ torture_onoff(void *arg) */ int torture_onoff_init(long ooholdoff, long oointerval) { - int ret = 0; - #ifdef CONFIG_HOTPLUG_CPU onoff_holdoff = ooholdoff; onoff_interval = oointerval; if (onoff_interval <= 0) return 0; - ret = torture_create_kthread(torture_onoff, NULL, onoff_task); -#endif /* #ifdef CONFIG_HOTPLUG_CPU */ - return ret; + return torture_create_kthread(torture_onoff, NULL, onoff_task); +#else /* #ifdef CONFIG_HOTPLUG_CPU */ + return 0; +#endif /* #else #ifdef CONFIG_HOTPLUG_CPU */ } EXPORT_SYMBOL_GPL(torture_onoff_init); @@ -513,15 +512,13 @@ static int torture_shutdown(void *arg) */ int torture_shutdown_init(int ssecs, void (*cleanup)(void)) { - int ret = 0; - torture_shutdown_hook = cleanup; if (ssecs > 0) { shutdown_time = ktime_add(ktime_get(), ktime_set(ssecs, 0)); - ret = torture_create_kthread(torture_shutdown, NULL, + return torture_create_kthread(torture_shutdown, NULL, shutdown_task); } - return ret; + return 0; } EXPORT_SYMBOL_GPL(torture_shutdown_init); @@ -619,13 +616,10 @@ static int torture_stutter(void *arg) /* * Initialize and kick off the torture_stutter kthread. */ -int torture_stutter_init(int s) +int torture_stutter_init(const int s) { - int ret; - stutter = s; - ret = torture_create_kthread(torture_stutter, NULL, stutter_task); - return ret; + return torture_create_kthread(torture_stutter, NULL, stutter_task); } EXPORT_SYMBOL_GPL(torture_stutter_init); -- 2.19.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] torture-test modules: Remove unnecessary "ret" variables 2018-09-22 1:21 [PATCH] torture-test modules: Remove unnecessary "ret" variables Pierce Griffiths @ 2018-09-22 21:26 ` Paul E. McKenney 2018-09-25 19:47 ` Pierce Griffiths 0 siblings, 1 reply; 4+ messages in thread From: Paul E. McKenney @ 2018-09-22 21:26 UTC (permalink / raw) To: Pierce Griffiths; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel On Fri, Sep 21, 2018 at 08:21:31PM -0500, Pierce Griffiths wrote: > Remove return variables (declared as "ret") in cases where, > depending on whether a condition evaluates as true, the result of a > function call can be immediately returned instead of storing the result in > the return variable. When the condition evaluates as false, the constant > initially stored in the return variable at declaration is returned instead. > > Signed-off-by: Pierce Griffiths <pierceagriffiths@gmail.com> Not bad, as it does safe a couple of lines. Is it possible to save a few more by using "if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))" instead of the #ifdef? Or does that end up breaking the build for either CONFIG_HOTPLUG_CPU=y or CONFIG_HOTPLUG_CPU=n builds? Thanx, Paul > --- > kernel/torture.c | 22 ++++++++-------------- > 1 file changed, 8 insertions(+), 14 deletions(-) > > diff --git a/kernel/torture.c b/kernel/torture.c > index 1ac24a826589..f4cec6db7f3c 100644 > --- a/kernel/torture.c > +++ b/kernel/torture.c > @@ -233,16 +233,15 @@ torture_onoff(void *arg) > */ > int torture_onoff_init(long ooholdoff, long oointerval) > { > - int ret = 0; > - > #ifdef CONFIG_HOTPLUG_CPU > onoff_holdoff = ooholdoff; > onoff_interval = oointerval; > if (onoff_interval <= 0) > return 0; > - ret = torture_create_kthread(torture_onoff, NULL, onoff_task); > -#endif /* #ifdef CONFIG_HOTPLUG_CPU */ > - return ret; > + return torture_create_kthread(torture_onoff, NULL, onoff_task); > +#else /* #ifdef CONFIG_HOTPLUG_CPU */ > + return 0; > +#endif /* #else #ifdef CONFIG_HOTPLUG_CPU */ > } > EXPORT_SYMBOL_GPL(torture_onoff_init); > > @@ -513,15 +512,13 @@ static int torture_shutdown(void *arg) > */ > int torture_shutdown_init(int ssecs, void (*cleanup)(void)) > { > - int ret = 0; > - > torture_shutdown_hook = cleanup; > if (ssecs > 0) { > shutdown_time = ktime_add(ktime_get(), ktime_set(ssecs, 0)); > - ret = torture_create_kthread(torture_shutdown, NULL, > + return torture_create_kthread(torture_shutdown, NULL, > shutdown_task); > } > - return ret; > + return 0; > } > EXPORT_SYMBOL_GPL(torture_shutdown_init); > > @@ -619,13 +616,10 @@ static int torture_stutter(void *arg) > /* > * Initialize and kick off the torture_stutter kthread. > */ > -int torture_stutter_init(int s) > +int torture_stutter_init(const int s) > { > - int ret; > - > stutter = s; > - ret = torture_create_kthread(torture_stutter, NULL, stutter_task); > - return ret; > + return torture_create_kthread(torture_stutter, NULL, stutter_task); > } > EXPORT_SYMBOL_GPL(torture_stutter_init); > > -- > 2.19.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] torture-test modules: Remove unnecessary "ret" variables 2018-09-22 21:26 ` Paul E. McKenney @ 2018-09-25 19:47 ` Pierce Griffiths 2018-09-26 22:09 ` Paul E. McKenney 0 siblings, 1 reply; 4+ messages in thread From: Pierce Griffiths @ 2018-09-25 19:47 UTC (permalink / raw) To: Paul E. McKenney; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel Paul, The #ifdef cannot be replaced, as this would break the build for CONFIG_HOTPLUG_CPU=n builds. There are assignments and references to two global variables, a struct, and a function that are declared/defined in a large "#ifdef CONFIG_HOTPLUG_CPU" block which ends directly above torture_onoff_init(). IS_ENABLED() would cause these references to be present at compile time, but not their declarations or definitions, which would certainly cause a compilation error. On Sat, Sep 22, 2018 at 02:26:30PM -0700, Paul E. McKenney wrote: > On Fri, Sep 21, 2018 at 08:21:31PM -0500, Pierce Griffiths wrote: > > Remove return variables (declared as "ret") in cases where, > > depending on whether a condition evaluates as true, the result of a > > function call can be immediately returned instead of storing the result in > > the return variable. When the condition evaluates as false, the constant > > initially stored in the return variable at declaration is returned instead. > > > > Signed-off-by: Pierce Griffiths <pierceagriffiths@gmail.com> > > Not bad, as it does safe a couple of lines. Is it possible to save > a few more by using "if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))" instead > of the #ifdef? Or does that end up breaking the build for either > CONFIG_HOTPLUG_CPU=y or CONFIG_HOTPLUG_CPU=n builds? > > Thanx, Paul > > > --- > > kernel/torture.c | 22 ++++++++-------------- > > 1 file changed, 8 insertions(+), 14 deletions(-) > > > > diff --git a/kernel/torture.c b/kernel/torture.c > > index 1ac24a826589..f4cec6db7f3c 100644 > > --- a/kernel/torture.c > > +++ b/kernel/torture.c > > @@ -233,16 +233,15 @@ torture_onoff(void *arg) > > */ > > int torture_onoff_init(long ooholdoff, long oointerval) > > { > > - int ret = 0; > > - > > #ifdef CONFIG_HOTPLUG_CPU > > onoff_holdoff = ooholdoff; > > onoff_interval = oointerval; > > if (onoff_interval <= 0) > > return 0; > > - ret = torture_create_kthread(torture_onoff, NULL, onoff_task); > > -#endif /* #ifdef CONFIG_HOTPLUG_CPU */ > > - return ret; > > + return torture_create_kthread(torture_onoff, NULL, onoff_task); > > +#else /* #ifdef CONFIG_HOTPLUG_CPU */ > > + return 0; > > +#endif /* #else #ifdef CONFIG_HOTPLUG_CPU */ > > } > > EXPORT_SYMBOL_GPL(torture_onoff_init); > > > > @@ -513,15 +512,13 @@ static int torture_shutdown(void *arg) > > */ > > int torture_shutdown_init(int ssecs, void (*cleanup)(void)) > > { > > - int ret = 0; > > - > > torture_shutdown_hook = cleanup; > > if (ssecs > 0) { > > shutdown_time = ktime_add(ktime_get(), ktime_set(ssecs, 0)); > > - ret = torture_create_kthread(torture_shutdown, NULL, > > + return torture_create_kthread(torture_shutdown, NULL, > > shutdown_task); > > } > > - return ret; > > + return 0; > > } > > EXPORT_SYMBOL_GPL(torture_shutdown_init); > > > > @@ -619,13 +616,10 @@ static int torture_stutter(void *arg) > > /* > > * Initialize and kick off the torture_stutter kthread. > > */ > > -int torture_stutter_init(int s) > > +int torture_stutter_init(const int s) > > { > > - int ret; > > - > > stutter = s; > > - ret = torture_create_kthread(torture_stutter, NULL, stutter_task); > > - return ret; > > + return torture_create_kthread(torture_stutter, NULL, stutter_task); > > } > > EXPORT_SYMBOL_GPL(torture_stutter_init); > > > > -- > > 2.19.0 > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] torture-test modules: Remove unnecessary "ret" variables 2018-09-25 19:47 ` Pierce Griffiths @ 2018-09-26 22:09 ` Paul E. McKenney 0 siblings, 0 replies; 4+ messages in thread From: Paul E. McKenney @ 2018-09-26 22:09 UTC (permalink / raw) To: Pierce Griffiths; +Cc: Davidlohr Bueso, Josh Triplett, linux-kernel On Tue, Sep 25, 2018 at 02:47:08PM -0500, Pierce Griffiths wrote: > Paul, > The #ifdef cannot be replaced, as this would break the build for > CONFIG_HOTPLUG_CPU=n builds. There are assignments and references to two > global variables, a struct, and a function that are declared/defined in > a large "#ifdef CONFIG_HOTPLUG_CPU" block which ends directly above > torture_onoff_init(). IS_ENABLED() would cause these references to be > present at compile time, but not their declarations or definitions, > which would certainly cause a compilation error. Good point, and thank you! I have queued the original patch with a minor edit to the Subject line. It will make it to my -rcu tree once I find a decent internet connection. Thanx, Paul > On Sat, Sep 22, 2018 at 02:26:30PM -0700, Paul E. McKenney wrote: > > On Fri, Sep 21, 2018 at 08:21:31PM -0500, Pierce Griffiths wrote: > > > Remove return variables (declared as "ret") in cases where, > > > depending on whether a condition evaluates as true, the result of a > > > function call can be immediately returned instead of storing the result in > > > the return variable. When the condition evaluates as false, the constant > > > initially stored in the return variable at declaration is returned instead. > > > > > > Signed-off-by: Pierce Griffiths <pierceagriffiths@gmail.com> > > > > Not bad, as it does safe a couple of lines. Is it possible to save > > a few more by using "if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))" instead > > of the #ifdef? Or does that end up breaking the build for either > > CONFIG_HOTPLUG_CPU=y or CONFIG_HOTPLUG_CPU=n builds? > > > > Thanx, Paul > > > > > --- > > > kernel/torture.c | 22 ++++++++-------------- > > > 1 file changed, 8 insertions(+), 14 deletions(-) > > > > > > diff --git a/kernel/torture.c b/kernel/torture.c > > > index 1ac24a826589..f4cec6db7f3c 100644 > > > --- a/kernel/torture.c > > > +++ b/kernel/torture.c > > > @@ -233,16 +233,15 @@ torture_onoff(void *arg) > > > */ > > > int torture_onoff_init(long ooholdoff, long oointerval) > > > { > > > - int ret = 0; > > > - > > > #ifdef CONFIG_HOTPLUG_CPU > > > onoff_holdoff = ooholdoff; > > > onoff_interval = oointerval; > > > if (onoff_interval <= 0) > > > return 0; > > > - ret = torture_create_kthread(torture_onoff, NULL, onoff_task); > > > -#endif /* #ifdef CONFIG_HOTPLUG_CPU */ > > > - return ret; > > > + return torture_create_kthread(torture_onoff, NULL, onoff_task); > > > +#else /* #ifdef CONFIG_HOTPLUG_CPU */ > > > + return 0; > > > +#endif /* #else #ifdef CONFIG_HOTPLUG_CPU */ > > > } > > > EXPORT_SYMBOL_GPL(torture_onoff_init); > > > > > > @@ -513,15 +512,13 @@ static int torture_shutdown(void *arg) > > > */ > > > int torture_shutdown_init(int ssecs, void (*cleanup)(void)) > > > { > > > - int ret = 0; > > > - > > > torture_shutdown_hook = cleanup; > > > if (ssecs > 0) { > > > shutdown_time = ktime_add(ktime_get(), ktime_set(ssecs, 0)); > > > - ret = torture_create_kthread(torture_shutdown, NULL, > > > + return torture_create_kthread(torture_shutdown, NULL, > > > shutdown_task); > > > } > > > - return ret; > > > + return 0; > > > } > > > EXPORT_SYMBOL_GPL(torture_shutdown_init); > > > > > > @@ -619,13 +616,10 @@ static int torture_stutter(void *arg) > > > /* > > > * Initialize and kick off the torture_stutter kthread. > > > */ > > > -int torture_stutter_init(int s) > > > +int torture_stutter_init(const int s) > > > { > > > - int ret; > > > - > > > stutter = s; > > > - ret = torture_create_kthread(torture_stutter, NULL, stutter_task); > > > - return ret; > > > + return torture_create_kthread(torture_stutter, NULL, stutter_task); > > > } > > > EXPORT_SYMBOL_GPL(torture_stutter_init); > > > > > > -- > > > 2.19.0 > > > > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-27 4:28 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-22 1:21 [PATCH] torture-test modules: Remove unnecessary "ret" variables Pierce Griffiths 2018-09-22 21:26 ` Paul E. McKenney 2018-09-25 19:47 ` Pierce Griffiths 2018-09-26 22:09 ` Paul E. McKenney
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).