mbox series

[0/1,resend] Add vhost-pci-blk driver

Message ID 20181105140327.8363-1-v.mayatskih@gmail.com (mailing list archive)
Headers show
Series Add vhost-pci-blk driver | expand

Message

Vitaly Mayatskih Nov. 5, 2018, 2:03 p.m. UTC
This driver moves virtio-blk host-side processing to kernel (via new
vhost_blk kernel driver). It accelerates virtual disk performance
close to bare metal levels, especially for parellel loads.

For example, fio numjobs=16 gets 101k randread IOPS using virtio-blk
and 1202k IOPS using vhost-blk, close to 1480k of raw disk performance.

See the IOPS numbers below.

The kernel part if you want to try:
- vhost_blk: https://lkml.org/lkml/2018/11/2/648
- vhost num-queues scalability fix: https://lkml.org/lkml/2018/11/2/550

# fio num-jobs
# A: bare metal over block
# B: bare metal over file
# C: virtio-blk over block
# D: virtio-blk over file
# E: vhost-blk over block
# F: vhost-blk over file
#
#  A     B     C    D    E     F

1  171k  151k  148k 151k 187k  175k
2  328k  302k  249k 241k 334k  296k
3  479k  437k  179k 174k 464k  404k
4  622k  568k  143k 183k 580k  492k
5  755k  697k  136k 128k 693k  579k
6  887k  808k  131k 120k 782k  640k
7  1004k 926k  126k 131k 863k  693k
8  1099k 1015k 117k 115k 931k  712k
9  1194k 1119k 115k 111k 991k  711k
10 1278k 1207k 109k 114k 1046k 695k
11 1345k 1280k 110k 108k 1091k 663k
12 1411k 1356k 104k 106k 1142k 629k
13 1466k 1423k 106k 106k 1170k 607k
14 1517k 1486k 103k 106k 1179k 589k
15 1552k 1543k 102k 102k 1191k 571k
16 1480k 1506k 101k 102k 1202k 566k

Vitaly Mayatskikh (1):
  Add vhost-pci-blk driver

 configure                  | 10 +++++++
 default-configs/virtio.mak |  1 +
 hw/block/Makefile.objs     |  1 +
 hw/virtio/virtio-pci.c     | 60 ++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio-pci.h     | 19 ++++++++++++
 5 files changed, 91 insertions(+)

Comments

Michael S. Tsirkin Nov. 5, 2018, 5:45 p.m. UTC | #1
On Mon, Nov 05, 2018 at 02:03:26PM +0000, Vitaly Mayatskikh wrote:
> This driver moves virtio-blk host-side processing to kernel (via new
> vhost_blk kernel driver). It accelerates virtual disk performance
> close to bare metal levels, especially for parellel loads.
> 
> For example, fio numjobs=16 gets 101k randread IOPS using virtio-blk
> and 1202k IOPS using vhost-blk, close to 1480k of raw disk performance.
> 
> See the IOPS numbers below.
> 
> The kernel part if you want to try:
> - vhost_blk: https://lkml.org/lkml/2018/11/2/648
> - vhost num-queues scalability fix: https://lkml.org/lkml/2018/11/2/550
> 
> # fio num-jobs
> # A: bare metal over block
> # B: bare metal over file
> # C: virtio-blk over block
> # D: virtio-blk over file
> # E: vhost-blk over block
> # F: vhost-blk over file
> #
> #  A     B     C    D    E     F
> 
> 1  171k  151k  148k 151k 187k  175k
> 2  328k  302k  249k 241k 334k  296k
> 3  479k  437k  179k 174k 464k  404k
> 4  622k  568k  143k 183k 580k  492k
> 5  755k  697k  136k 128k 693k  579k
> 6  887k  808k  131k 120k 782k  640k
> 7  1004k 926k  126k 131k 863k  693k
> 8  1099k 1015k 117k 115k 931k  712k
> 9  1194k 1119k 115k 111k 991k  711k
> 10 1278k 1207k 109k 114k 1046k 695k
> 11 1345k 1280k 110k 108k 1091k 663k
> 12 1411k 1356k 104k 106k 1142k 629k
> 13 1466k 1423k 106k 106k 1170k 607k
> 14 1517k 1486k 103k 106k 1179k 589k
> 15 1552k 1543k 102k 102k 1191k 571k
> 16 1480k 1506k 101k 102k 1202k 566k
> 
> Vitaly Mayatskikh (1):
>   Add vhost-pci-blk driver
> 
>  configure                  | 10 +++++++
>  default-configs/virtio.mak |  1 +
>  hw/block/Makefile.objs     |  1 +
>  hw/virtio/virtio-pci.c     | 60 ++++++++++++++++++++++++++++++++++++++
>  hw/virtio/virtio-pci.h     | 19 ++++++++++++
>  5 files changed, 91 insertions(+)

I think you should Cc more widely to get meaningful
review. At least virtio-blk and block layer core people.

> -- 
> 2.17.1
no-reply@patchew.org Nov. 5, 2018, 6:23 p.m. UTC | #2
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20181105140327.8363-1-v.mayatskih@gmail.com
Subject: [Qemu-devel] [PATCH 0/1 resend] Add vhost-pci-blk driver

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
19803cc4ae Add vhost-pci-blk driver

=== OUTPUT BEGIN ===
Checking PATCH 1/1: Add vhost-pci-blk driver...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#82: 
new file mode 100644

ERROR: Error messages should not contain newlines
#229: FILE: hw/block/vhost-blk.c:143:
+        error_report("Error opening backing store: %d\n", -errno);

ERROR: braces {} are necessary for all arms of this statement
#241: FILE: hw/block/vhost-blk.c:155:
+    if (s->bs_fd > 0)
[...]

ERROR: space prohibited after that '-' (ctx:WxW)
#327: FILE: hw/block/vhost-blk.c:241:
+    int fd = - 1;
              ^

WARNING: line over 80 characters
#331: FILE: hw/block/vhost-blk.c:245:
+	    error_report("Can't open device %s: %d\n", blk_bs(s->blk)->filename, errno);

ERROR: code indent should never use tabs
#331: FILE: hw/block/vhost-blk.c:245:
+^I    error_report("Can't open device %s: %d\n", blk_bs(s->blk)->filename, errno);$

ERROR: Error messages should not contain newlines
#331: FILE: hw/block/vhost-blk.c:245:
+	    error_report("Can't open device %s: %d\n", blk_bs(s->blk)->filename, errno);

ERROR: code indent should never use tabs
#332: FILE: hw/block/vhost-blk.c:246:
+^I    goto out;$

ERROR: code indent should never use tabs
#336: FILE: hw/block/vhost-blk.c:250:
+^I    ret = ioctl(fd, BLKGETSIZE, &var);$

ERROR: code indent should never use tabs
#337: FILE: hw/block/vhost-blk.c:251:
+^I    var64 = var;$

ERROR: code indent should never use tabs
#340: FILE: hw/block/vhost-blk.c:254:
+^I    error_report("Can't get drive size: %d\n", errno);$

ERROR: Error messages should not contain newlines
#340: FILE: hw/block/vhost-blk.c:254:
+	    error_report("Can't get drive size: %d\n", errno);

ERROR: code indent should never use tabs
#341: FILE: hw/block/vhost-blk.c:255:
+^I    goto out;$

ERROR: line over 90 characters
#347: FILE: hw/block/vhost-blk.c:261:
+	    error_report("Can't get drive logical sector size, assuming 512: %d\n", errno);

ERROR: code indent should never use tabs
#347: FILE: hw/block/vhost-blk.c:261:
+^I    error_report("Can't get drive logical sector size, assuming 512: %d\n", errno);$

ERROR: Error messages should not contain newlines
#347: FILE: hw/block/vhost-blk.c:261:
+	    error_report("Can't get drive logical sector size, assuming 512: %d\n", errno);

ERROR: code indent should never use tabs
#348: FILE: hw/block/vhost-blk.c:262:
+^I    var = 512;$

ERROR: braces {} are necessary for all arms of this statement
#360: FILE: hw/block/vhost-blk.c:274:
+    if (fd > 0)
[...]

ERROR: code indent should never use tabs
#361: FILE: hw/block/vhost-blk.c:275:
+^I    close(fd);$

ERROR: code indent should never use tabs
#410: FILE: hw/block/vhost-blk.c:324:
+^Igoto virtio_err;$

ERROR: line over 90 characters
#413: FILE: hw/block/vhost-blk.c:327:
+    ret = vhost_dev_init(&s->dev, (void *)(uintptr_t)s->vhostfd, VHOST_BACKEND_TYPE_KERNEL, 0);

total: 19 errors, 2 warnings, 613 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Vitaly Mayatskih Nov. 5, 2018, 8:08 p.m. UTC | #3
On Mon, Nov 5, 2018 at 12:45 PM Michael S. Tsirkin <mst@redhat.com> wrote:

> I think you should Cc more widely to get meaningful
> review. At least virtio-blk and block layer core people.

Thanks, it turns out I missed the existence of qemu/scripts directory
completely.