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 <[email protected]>
---
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
Mao Wenan <[email protected]> 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 <[email protected]>
> ---
> 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
On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
> Mao Wenan <[email protected]> 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
Dan Carpenter <[email protected]> writes:
> On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
>> Mao Wenan <[email protected]> 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
On Tue, Nov 19, 2019 at 01:28:32PM +0100, Vitaly Kuznetsov wrote:
> Dan Carpenter <[email protected]> writes:
>
> > On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
> >> Mao Wenan <[email protected]> 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
在 2019/11/19 19:58, Vitaly Kuznetsov 写道:
> Mao Wenan <[email protected]> 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 <[email protected]>
>> ---
>> 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);
>> }
>
Dan Carpenter <[email protected]> writes:
> On Tue, Nov 19, 2019 at 01:28:32PM +0100, Vitaly Kuznetsov wrote:
>> Dan Carpenter <[email protected]> writes:
>>
>> > On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
>> >> Mao Wenan <[email protected]> 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
maowenan <[email protected]> writes:
> 在 2019/11/19 19:58, Vitaly Kuznetsov 写道:
>> Mao Wenan <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
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
shall we send v2 with fixes tag?
在 2019/11/21 17:13, Paolo Bonzini 写道:
> atch IMHO does.
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 <[email protected]>
---
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
On 11/19/19 8:25 AM, Vitaly Kuznetsov wrote:
> Dan Carpenter <[email protected]> writes:
>
>> On Tue, Nov 19, 2019 at 01:28:32PM +0100, Vitaly Kuznetsov wrote:
>>> Dan Carpenter <[email protected]> writes:
>>>
>>>> On Tue, Nov 19, 2019 at 12:58:54PM +0100, Vitaly Kuznetsov wrote:
>>>>> Mao Wenan <[email protected]> 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
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
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