diff mbox series

sched: core: Simplify cpuset_cpumask_can_shrink()

Message ID 20230518203416.3323-1-zeming@nfschina.com
State New
Headers
Series sched: core: Simplify cpuset_cpumask_can_shrink() |

Commit Message

Li zeming May 18, 2023, 8:34 p.m. UTC 
  Remove useless intermediate variable "ret" and its initialization.
Directly return dl_cpuset_cpumask_can_shrink() result.

Signed-off-by: Li zeming <zeming@nfschina.com>
---
 kernel/sched/core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Comments

Mukesh Ojha May 18, 2023, 1:44 p.m. UTC | #1 
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 <zeming@nfschina.com>

Acked-by : Mukesh Ojha <quic_mojha@quicinc.com>

-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,
Valentin Schneider June 12, 2023, 10:15 a.m. UTC | #2 
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 <vschneid@redhat.com>

> Signed-off-by: Li zeming <zeming@nfschina.com>
> ---
>  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
diff mbox series

Patch

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,