From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751976AbbIKHjj (ORCPT ); Fri, 11 Sep 2015 03:39:39 -0400 Received: from e06smtp09.uk.ibm.com ([195.75.94.105]:41728 "EHLO e06smtp09.uk.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751270AbbIKHjh (ORCPT ); Fri, 11 Sep 2015 03:39:37 -0400 X-Helo: d06dlp02.portsmouth.uk.ibm.com X-MailFrom: cornelia.huck@de.ibm.com X-RcptTo: linux-kernel@vger.kernel.org Date: Fri, 11 Sep 2015 09:39:28 +0200 From: Cornelia Huck To: Jason Wang Cc: gleb@kernel.org, pbonzini@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, mst@redhat.com Subject: Re: [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic Message-ID: <20150911093928.5cb7173c.cornelia.huck@de.ibm.com> In-Reply-To: <1441941457-23630-2-git-send-email-jasowang@redhat.com> References: <1441941457-23630-1-git-send-email-jasowang@redhat.com> <1441941457-23630-2-git-send-email-jasowang@redhat.com> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.8.0 (GTK+ 2.24.10; i686-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15091107-0037-0000-0000-000003DEA5D2 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 11 Sep 2015 11:17:34 +0800 Jason Wang wrote: > This patch factors out core eventfd assign/deassign logic and leave > the argument checking and bus index selection to callers. > > Cc: Gleb Natapov > Cc: Paolo Bonzini > Signed-off-by: Jason Wang > --- > virt/kvm/eventfd.c | 83 ++++++++++++++++++++++++++++++++---------------------- > 1 file changed, 49 insertions(+), 34 deletions(-) > > diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c > index 9ff4193..163258d 100644 > --- a/virt/kvm/eventfd.c > +++ b/virt/kvm/eventfd.c > @@ -771,40 +771,14 @@ static enum kvm_bus ioeventfd_bus_from_flags(__u32 flags) > return KVM_MMIO_BUS; > } > > -static int > -kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > +static int kvm_assign_ioeventfd_idx(struct kvm *kvm, > + enum kvm_bus bus_idx, > + struct kvm_ioeventfd *args) > { > - enum kvm_bus bus_idx; > - struct _ioeventfd *p; > - struct eventfd_ctx *eventfd; > - int ret; > - > - bus_idx = ioeventfd_bus_from_flags(args->flags); > - /* must be natural-word sized, or 0 to ignore length */ > - switch (args->len) { > - case 0: > - case 1: > - case 2: > - case 4: > - case 8: > - break; > - default: > - return -EINVAL; > - } > > - /* check for range overflow */ > - if (args->addr + args->len < args->addr) > - return -EINVAL; > - > - /* check for extra flags that we don't understand */ > - if (args->flags & ~KVM_IOEVENTFD_VALID_FLAG_MASK) > - return -EINVAL; > - > - /* ioeventfd with no length can't be combined with DATAMATCH */ > - if (!args->len && > - args->flags & (KVM_IOEVENTFD_FLAG_PIO | > - KVM_IOEVENTFD_FLAG_DATAMATCH)) > - return -EINVAL; > + struct eventfd_ctx *eventfd; > + struct _ioeventfd *p; > + int ret; > > eventfd = eventfd_ctx_fdget(args->fd); > if (IS_ERR(eventfd)) > @@ -873,14 +847,48 @@ fail: > } > > static int > -kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > +kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) You'll move this function to below the deassign function in patch 2. Maybe do it already here? > { > enum kvm_bus bus_idx; > + > + bus_idx = ioeventfd_bus_from_flags(args->flags); > + /* must be natural-word sized, or 0 to ignore length */ > + switch (args->len) { > + case 0: > + case 1: > + case 2: > + case 4: > + case 8: > + break; > + default: > + return -EINVAL; > + } > + > + /* check for range overflow */ > + if (args->addr + args->len < args->addr) > + return -EINVAL; > + > + /* check for extra flags that we don't understand */ > + if (args->flags & ~KVM_IOEVENTFD_VALID_FLAG_MASK) > + return -EINVAL; > + > + /* ioeventfd with no length can't be combined with DATAMATCH */ > + if (!args->len && > + args->flags & (KVM_IOEVENTFD_FLAG_PIO | > + KVM_IOEVENTFD_FLAG_DATAMATCH)) > + return -EINVAL; > + > + return kvm_assign_ioeventfd_idx(kvm, bus_idx, args); > +} > + > +static int > +kvm_deassign_ioeventfd_idx(struct kvm *kvm, enum kvm_bus bus_idx, > + struct kvm_ioeventfd *args) While this file uses newline before function name quite often, putting it on the same line seems more common - don't know which one the maintainers prefer. > +{ > struct _ioeventfd *p, *tmp; > struct eventfd_ctx *eventfd; > int ret = -ENOENT; > > - bus_idx = ioeventfd_bus_from_flags(args->flags); > eventfd = eventfd_ctx_fdget(args->fd); > if (IS_ERR(eventfd)) > return PTR_ERR(eventfd); > @@ -918,6 +926,13 @@ kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > return ret; > } > > +static int kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > +{ > + enum kvm_bus bus_idx = ioeventfd_bus_from_flags(args->flags); > + > + return kvm_deassign_ioeventfd_idx(kvm, bus_idx, args); > +} > + > int > kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > {