xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xen: use native disk xenbus protocol if possible
@ 2016-06-17 11:14 Juergen Gross
  2016-06-27 10:54 ` [Xen-devel] " Juergen Gross
  2016-06-29 15:20 ` Anthony PERARD
  0 siblings, 2 replies; 4+ messages in thread
From: Juergen Gross @ 2016-06-17 11:14 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: anthony.perard, Juergen Gross, sstabellini, kraxel

The qdisk implementation is using the native xenbus protocol only in
case of no protocol specified at all. As using the explicit 32- or
64-bit protocol is slower than the native one due to copying requests
not by memcpy but element for element, this is not optimal.

Correct this by using the native protocol in case word sizes of
frontend and backend match.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 hw/block/xen_disk.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 2862b59..0961fcb 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -976,14 +976,14 @@ static int blk_connect(struct XenDevice *xendev)
         blkdev->feature_persistent = !!pers;
     }
 
-    blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
-    if (blkdev->xendev.protocol) {
-        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
-            blkdev->protocol = BLKIF_PROTOCOL_X86_32;
-        }
-        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
-            blkdev->protocol = BLKIF_PROTOCOL_X86_64;
-        }
+    if (!blkdev->xendev.protocol) {
+        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
+    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
+        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
+    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
+        blkdev->protocol = BLKIF_PROTOCOL_X86_32;
+    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
+        blkdev->protocol = BLKIF_PROTOCOL_X86_64;
     }
 
     blkdev->sring = xengnttab_map_grant_ref(blkdev->xendev.gnttabdev,
-- 
2.6.6

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [Xen-devel] [PATCH] xen: use native disk xenbus protocol if possible
  2016-06-17 11:14 [PATCH] xen: use native disk xenbus protocol if possible Juergen Gross
@ 2016-06-27 10:54 ` Juergen Gross
  2016-06-29 15:20 ` Anthony PERARD
  1 sibling, 0 replies; 4+ messages in thread
From: Juergen Gross @ 2016-06-27 10:54 UTC (permalink / raw)
  To: qemu-devel, xen-devel; +Cc: anthony.perard, sstabellini, kraxel

On 17/06/16 13:14, Juergen Gross wrote:
> The qdisk implementation is using the native xenbus protocol only in
> case of no protocol specified at all. As using the explicit 32- or
> 64-bit protocol is slower than the native one due to copying requests
> not by memcpy but element for element, this is not optimal.
> 
> Correct this by using the native protocol in case word sizes of
> frontend and backend match.

Ping?

> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  hw/block/xen_disk.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 2862b59..0961fcb 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -976,14 +976,14 @@ static int blk_connect(struct XenDevice *xendev)
>          blkdev->feature_persistent = !!pers;
>      }
>  
> -    blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
> -    if (blkdev->xendev.protocol) {
> -        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
> -            blkdev->protocol = BLKIF_PROTOCOL_X86_32;
> -        }
> -        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
> -            blkdev->protocol = BLKIF_PROTOCOL_X86_64;
> -        }
> +    if (!blkdev->xendev.protocol) {
> +        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
> +        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
> +        blkdev->protocol = BLKIF_PROTOCOL_X86_32;
> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
> +        blkdev->protocol = BLKIF_PROTOCOL_X86_64;
>      }
>  
>      blkdev->sring = xengnttab_map_grant_ref(blkdev->xendev.gnttabdev,
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] xen: use native disk xenbus protocol if possible
  2016-06-17 11:14 [PATCH] xen: use native disk xenbus protocol if possible Juergen Gross
  2016-06-27 10:54 ` [Xen-devel] " Juergen Gross
@ 2016-06-29 15:20 ` Anthony PERARD
  2016-06-29 15:38   ` Juergen Gross
  1 sibling, 1 reply; 4+ messages in thread
From: Anthony PERARD @ 2016-06-29 15:20 UTC (permalink / raw)
  To: Juergen Gross; +Cc: xen-devel, sstabellini, qemu-devel, kraxel

On Fri, Jun 17, 2016 at 01:14:56PM +0200, Juergen Gross wrote:
> The qdisk implementation is using the native xenbus protocol only in
> case of no protocol specified at all. As using the explicit 32- or
> 64-bit protocol is slower than the native one due to copying requests
> not by memcpy but element for element, this is not optimal.
> 
> Correct this by using the native protocol in case word sizes of
> frontend and backend match.
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
>  hw/block/xen_disk.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 2862b59..0961fcb 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -976,14 +976,14 @@ static int blk_connect(struct XenDevice *xendev)
>          blkdev->feature_persistent = !!pers;
>      }
>  
> -    blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
> -    if (blkdev->xendev.protocol) {
> -        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
> -            blkdev->protocol = BLKIF_PROTOCOL_X86_32;
> -        }
> -        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
> -            blkdev->protocol = BLKIF_PROTOCOL_X86_64;
> -        }
> +    if (!blkdev->xendev.protocol) {
> +        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
> +        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
> +        blkdev->protocol = BLKIF_PROTOCOL_X86_32;
> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
> +        blkdev->protocol = BLKIF_PROTOCOL_X86_64;

There is one difference with the previous code, in case the protocol is
specified, but it not x86_32 or x86_64, then no blkdev->protocol is
selected (then no ring is initialized). Could you re-add this case?

-- 
Anthony PERARD

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] xen: use native disk xenbus protocol if possible
  2016-06-29 15:20 ` Anthony PERARD
@ 2016-06-29 15:38   ` Juergen Gross
  0 siblings, 0 replies; 4+ messages in thread
From: Juergen Gross @ 2016-06-29 15:38 UTC (permalink / raw)
  To: Anthony PERARD; +Cc: xen-devel, sstabellini, qemu-devel, kraxel

On 29/06/16 17:20, Anthony PERARD wrote:
> On Fri, Jun 17, 2016 at 01:14:56PM +0200, Juergen Gross wrote:
>> The qdisk implementation is using the native xenbus protocol only in
>> case of no protocol specified at all. As using the explicit 32- or
>> 64-bit protocol is slower than the native one due to copying requests
>> not by memcpy but element for element, this is not optimal.
>>
>> Correct this by using the native protocol in case word sizes of
>> frontend and backend match.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>>  hw/block/xen_disk.c | 16 ++++++++--------
>>  1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
>> index 2862b59..0961fcb 100644
>> --- a/hw/block/xen_disk.c
>> +++ b/hw/block/xen_disk.c
>> @@ -976,14 +976,14 @@ static int blk_connect(struct XenDevice *xendev)
>>          blkdev->feature_persistent = !!pers;
>>      }
>>  
>> -    blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
>> -    if (blkdev->xendev.protocol) {
>> -        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
>> -            blkdev->protocol = BLKIF_PROTOCOL_X86_32;
>> -        }
>> -        if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
>> -            blkdev->protocol = BLKIF_PROTOCOL_X86_64;
>> -        }
>> +    if (!blkdev->xendev.protocol) {
>> +        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
>> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_NATIVE) == 0) {
>> +        blkdev->protocol = BLKIF_PROTOCOL_NATIVE;
>> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_32) == 0) {
>> +        blkdev->protocol = BLKIF_PROTOCOL_X86_32;
>> +    } else if (strcmp(blkdev->xendev.protocol, XEN_IO_PROTO_ABI_X86_64) == 0) {
>> +        blkdev->protocol = BLKIF_PROTOCOL_X86_64;
> 
> There is one difference with the previous code, in case the protocol is
> specified, but it not x86_32 or x86_64, then no blkdev->protocol is
> selected (then no ring is initialized). Could you re-add this case?

Aah, of course!

Thanks,

Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-29 15:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17 11:14 [PATCH] xen: use native disk xenbus protocol if possible Juergen Gross
2016-06-27 10:54 ` [Xen-devel] " Juergen Gross
2016-06-29 15:20 ` Anthony PERARD
2016-06-29 15:38   ` Juergen Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).