| Message ID | 1466162096-4934-1-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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, >
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?
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
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,
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(-)