[PATCH -next] KVM: x86: remove set but not used variable 'called'

[PATCH -next] KVM: x86: remove set but not used variable 'called'

[Mao Wenan]

Fixes gcc '-Wunused-but-set-variable' warning:

arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
used [-Wunused-but-set-variable]

It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
IOAPIC scan request to target vCPUs")

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 arch/x86/kvm/x86.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0d0a682..870f0bc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7908,12 +7908,11 @@ void kvm_make_scan_ioapic_request_mask(struct kvm *kvm,
 				       unsigned long *vcpu_bitmap)
 {
 	cpumask_var_t cpus;
-	bool called;
 
 	zalloc_cpumask_var(&cpus, GFP_ATOMIC);
 
-	called = kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
-					     vcpu_bitmap, cpus);
+	kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
+				    vcpu_bitmap, cpus);
 
 	free_cpumask_var(cpus);
 }
-- 
2.7.4

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Vitaly Kuznetsov]

Mao Wenan <maowenan@huawei.com> writes:

> Fixes gcc '-Wunused-but-set-variable' warning:
>
> arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
> arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
> used [-Wunused-but-set-variable]
>
> It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
> IOAPIC scan request to target vCPUs")

Better expressed as 

Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")

>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  arch/x86/kvm/x86.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0d0a682..870f0bc 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7908,12 +7908,11 @@ void kvm_make_scan_ioapic_request_mask(struct kvm *kvm,
>  				       unsigned long *vcpu_bitmap)
>  {
>  	cpumask_var_t cpus;
> -	bool called;
>  
>  	zalloc_cpumask_var(&cpus, GFP_ATOMIC);
>  
> -	called = kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
> -					     vcpu_bitmap, cpus);
> +	kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
> +				    vcpu_bitmap, cpus);

IMHO as kvm_make_vcpus_request_mask() returns value it would probably
make sense to explicitly show that we're not interested in the result,

(void)kvm_make_vcpus_request_mask()

>  
>  	free_cpumask_var(cpus);
>  }

-- 
Vitaly

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Dan Carpenter]

On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
> Mao Wenan <maowenan@huawei.com> writes:
> 
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
> > arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
> > used [-Wunused-but-set-variable]
> >
> > It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
> > IOAPIC scan request to target vCPUs")
> 
> Better expressed as 
> 
> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
> 

There is sort of a debate about this whether the Fixes tag should be
used if it's only a cleanup.

regards,
dan carpenter

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Vitaly Kuznetsov]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
>> Mao Wenan <maowenan@huawei.com> writes:
>> 
>> > Fixes gcc '-Wunused-but-set-variable' warning:
>> >
>> > arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
>> > arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
>> > used [-Wunused-but-set-variable]
>> >
>> > It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
>> > IOAPIC scan request to target vCPUs")
>> 
>> Better expressed as 
>> 
>> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
>> 
>
> There is sort of a debate about this whether the Fixes tag should be
> used if it's only a cleanup.
>

I have to admit I'm involved in doing backporting sometimes and I really
appreciate Fixes: tags. Just so you know on which side of the debate I
am :-)

-- 
Vitaly

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Dan Carpenter]

On Tue, Nov 19, 2019 at 01:28:32PM +0100, Vitaly Kuznetsov wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
> 
> > On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
> >> Mao Wenan <maowenan@huawei.com> writes:
> >> 
> >> > Fixes gcc '-Wunused-but-set-variable' warning:
> >> >
> >> > arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
> >> > arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
> >> > used [-Wunused-but-set-variable]
> >> >
> >> > It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
> >> > IOAPIC scan request to target vCPUs")
> >> 
> >> Better expressed as 
> >> 
> >> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
> >> 
> >
> > There is sort of a debate about this whether the Fixes tag should be
> > used if it's only a cleanup.
> >
> 
> I have to admit I'm involved in doing backporting sometimes and I really
> appreciate Fixes: tags. Just so you know on which side of the debate I
> am :-)

But we're not going to backport this hopefully?

regards,
dan carpenter

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Vitaly Kuznetsov]

Dan Carpenter <dan.carpenter@oracle.com> writes:

> On Tue, Nov 19, 2019 at 01:28:32PM +0100, Vitaly Kuznetsov wrote:
>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>> 
>> > On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
>> >> Mao Wenan <maowenan@huawei.com> writes:
>> >> 
>> >> > Fixes gcc '-Wunused-but-set-variable' warning:
>> >> >
>> >> > arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
>> >> > arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
>> >> > used [-Wunused-but-set-variable]
>> >> >
>> >> > It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
>> >> > IOAPIC scan request to target vCPUs")
>> >> 
>> >> Better expressed as 
>> >> 
>> >> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
>> >> 
>> >
>> > There is sort of a debate about this whether the Fixes tag should be
>> > used if it's only a cleanup.
>> >
>> 
>> I have to admit I'm involved in doing backporting sometimes and I really
>> appreciate Fixes: tags. Just so you know on which side of the debate I
>> am :-)
>
> But we're not going to backport this hopefully?
>

In case we're speaking about stable@ kernels, 7ee30bc132c6 doesn't look
like a good candidate (to me) but who knows, it may get pulled in
because of some code dependency or some other 'autosel magic'. And
that's when 'Fixes:' tags become handy.

-- 
Vitaly

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Nitesh Narayan Lal]

[-- Attachment #1.1: Type: text/plain, Size: 1731 bytes --]


On 11/19/19 8:25 AM, Vitaly Kuznetsov wrote:
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
>> On Tue, Nov 19, 2019 at 01:28:32PM +0100, Vitaly Kuznetsov wrote:
>>> Dan Carpenter <dan.carpenter@oracle.com> writes:
>>>
>>>> On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
>>>>> Mao Wenan <maowenan@huawei.com> writes:
>>>>>
>>>>>> Fixes gcc '-Wunused-but-set-variable' warning:
>>>>>>
>>>>>> arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
>>>>>> arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
>>>>>> used [-Wunused-but-set-variable]
>>>>>>
>>>>>> It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
>>>>>> IOAPIC scan request to target vCPUs")
>>>>> Better expressed as 
>>>>>
>>>>> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
>>>>>
>>>> There is sort of a debate about this whether the Fixes tag should be
>>>> used if it's only a cleanup.
>>>>
>>> I have to admit I'm involved in doing backporting sometimes and I really
>>> appreciate Fixes: tags. Just so you know on which side of the debate I
>>> am :-)
>> But we're not going to backport this hopefully?
>>
> In case we're speaking about stable@ kernels, 7ee30bc132c6 doesn't look
> like a good candidate (to me) but who knows, it may get pulled in
> because of some code dependency or some other 'autosel magic'. And
> that's when 'Fixes:' tags become handy.

Anything I can improve upon? If required I can send fixes on top of it.
For the build error, I didn't trigger it because I didn't compile with
appropriate flags.
I will make a note for myself for next time.

Thanks, Mao for sending the fix.

>
-- 
Nitesh


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Dan Carpenter]

On Fri, Nov 22, 2019 at 06:58:51AM -0500, Nitesh Narayan Lal wrote:
> For the build error, I didn't trigger it because I didn't compile with
> appropriate flags.

It's going to be a serveral years before we can enable that flag by
default.

regards,
dan carpenter

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Nitesh Narayan Lal]

[-- Attachment #1.1: Type: text/plain, Size: 436 bytes --]


On 11/22/19 7:25 AM, Dan Carpenter wrote:
> On Fri, Nov 22, 2019 at 06:58:51AM -0500, Nitesh Narayan Lal wrote:
>> For the build error, I didn't trigger it because I didn't compile with
>> appropriate flags.
> It's going to be a serveral years before we can enable that flag by
> default.

I see I have made a note of it so that I can do it before sending the patches.

> regards,
> dan carpenter
>
-- 
Thanks
Nitesh


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Paolo Bonzini]

On 19/11/19 13:14, Dan Carpenter wrote:
>> Better expressed as 
>>
>> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
>
> There is sort of a debate about this whether the Fixes tag should be
> used if it's only a cleanup.

The other debate is whether this is a cleanup, since the build is broken
with -Werror.  I agree that code cleanups generally don't deserve Fixes
tags, but this patch IMHO does.

Paolo

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[maowenan]

shall we send v2 with fixes tag?

在 2019/11/21 17:13, Paolo Bonzini 写道:
> atch IMHO does.

[PATCH -next v2] KVM: x86: remove set but not used variable 'called'

[Mao Wenan]

Fixes gcc '-Wunused-but-set-variable' warning:

arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
used [-Wunused-but-set-variable]

It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
IOAPIC scan request to target vCPUs")

Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v2: add fixes tag since Paolo and Vitaly proposal.
 arch/x86/kvm/x86.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0d0a682..870f0bc 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7908,12 +7908,11 @@ void kvm_make_scan_ioapic_request_mask(struct kvm *kvm,
 				       unsigned long *vcpu_bitmap)
 {
 	cpumask_var_t cpus;
-	bool called;
 
 	zalloc_cpumask_var(&cpus, GFP_ATOMIC);
 
-	called = kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
-					     vcpu_bitmap, cpus);
+	kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
+				    vcpu_bitmap, cpus);
 
 	free_cpumask_var(cpus);
 }
-- 
2.7.4

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[maowenan]

在 2019/11/19 19:58, Vitaly Kuznetsov 写道:
> Mao Wenan <maowenan@huawei.com> writes:
> 
>> Fixes gcc '-Wunused-but-set-variable' warning:
>>
>> arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
>> arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
>> used [-Wunused-but-set-variable]
>>
>> It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
>> IOAPIC scan request to target vCPUs")
> 
> Better expressed as 
> 
> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")

This is just a cleanup, so Fixes tag is no need.
> 
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  arch/x86/kvm/x86.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 0d0a682..870f0bc 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -7908,12 +7908,11 @@ void kvm_make_scan_ioapic_request_mask(struct kvm *kvm,
>>  				       unsigned long *vcpu_bitmap)
>>  {
>>  	cpumask_var_t cpus;
>> -	bool called;
>>  
>>  	zalloc_cpumask_var(&cpus, GFP_ATOMIC);
>>  
>> -	called = kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
>> -					     vcpu_bitmap, cpus);
>> +	kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
>> +				    vcpu_bitmap, cpus);
> 
> IMHO as kvm_make_vcpus_request_mask() returns value it would probably
> make sense to explicitly show that we're not interested in the result,
> 
> (void)kvm_make_vcpus_request_mask()

thanks, but I think is no need to add (void) before kvm_make_vcpus_request_mask()
because we are not interested in it's return value.

> 
>>  
>>  	free_cpumask_var(cpus);
>>  }
> 

Re: [PATCH -next] KVM: x86: remove set but not used variable 'called'

[Vitaly Kuznetsov]

maowenan <maowenan@huawei.com> writes:

> 在 2019/11/19 19:58, Vitaly Kuznetsov 写道:
>> Mao Wenan <maowenan@huawei.com> writes:
>> 
>>> Fixes gcc '-Wunused-but-set-variable' warning:
>>>
>>> arch/x86/kvm/x86.c: In function kvm_make_scan_ioapic_request_mask:
>>> arch/x86/kvm/x86.c:7911:7: warning: variable called set but not
>>> used [-Wunused-but-set-variable]
>>>
>>> It is not used since commit 7ee30bc132c6 ("KVM: x86: deliver KVM
>>> IOAPIC scan request to target vCPUs")
>> 
>> Better expressed as 
>> 
>> Fixes: 7ee30bc132c6 ("KVM: x86: deliver KVM IOAPIC scan request to target vCPUs")
>
> This is just a cleanup, so Fixes tag is no need.
>> 

Just a cleanup -- unless we compile with '-Werror'.

>>>
>>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>>> ---
>>>  arch/x86/kvm/x86.c | 5 ++---
>>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 0d0a682..870f0bc 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -7908,12 +7908,11 @@ void kvm_make_scan_ioapic_request_mask(struct kvm *kvm,
>>>  				       unsigned long *vcpu_bitmap)
>>>  {
>>>  	cpumask_var_t cpus;
>>> -	bool called;
>>>  
>>>  	zalloc_cpumask_var(&cpus, GFP_ATOMIC);
>>>  
>>> -	called = kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
>>> -					     vcpu_bitmap, cpus);
>>> +	kvm_make_vcpus_request_mask(kvm, KVM_REQ_SCAN_IOAPIC,
>>> +				    vcpu_bitmap, cpus);
>> 
>> IMHO as kvm_make_vcpus_request_mask() returns value it would probably
>> make sense to explicitly show that we're not interested in the result,
>> 
>> (void)kvm_make_vcpus_request_mask()
>
> thanks, but I think is no need to add (void) before kvm_make_vcpus_request_mask()
> because we are not interested in it's return value.

Hm, that's exactly the reason why I suggested adding it there :-) Not a
big deal, feel free to ignore.

-- 
Vitaly