Remove useless intermediate variable "ret" and its initialization.
Directly return dl_cpuset_cpumask_can_shrink() result.
Signed-off-by: Li zeming <[email protected]>
---
kernel/sched/core.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index a66960da3f5c..f3f2ece26291 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -9273,14 +9273,10 @@ void __init init_idle(struct task_struct *idle, int cpu)
int cpuset_cpumask_can_shrink(const struct cpumask *cur,
const struct cpumask *trial)
{
- int ret = 1;
-
if (cpumask_empty(cur))
- return ret;
-
- ret = dl_cpuset_cpumask_can_shrink(cur, trial);
+ return 1;
- return ret;
+ return dl_cpuset_cpumask_can_shrink(cur, trial);
}
int task_can_attach(struct task_struct *p,
--
2.18.2
On 5/19/2023 2:04 AM, Li zeming wrote:
> Remove useless intermediate variable "ret" and its initialization.
> Directly return dl_cpuset_cpumask_can_shrink() result.
>
> Signed-off-by: Li zeming <[email protected]>
Acked-by : Mukesh Ojha <[email protected]>
-Mukesh
> ---
> kernel/sched/core.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a66960da3f5c..f3f2ece26291 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -9273,14 +9273,10 @@ void __init init_idle(struct task_struct *idle, int cpu)
> int cpuset_cpumask_can_shrink(const struct cpumask *cur,
> const struct cpumask *trial)
> {
> - int ret = 1;
> -
> if (cpumask_empty(cur))
> - return ret;
> -
> - ret = dl_cpuset_cpumask_can_shrink(cur, trial);
> + return 1;
>
> - return ret;
> + return dl_cpuset_cpumask_can_shrink(cur, trial);
> }
>
> int task_can_attach(struct task_struct *p,
On 19/05/23 04:34, Li zeming wrote:
> Remove useless intermediate variable "ret" and its initialization.
> Directly return dl_cpuset_cpumask_can_shrink() result.
>
FWIW, this sort of intermediate variable can and will be optimized away by
the compiler. It's pretty obvious to see in the objdump (GCC 13.1):
before/core.o: mainline core.o
after/core.o: patched core.o
$ gdb -batch -ex 'disassemble cpuset_cpumask_can_shrink' before/core.o
Dump of assembler code for function cpuset_cpumask_can_shrink:
0x000000000000b260 <+0>: endbr64
0x000000000000b264 <+4>: cmpq $0x0,(%rdi)
0x000000000000b268 <+8>: jne 0xb274 <cpuset_cpumask_can_shrink+20>
0x000000000000b26a <+10>: mov $0x1,%eax
0x000000000000b26f <+15>: jmpq 0xb274 <cpuset_cpumask_can_shrink+20>
0x000000000000b274 <+20>: jmpq 0xb279
$ diff <(gdb -batch -ex 'disassemble cpuset_cpumask_can_shrink' before/core.o) \
<(gdb -batch -ex 'disassemble cpuset_cpumask_can_shrink' after/core.o)
<nothing>
Generally I think it's good to not be afraid to use intermediate variables
if they improve readability, even if they have only one use (e.g. as
parameter to a function with many arguments / long expressions used as
parameters).
In this case though, the intermediate variable really doesn't do much for
readability :-)
Reviewed-by: Valentin Schneider <[email protected]>
> Signed-off-by: Li zeming <[email protected]>
> ---
> kernel/sched/core.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a66960da3f5c..f3f2ece26291 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -9273,14 +9273,10 @@ void __init init_idle(struct task_struct *idle, int cpu)
> int cpuset_cpumask_can_shrink(const struct cpumask *cur,
> const struct cpumask *trial)
> {
> - int ret = 1;
> -
> if (cpumask_empty(cur))
> - return ret;
> -
> - ret = dl_cpuset_cpumask_can_shrink(cur, trial);
> + return 1;
>
> - return ret;
> + return dl_cpuset_cpumask_can_shrink(cur, trial);
> }
>
> int task_can_attach(struct task_struct *p,
> --
> 2.18.2