From patchwork Mon Sep 22 18:35:42 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Matlack X-Patchwork-Id: 4949751 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id EA74F9F2BB for ; Mon, 22 Sep 2014 18:36:03 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2D466201D3 for ; Mon, 22 Sep 2014 18:36:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3FE3220221 for ; Mon, 22 Sep 2014 18:36:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754836AbaIVSfs (ORCPT ); Mon, 22 Sep 2014 14:35:48 -0400 Received: from mail-ig0-f173.google.com ([209.85.213.173]:49336 "EHLO mail-ig0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754829AbaIVSfp (ORCPT ); Mon, 22 Sep 2014 14:35:45 -0400 Received: by mail-ig0-f173.google.com with SMTP id l13so3259984iga.0 for ; Mon, 22 Sep 2014 11:35:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=/UdZy7O6qyGnSXv5M2K/KHe+TxyrY7SEnk9FbJjRjc0=; b=X08gr9LvqB5ot/dBdfTxkAXF/udapHT5VQvJuwc6/jWrvDYtgX47cgicPdVo8M3JKd vCLYkNMPS+cFeYWYIg8GNEyP9tEWVbIO4vqMt9JsdZY2Y0cJo3jJOMPs9nw/jONwyldv BG7HJNvkGVaCOzuhFmpdm8xanSiTsdcgNWh5n/iP8EmGMTLJRYvVvZwLe8whE9yVgANZ Lx0ujn2S2kkBPmiCZhiy+m8NpYecT5jZQtNBZW184malIuAh6xaJb7hCIetAdqmofsiN tv5EHrHwbgDko4YTzM9pL2SyvaqahWj5pZ6h+6KY0vRMxqagMutdMBdxIzJyIfuGaYvY BuSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=/UdZy7O6qyGnSXv5M2K/KHe+TxyrY7SEnk9FbJjRjc0=; b=Xe6y6U77qPog5gxD9wzT/SdlqiJMUgsQNiTqG5KCiTiuobzL5S0zHPfPuwAqcVZCcx NJSkOCrQGrmtXk8EuKSrKZIHEWa2Y99L0VCMLKCHf7PdE/z1zfH+3wEIY55/WR3h9b0e 1xzY37Juoz5CTnG4GLmpsCgkEjyza7xyNXAtYWjsEHa9ieV7SRDuZ1/bW8+d7TOzDLUS Mh8wECw5rDUZ/a55kytGm72l2GGOdLo+hwqqrGlo8wWJOA5HucmDjaV2uhusSDe5RQDU mp23V7PcSvjTbK8X6Wtj6GoQH24yv3W7YodnGJu5wG6YAdPLg+/pwlHxypp5FipO94+t 4DKg== X-Gm-Message-State: ALoCoQkcQlyuO0eyDYxhxwqOB/7AVuk8ElgdidJEVV4Y/nSJYnO7DJu0N/bMofzQPlPshuy9XFUU X-Received: by 10.50.22.101 with SMTP id c5mr16156821igf.29.1411410944985; Mon, 22 Sep 2014 11:35:44 -0700 (PDT) Received: from google.com ([2620:0:1009:3:5d51:1bb1:12e4:2e76]) by mx.google.com with ESMTPSA id au4sm9403430igc.3.2014.09.22.11.35.43 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Mon, 22 Sep 2014 11:35:44 -0700 (PDT) Date: Mon, 22 Sep 2014 11:35:42 -0700 From: David Matlack To: Paolo Bonzini Cc: Christian Borntraeger , Gleb Natapov , kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] kvm: don't take vcpu mutex for obviously invalid vcpu ioctls Message-ID: <20140922183542.GA1018@google.com> References: <1411167805-2458-1-git-send-email-dmatlack@google.com> <541FFEDE.9030800@redhat.com> <542027F6.4050205@de.ibm.com> <542032D4.305@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <542032D4.305@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-7.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On 09/22, Paolo Bonzini wrote: > Il 22/09/2014 15:45, Christian Borntraeger ha scritto: > > We now have an extra condition check for every valid ioctl, to make an error case go faster. > > I know, the extra check is just a 1 or 2 cycles if branch prediction is right, but still. > > I applied the patch because the delay could be substantial, depending on > what the other VCPU is doing. Perhaps something like this would be > better? I'm happy with either approach. > > (Untested, but Tested-by/Reviewed-bys are welcome). There were a few build bugs in your diff. Here's a working version that I tested. Feel free to add my Tested-by and Reviewed-by if you go with this. --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index c71931f..fbdcdc2 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -133,12 +133,10 @@ bool kvm_is_mmio_pfn(pfn_t pfn) /* * Switches to specified vcpu, until a matching vcpu_put() */ -int vcpu_load(struct kvm_vcpu *vcpu) +static void __vcpu_load(struct kvm_vcpu *vcpu) { int cpu; - if (mutex_lock_killable(&vcpu->mutex)) - return -EINTR; if (unlikely(vcpu->pid != current->pids[PIDTYPE_PID].pid)) { /* The thread running this VCPU changed. */ struct pid *oldpid = vcpu->pid; @@ -151,6 +149,14 @@ int vcpu_load(struct kvm_vcpu *vcpu) preempt_notifier_register(&vcpu->preempt_notifier); kvm_arch_vcpu_load(vcpu, cpu); put_cpu(); +} + +int vcpu_load(struct kvm_vcpu *vcpu) +{ + if (mutex_lock_killable(&vcpu->mutex)) + return -EINTR; + + __vcpu_load(vcpu); return 0; } @@ -2197,10 +2203,21 @@ static long kvm_vcpu_ioctl(struct file *filp, return kvm_arch_vcpu_ioctl(filp, ioctl, arg); #endif + if (!mutex_trylock(&vcpu->mutex)) { + /* + * Before a potentially long sleep, check if we'd exit anyway. + * The common case is for the mutex not to be contended, when + * this does not add overhead. + */ + if (unlikely(_IOC_TYPE(ioctl) != KVMIO)) + return -EINVAL; + + if (mutex_lock_killable(&vcpu->mutex)) + return -EINTR; + } + + __vcpu_load(vcpu); - r = vcpu_load(vcpu); - if (r) - return r; switch (ioctl) { case KVM_RUN: r = -EINVAL;