On 05/07/2017 14:22, David Hildenbrand wrote: >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index f0fe9d02f6bb..09368501d9cf 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -187,12 +187,23 @@ static void ack_flush(void *_completed) >> { >> } >> >> +static inline bool kvm_kick_many_cpus(const struct cpumask *cpus, bool wait) >> +{ >> + if (unlikely(!cpus)) >> + cpus = cpu_online_mask; >> + >> + if (cpumask_empty(cpus)) >> + return false; >> + >> + smp_call_function_many(cpus, ack_flush, NULL, wait); >> + return true; >> +} > > wonder if the !cpus case would be worth moving into smp_call_function_many. > > smp_call_function_many() might also not kick any cpu, so we could make > it return if it actually kicked/called this on any cpu. Then you could > even get rid of the special handling of cpumask_empty(cpus) here and > simply return the result of smp_call_function_many. Separate patch of course. :) >> + >> bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req) >> { >> int i, cpu, me; >> cpumask_var_t cpus; >> - bool called = true; >> - bool wait = req & KVM_REQUEST_WAIT; >> + bool called; >> struct kvm_vcpu *vcpu; >> >> zalloc_cpumask_var(&cpus, GFP_ATOMIC); >> @@ -207,14 +218,9 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req) >> >> if (cpus != NULL && cpu != -1 && cpu != me && >> kvm_request_needs_ipi(vcpu, req)) >> - cpumask_set_cpu(cpu, cpus); >> + __cpumask_set_cpu(cpu, cpus); >> } >> - if (unlikely(cpus == NULL)) >> - smp_call_function_many(cpu_online_mask, ack_flush, NULL, wait); >> - else if (!cpumask_empty(cpus)) >> - smp_call_function_many(cpus, ack_flush, NULL, wait); >> - else >> - called = false; >> + called = kvm_kick_many_cpus(cpus, !!(req & KVM_REQUEST_WAIT)); > > Is the !! really needed here? I think not. I prefer having it. There are corner cases (e.g. isolating bit 32 or higher and the function accepting an unsigned int instead of a bool) where it can save your butt, and it's idiomatic C. Paolo >> put_cpu(); >> free_cpumask_var(cpus); >> return called; >> > > I like this from a cleanup point as well. > >