From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965598AbcAURVO (ORCPT ); Thu, 21 Jan 2016 12:21:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45008 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759706AbcAURVN (ORCPT ); Thu, 21 Jan 2016 12:21:13 -0500 Date: Thu, 21 Jan 2016 18:21:10 +0100 From: "rkrcmar@redhat.com" To: "Wu, Feng" Cc: Yang Zhang , "pbonzini@redhat.com" , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" Subject: Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver lowest-priority interrupts Message-ID: <20160121172109.GB17514@potion.brq.redhat.com> References: <1453254177-103002-1-git-send-email-feng.wu@intel.com> <1453254177-103002-3-git-send-email-feng.wu@intel.com> <56A06B60.5030501@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2016-01-21 05:33+0000, Wu, Feng: >> From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel- >> owner@vger.kernel.org] On Behalf Of Yang Zhang >> On 2016/1/20 9:42, Feng Wu wrote: >> > + /* >> > + * We may find a hardware disabled LAPIC here, if >> that >> > + * is the case, print out a error message once for each >> > + * guest and return. >> > + */ >> > + if (!dst[idx-1] && >> > + (kvm->arch.disabled_lapic_found == 0)) { >> > + kvm->arch.disabled_lapic_found = 1; >> > + printk(KERN_ERR >> > + "Disabled LAPIC found during irq >> injection\n"); >> > + goto out; >> >> What does "goto out" mean? Inject successfully or fail? According the >> value of ret which is set to ture here, it means inject successfully but (true actually means that fast path did the job and slow path isn't needed.) >> i = -1. (I think there isn't a practical difference between *r=-1 and *r=0.) > Oh, I didn't notice 'ret' is initialized to true, I thought it was initialized > to false like another function, I should add a "ret = false' here. We should > failed to inject the interrupt since hardware disabled LAPIC is found. 'ret = true' is the better one. We know that the interrupt is not deliverable [1], so there's no point in trying to deliver with the slow path. We behave similarly when the interrupt targets a single disabled APIC. --- 1: Well ... it's possible that slowpath would deliver it thanks to different handling of disabled APICs, but it's undefined behavior, so it doesn't matter matter if we don't try.