| Message ID | 20191119030640.25097-1-maowenan@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] KVM: x86: remove set but not used variable 'called' | expand |
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); > }
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
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 :-)
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
在 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); >> } >
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.
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.
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.
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. >
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 >
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); }
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(-)