linux-kernel - Re: [PATCH] x86: The return type of the function could be void

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87v8ujytu2.fsf@redhat.com>
Date:   Fri, 06 May 2022 09:59:49 +0200
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Li kunyu <kunyu@...china.com>
Cc:     kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        Li kunyu <kunyu@...china.com>, pbonzini@...hat.com,
        seanjc@...gle.com, wanpengli@...cent.com, jmattson@...gle.com,
        joro@...tes.org, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com
Subject: Re: [PATCH] x86: The return type of the function could be void

Li kunyu <kunyu@...china.com> writes:

> Perhaps the return value of the function is not used.
> It may be possible to optimize the execution instructions.
>
> Signed-off-by: Li kunyu <kunyu@...china.com>
> ---
>  arch/x86/kvm/hyperv.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 46f9dfb60469..64c0d1f54258 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -608,7 +608,7 @@ static enum hrtimer_restart stimer_timer_callback(struct hrtimer *timer)
>   * a) stimer->count is not equal to 0
>   * b) stimer->config has HV_STIMER_ENABLE flag
>   */
> -static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
> +static void stimer_start(struct kvm_vcpu_hv_stimer *stimer)
>  {
>  	u64 time_now;
>  	ktime_t ktime_now;
> @@ -638,7 +638,7 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
>  			      ktime_add_ns(ktime_now,
>  					   100 * (stimer->exp_time - time_now)),
>  			      HRTIMER_MODE_ABS);
> -		return 0;
> +		return;
>  	}
>  	stimer->exp_time = stimer->count;
>  	if (time_now >= stimer->count) {
> @@ -649,7 +649,7 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
>  		 * the past, it will expire immediately."
>  		 */
>  		stimer_mark_pending(stimer, false);
> -		return 0;
> +		return;
>  	}
>  
>  	trace_kvm_hv_stimer_start_one_shot(hv_stimer_to_vcpu(stimer)->vcpu_id,
> @@ -659,7 +659,6 @@ static int stimer_start(struct kvm_vcpu_hv_stimer *stimer)
>  	hrtimer_start(&stimer->timer,
>  		      ktime_add_ns(ktime_now, 100 * (stimer->count - time_now)),
>  		      HRTIMER_MODE_ABS);
> -	return 0;
>  }
>  
>  static int stimer_set_config(struct kvm_vcpu_hv_stimer *stimer, u64 config,

stimer_start() has only one user so it'll likely get inlined by the
compiler which then will be able to figure out that the return value is
not used and thus the assembley code will likely remain the same but
this is a good cleanup nevertheless, so 

Reviewed-by: Vitaly Kuznetsov <vkuznets@...hat.com>

-- 
Vitaly