From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752562AbcAVEA0 (ORCPT ); Thu, 21 Jan 2016 23:00:26 -0500 Received: from mail-pf0-f193.google.com ([209.85.192.193]:35392 "EHLO mail-pf0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752069AbcAVEAW (ORCPT ); Thu, 21 Jan 2016 23:00:22 -0500 Subject: Re: [PATCH v3 2/4] KVM: x86: Use vector-hashing to deliver lowest-priority interrupts To: "rkrcmar@redhat.com" , "Wu, Feng" 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> <20160121172109.GB17514@potion.brq.redhat.com> Cc: "pbonzini@redhat.com" , "linux-kernel@vger.kernel.org" , "kvm@vger.kernel.org" From: Yang Zhang Message-ID: <56A1A94C.7010703@gmail.com> Date: Fri, 22 Jan 2016 12:00:12 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160121172109.GB17514@potion.brq.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016/1/22 1:21, rkrcmar@redhat.com wrote: > 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.) Currently, if *r == -1, the remote_irr may get set. But it seems wrong. I need to have a double check to see whether it is a bug in current code. > >> 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, why it is undefined behavior? Besides, why we will keep two different handling logic for the fast path and slow path? It looks weird. > so it doesn't matter matter if we don't try. > -- best regards yang