| Message ID | 1467215448-29216-1-git-send-email-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 29, 2016 at 05:50:48PM +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> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Thanks, > --- > V2: use native protocol in case of unknown protocol specified as > requested by Anthony Perard > --- > hw/block/xen_disk.c | 18 ++++++++++-------- > 1 file changed, 10 insertions(+), 8 deletions(-) > > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > index 90aca73..d0aae67 100644 > --- a/hw/block/xen_disk.c > +++ b/hw/block/xen_disk.c > @@ -975,14 +975,16 @@ 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; > + } else { > + blkdev->protocol = BLKIF_PROTOCOL_NATIVE; > } > > blkdev->sring = xengnttab_map_grant_ref(blkdev->xendev.gnttabdev, > -- > 2.6.6 >
On Wed, 29 Jun 2016, Anthony PERARD wrote: > On Wed, Jun 29, 2016 at 05:50:48PM +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> > > Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> Added to my queue > > --- > > V2: use native protocol in case of unknown protocol specified as > > requested by Anthony Perard > > --- > > hw/block/xen_disk.c | 18 ++++++++++-------- > > 1 file changed, 10 insertions(+), 8 deletions(-) > > > > diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c > > index 90aca73..d0aae67 100644 > > --- a/hw/block/xen_disk.c > > +++ b/hw/block/xen_disk.c > > @@ -975,14 +975,16 @@ 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; > > + } else { > > + blkdev->protocol = BLKIF_PROTOCOL_NATIVE; > > } > > > > blkdev->sring = xengnttab_map_grant_ref(blkdev->xendev.gnttabdev, > > -- > > 2.6.6 > > > > -- > Anthony PERARD >
On 05/07/16 16:55, Stefano Stabellini wrote: > On Wed, 29 Jun 2016, Anthony PERARD wrote: >> On Wed, Jun 29, 2016 at 05:50:48PM +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> >> >> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> > > Added to my queue Any plans to push it? Juergen
On Tue, 30 Aug 2016, Juergen Gross wrote: > On 05/07/16 16:55, Stefano Stabellini wrote: > > On Wed, 29 Jun 2016, Anthony PERARD wrote: > >> On Wed, Jun 29, 2016 at 05:50:48PM +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> > >> > >> Reviewed-by: Anthony PERARD <anthony.perard@citrix.com> > > > > Added to my queue > > Any plans to push it? Yes, I am sorry. I should have done it earlier. A QEMU release should be imminent (latest is v2.7.0-rc5), I'll send a pull request right after.
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c index 90aca73..d0aae67 100644 --- a/hw/block/xen_disk.c +++ b/hw/block/xen_disk.c @@ -975,14 +975,16 @@ 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; + } else { + blkdev->protocol = BLKIF_PROTOCOL_NATIVE; } 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> --- V2: use native protocol in case of unknown protocol specified as requested by Anthony Perard --- hw/block/xen_disk.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)