From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752593AbdGEMYZ (ORCPT ); Wed, 5 Jul 2017 08:24:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49956 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751727AbdGEMYY (ORCPT ); Wed, 5 Jul 2017 08:24:24 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7DB4518DF47 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=pbonzini@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 7DB4518DF47 Subject: Re: [PATCH] kvm: avoid unused variable warning for UP builds To: David Hildenbrand , linux-kernel@vger.kernel.org, kvm@vger.kernel.org Cc: paulus@ozlabs.org, stable@vger.kernel.org References: <1499250930-34010-1-git-send-email-pbonzini@redhat.com> From: Paolo Bonzini Message-ID: <7f7ee5d9-22b6-383f-e7c8-d39ced64314a@redhat.com> Date: Wed, 5 Jul 2017 14:24:16 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Wed, 05 Jul 2017 12:24:23 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. > >