Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1946198AbbHGUJT (ORCPT ); Fri, 7 Aug 2015 16:09:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51338 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751373AbbHGUJR (ORCPT ); Fri, 7 Aug 2015 16:09:17 -0400 Message-ID: <1438978155.4023.263.camel@redhat.com> Subject: Re: [PATCH v4 5/5] KVM: eventfd: add irq bypass consumer management From: Alex Williamson To: Eric Auger Cc: eric.auger@st.com, feng.wu@intel.com, pbonzini@redhat.com, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, avi.kivity@gmail.com, mtosatti@redhat.com, joro@8bytes.org, marc.zyngier@arm.com, christoffer.dall@linaro.org, linux-kernel@vger.kernel.org, patches@linaro.org Date: Fri, 07 Aug 2015 14:09:15 -0600 In-Reply-To: <1438622444-25444-6-git-send-email-eric.auger@linaro.org> References: <1438622444-25444-1-git-send-email-eric.auger@linaro.org> <1438622444-25444-6-git-send-email-eric.auger@linaro.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2964 Lines: 90 On Mon, 2015-08-03 at 19:20 +0200, Eric Auger wrote: > This patch adds the registration/unregistration of an > irq_bypass_consumer on irqfd assignment/deassignment. > > Signed-off-by: Eric Auger > Signed-off-by: Feng Wu > > --- > > v2 -> v3 (Feng Wu): > - Use kvm_arch_irq_bypass_start > - Remove kvm_arch_irq_bypass_update > - Add member 'struct irq_bypass_producer *producer' in > 'struct kvm_kernel_irqfd', it is needed by posted interrupt. > - Remove 'irq_bypass_unregister_consumer' in kvm_irqfd_deassign() > > v1 -> v2: > - populate of kvm and gsi removed > - unregister the consumer on irqfd_shutdown > --- > include/linux/kvm_irqfd.h | 2 ++ > virt/kvm/eventfd.c | 10 ++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/include/linux/kvm_irqfd.h b/include/linux/kvm_irqfd.h > index f926b39..0c1de05 100644 > --- a/include/linux/kvm_irqfd.h > +++ b/include/linux/kvm_irqfd.h > @@ -64,6 +64,8 @@ struct kvm_kernel_irqfd { > struct list_head list; > poll_table pt; > struct work_struct shutdown; > + struct irq_bypass_consumer consumer; > + struct irq_bypass_producer *producer; > }; > > #endif /* __LINUX_KVM_IRQFD_H */ > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c > index 647ffb8..08855de 100644 > --- a/virt/kvm/eventfd.c > +++ b/virt/kvm/eventfd.c > @@ -35,6 +35,7 @@ > #include > #include > #include > +#include > #include > > #include > @@ -140,6 +141,7 @@ irqfd_shutdown(struct work_struct *work) > /* > * It is now safe to release the object's resources > */ > + irq_bypass_unregister_consumer(&irqfd->consumer); > eventfd_ctx_put(irqfd->eventfd); > kfree(irqfd); > } > @@ -380,6 +382,14 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args) > */ > fdput(f); > > + irqfd->consumer.token = (void *)irqfd->eventfd; > + irqfd->consumer.add_producer = kvm_arch_irq_bypass_add_producer; > + irqfd->consumer.del_producer = kvm_arch_irq_bypass_del_producer; > + irqfd->consumer.stop = kvm_arch_irq_bypass_stop; > + irqfd->consumer.start = kvm_arch_irq_bypass_start; > + ret = irq_bypass_register_consumer(&irqfd->consumer); > + WARN_ON(ret); This seems like a lazy way to handle this error. What is the stack trace from this WARN_ON going to tell us that we didn't already know? If we get the WARN_ON, it's probably means that an incompatible producer registered the token first. It means we can't do bypass, but it doesn't tell us anything about whether bypass is or is not enabled. Wouldn't a pr_info/debug() suffice for that? Thanks, Alex > + > return 0; > > fail: -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/