* [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).