From mboxrd@z Thu Jan 1 00:00:00 1970 From: Asias He Subject: Re: [PATCH 08/11] kvm tools: Split io request from completion Date: Mon, 31 Oct 2011 18:42:50 +0800 Message-ID: <4EAE7BAA.5070407@gmail.com> References: <1319996135-17501-1-git-send-email-levinsasha928@gmail.com> <1319996135-17501-9-git-send-email-levinsasha928@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: penberg@cs.helsinki.fi, kvm@vger.kernel.org, mingo@elte.hu, gorcunov@gmail.com To: Sasha Levin Return-path: Received: from mail-iy0-f174.google.com ([209.85.210.174]:55056 "EHLO mail-iy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932676Ab1JaKno (ORCPT ); Mon, 31 Oct 2011 06:43:44 -0400 Received: by iaby12 with SMTP id y12so7046347iab.19 for ; Mon, 31 Oct 2011 03:43:44 -0700 (PDT) In-Reply-To: <1319996135-17501-9-git-send-email-levinsasha928@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 10/31/2011 01:35 AM, Sasha Levin wrote: > This patch splits IO request processing from completion notification. >=20 > Signed-off-by: Sasha Levin > --- > tools/kvm/disk/core.c | 6 +- > tools/kvm/include/kvm/disk-image.h | 4 +- > tools/kvm/include/kvm/virtio-blk.h | 1 + > tools/kvm/virtio/blk.c | 109 ++++++++++++++++++++++++--= ---------- > 4 files changed, 79 insertions(+), 41 deletions(-) >=20 > diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c > index 842fef4..557a1a2 100644 > --- a/tools/kvm/disk/core.c > +++ b/tools/kvm/disk/core.c > @@ -152,7 +152,7 @@ ssize_t disk_image__read(struct disk_image *disk,= u64 sector, const struct iovec > } > =20 > if (disk->disk_req_cb) > - disk->disk_req_cb(param); > + disk->disk_req_cb(param, total); > =20 > return total; > } > @@ -183,7 +183,7 @@ ssize_t disk_image__write(struct disk_image *disk= , u64 sector, const struct iove > } > =20 > if (disk->disk_req_cb) > - disk->disk_req_cb(param); > + disk->disk_req_cb(param, total); > =20 > return total; > } > @@ -199,7 +199,7 @@ ssize_t disk_image__get_serial(struct disk_image = *disk, void *buffer, ssize_t *l > return *len; > } > =20 > -void disk_image__set_callback(struct disk_image *disk, void (*disk_r= eq_cb)(void *param)) > +void disk_image__set_callback(struct disk_image *disk, void (*disk_r= eq_cb)(void *param, long len)) > { > disk->disk_req_cb =3D disk_req_cb; > } > diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/k= vm/disk-image.h > index bae9744..bcb3396 100644 > --- a/tools/kvm/include/kvm/disk-image.h > +++ b/tools/kvm/include/kvm/disk-image.h > @@ -45,7 +45,7 @@ struct disk_image { > struct disk_image_operations ops; > void *priv; > void *disk_req_cb_param; > - void (*disk_req_cb)(void *param); > + void (*disk_req_cb)(void *param, long len); > }; > =20 > struct disk_image *disk_image__open(const char *filename, bool reado= nly); > @@ -72,5 +72,5 @@ ssize_t raw_image__read_sector_mmap(struct disk_ima= ge *disk, u64 sector, > ssize_t raw_image__write_sector_mmap(struct disk_image *disk, u64 se= ctor, > const struct iovec *iov, int iovcount, void *param); > int raw_image__close(struct disk_image *disk); > -void disk_image__set_callback(struct disk_image *disk, void (*disk_r= eq_cb)(void *param)); > +void disk_image__set_callback(struct disk_image *disk, void (*disk_r= eq_cb)(void *param, long len)); > #endif /* KVM__DISK_IMAGE_H */ > diff --git a/tools/kvm/include/kvm/virtio-blk.h b/tools/kvm/include/k= vm/virtio-blk.h > index 8c4fb91..63731a9 100644 > --- a/tools/kvm/include/kvm/virtio-blk.h > +++ b/tools/kvm/include/kvm/virtio-blk.h > @@ -8,5 +8,6 @@ struct kvm; > void virtio_blk__init(struct kvm *kvm, struct disk_image *disk); > void virtio_blk__init_all(struct kvm *kvm); > void virtio_blk__delete_all(struct kvm *kvm); > +void virtio_blk_complete(void *param, long len); > =20 > #endif /* KVM__BLK_VIRTIO_H */ > diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c > index 9db293e..0688f82 100644 > --- a/tools/kvm/virtio/blk.c > +++ b/tools/kvm/virtio/blk.c > @@ -29,17 +29,21 @@ > */ > #define DISK_SEG_MAX (VIRTIO_BLK_QUEUE_SIZE - 2) > =20 > -struct blk_dev_job { > +struct blk_dev_req { > + struct list_head list; > struct virt_queue *vq; > struct blk_dev *bdev; > struct iovec iov[VIRTIO_BLK_QUEUE_SIZE]; > u16 out, in, head; > - struct thread_pool__job job_id; > + struct kvm *kvm; > }; > =20 > struct blk_dev { > pthread_mutex_t mutex; > + pthread_mutex_t req_mutex; > + > struct list_head list; > + struct list_head req_list; > =20 > struct virtio_pci vpci; > struct virtio_blk_config blk_config; > @@ -47,41 +51,76 @@ struct blk_dev { > u32 features; > =20 > struct virt_queue vqs[NUM_VIRT_QUEUES]; > - struct blk_dev_job jobs[VIRTIO_BLK_QUEUE_SIZE]; > - u16 job_idx; > + struct blk_dev_req reqs[VIRTIO_BLK_QUEUE_SIZE]; > }; > =20 > static LIST_HEAD(bdevs); > static int compat_id; > =20 > -static void virtio_blk_do_io_request(struct kvm *kvm, void *param) > +static struct blk_dev_req *virtio_blk_req_pop(struct blk_dev *bdev) > +{ > + struct blk_dev_req *req; > + > + mutex_lock(&bdev->req_mutex); > + req =3D list_first_entry(&bdev->req_list, struct blk_dev_req, list)= ; > + list_del_init(&req->list); > + mutex_unlock(&bdev->req_mutex); > + > + return req; > +} > + > +static void virtio_blk_req_push(struct blk_dev *bdev, struct blk_dev= _req *req) > { > - struct virtio_blk_outhdr *req; > + mutex_lock(&bdev->req_mutex); > + list_add(&req->list, &bdev->req_list); > + mutex_unlock(&bdev->req_mutex); > +} > + > +void virtio_blk_complete(void *param, long len) > +{ > + struct blk_dev_req *req =3D param; > + struct blk_dev *bdev =3D req->bdev; > + int queueid =3D req->vq - bdev->vqs; > u8 *status; > + > + /* status */ > + status =3D req->iov[req->out + req->in - 1].iov_base; > + *status =3D (len < 0) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK; > + > + mutex_lock(&bdev->mutex); > + virt_queue__set_used_elem(req->vq, req->head, len); > + mutex_unlock(&bdev->mutex); > + > + virtio_pci__signal_vq(req->kvm, &bdev->vpci, queueid); > + > + virtio_blk_req_push(req->bdev, req); > +} > + > +static void virtio_blk_do_io_request(struct kvm *kvm, struct blk_dev= _req *req) > +{ > + struct virtio_blk_outhdr *req_hdr; > ssize_t block_cnt; > - struct blk_dev_job *job; > struct blk_dev *bdev; > struct virt_queue *queue; > struct iovec *iov; > u16 out, in, head; > =20 > block_cnt =3D -1; > - job =3D param; > - bdev =3D job->bdev; > - queue =3D job->vq; > - iov =3D job->iov; > - out =3D job->out; > - in =3D job->in; > - head =3D job->head; > - req =3D iov[0].iov_base; > - > - switch (req->type) { > + bdev =3D req->bdev; > + queue =3D req->vq; > + iov =3D req->iov; > + out =3D req->out; > + in =3D req->in; > + head =3D req->head; Hi, Sasha Please drop these queue and head local variables. virtio/blk.c: In function =E2=80=98virtio_blk_do_io_request=E2=80=99: virtio/blk.c:106:15: error: variable =E2=80=98head=E2=80=99 set but not= used [-Werror=3Dunused-but-set-variable] virtio/blk.c:104:21: error: variable =E2=80=98queue=E2=80=99 set but no= t used [-Werror=3Dunused-but-set-variable] cc1: all warnings being treated as errors > + req_hdr =3D iov[0].iov_base; > + > + switch (req_hdr->type) { > case VIRTIO_BLK_T_IN: > - block_cnt =3D disk_image__read(bdev->disk, req->sector, iov + 1, > + block_cnt =3D disk_image__read(bdev->disk, req_hdr->sector, iov + = 1, > in + out - 2, NULL); > break; > case VIRTIO_BLK_T_OUT: > - block_cnt =3D disk_image__write(bdev->disk, req->sector, iov + 1, > + block_cnt =3D disk_image__write(bdev->disk, req_hdr->sector, iov += 1, > in + out - 2, NULL); > break; > case VIRTIO_BLK_T_FLUSH: > @@ -92,35 +131,27 @@ static void virtio_blk_do_io_request(struct kvm = *kvm, void *param) > disk_image__get_serial(bdev->disk, (iov + 1)->iov_base, &block_cnt= ); > break; > default: > - pr_warning("request type %d", req->type); > + pr_warning("request type %d", req_hdr->type); > block_cnt =3D -1; > break; > } > =20 > - /* status */ > - status =3D iov[out + in - 1].iov_base; > - *status =3D (block_cnt < 0) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_O= K; > - > - mutex_lock(&bdev->mutex); > - virt_queue__set_used_elem(queue, head, block_cnt); > - mutex_unlock(&bdev->mutex); > - > - virtio_pci__signal_vq(kvm, &bdev->vpci, queue - bdev->vqs); > + virtio_blk_complete(req, block_cnt); > } > =20 > static void virtio_blk_do_io(struct kvm *kvm, struct virt_queue *vq,= struct blk_dev *bdev) > { > while (virt_queue__available(vq)) { > - struct blk_dev_job *job =3D &bdev->jobs[bdev->job_idx++ % VIRTIO_B= LK_QUEUE_SIZE]; > + struct blk_dev_req *req =3D virtio_blk_req_pop(bdev); > =20 > - *job =3D (struct blk_dev_job) { > - .vq =3D vq, > - .bdev =3D bdev, > + *req =3D (struct blk_dev_req) { > + .vq =3D vq, > + .bdev =3D bdev, > + .kvm =3D kvm, > }; > - job->head =3D virt_queue__get_iov(vq, job->iov, &job->out, &job->i= n, kvm); > + req->head =3D virt_queue__get_iov(vq, req->iov, &req->out, &req->i= n, kvm); > =20 > - thread_pool__init_job(&job->job_id, kvm, virtio_blk_do_io_request,= job); > - thread_pool__do_job(&job->job_id); > + virtio_blk_do_io_request(kvm, req); > } > } > =20 > @@ -191,6 +222,7 @@ static int get_size_vq(struct kvm *kvm, void *dev= , u32 vq) > void virtio_blk__init(struct kvm *kvm, struct disk_image *disk) > { > struct blk_dev *bdev; > + size_t i; > =20 > if (!disk) > return; > @@ -201,6 +233,7 @@ void virtio_blk__init(struct kvm *kvm, struct dis= k_image *disk) > =20 > *bdev =3D (struct blk_dev) { > .mutex =3D PTHREAD_MUTEX_INITIALIZER, > + .req_mutex =3D PTHREAD_MUTEX_INITIALIZER, > .disk =3D disk, > .blk_config =3D (struct virtio_blk_config) { > .capacity =3D disk->size / SECTOR_SIZE, > @@ -222,6 +255,10 @@ void virtio_blk__init(struct kvm *kvm, struct di= sk_image *disk) > =20 > list_add_tail(&bdev->list, &bdevs); > =20 > + INIT_LIST_HEAD(&bdev->req_list); > + for (i =3D 0; i < ARRAY_SIZE(bdev->reqs); i++) > + list_add(&bdev->reqs[i].list, &bdev->req_list); > + > if (compat_id !=3D -1) > compat_id =3D compat__add_message("virtio-blk device was not detec= ted", > "While you have requested a virtio-blk device, " --=20 Asias He