Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1222507 > unrolled thread

[PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic Jason Wang <jasowang@redhat.com> - 2015-09-11 05:20 +0200
    Re: [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign  logic Cornelia Huck <cornelia.huck@de.ibm.com> - 2015-09-11 09:40 +0200
      Re: [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic Paolo Bonzini <pbonzini@redhat.com> - 2015-09-11 10:20 +0200
      Re: [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic Jason Wang <jasowang@redhat.com> - 2015-09-11 11:20 +0200

#1222507 — [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic

This patch factors out core eventfd assign/deassign logic and leave
the argument checking and bus index selection to callers.

Cc: Gleb Natapov <gleb@kernel.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 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)
 {
 	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)
+{
 	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)
 {
-- 
2.1.4

--
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/

[toc] | [next] | [standalone]

#1222581 — Re: [PATCH V4 1/4] kvm: factor out core eventfd assign/deassign logic

On Fri, 11 Sep 2015 11:17:34 +0800
Jason Wang <jasowang@redhat.com> wrote:

> This patch factors out core eventfd assign/deassign logic and leave
> the argument checking and bus index selection to callers.
> 
> Cc: Gleb Natapov <gleb@kernel.org>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  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)
>  {

--
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/

[toc] | [prev] | [next] | [standalone]

#1222611

On 11/09/2015 09:39, Cornelia Huck wrote:
> > +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.

I prefer it this way if it doesn't make the declaration one line longer,
which seems to be the case here.

Paolo
--
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/

[toc] | [prev] | [next] | [standalone]

#1222642

On 09/11/2015 03:39 PM, Cornelia Huck wrote:
> On Fri, 11 Sep 2015 11:17:34 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> This patch factors out core eventfd assign/deassign logic and leave
>> the argument checking and bus index selection to callers.
>>
>> Cc: Gleb Natapov <gleb@kernel.org>
>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  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?
>

Yes, this can reduce the changes for patch2.
--
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/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web