From: Asias He <asias.hejun@gmail.com>
To: Sasha Levin <levinsasha928@gmail.com>
Cc: penberg@cs.helsinki.fi, kvm@vger.kernel.org, mingo@elte.hu,
gorcunov@gmail.com
Subject: Re: [PATCH 08/11] kvm tools: Split io request from completion
Date: Mon, 31 Oct 2011 18:42:50 +0800 [thread overview]
Message-ID: <4EAE7BAA.5070407@gmail.com> (raw)
In-Reply-To: <1319996135-17501-9-git-send-email-levinsasha928@gmail.com>
On 10/31/2011 01:35 AM, Sasha Levin wrote:
> This patch splits IO request processing from completion notification.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
> 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(-)
>
> 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
> }
>
> if (disk->disk_req_cb)
> - disk->disk_req_cb(param);
> + disk->disk_req_cb(param, total);
>
> return total;
> }
> @@ -183,7 +183,7 @@ ssize_t disk_image__write(struct disk_image *disk, u64 sector, const struct iove
> }
>
> if (disk->disk_req_cb)
> - disk->disk_req_cb(param);
> + disk->disk_req_cb(param, total);
>
> return total;
> }
> @@ -199,7 +199,7 @@ ssize_t disk_image__get_serial(struct disk_image *disk, void *buffer, ssize_t *l
> return *len;
> }
>
> -void disk_image__set_callback(struct disk_image *disk, void (*disk_req_cb)(void *param))
> +void disk_image__set_callback(struct disk_image *disk, void (*disk_req_cb)(void *param, long len))
> {
> disk->disk_req_cb = disk_req_cb;
> }
> diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/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);
> };
>
> struct disk_image *disk_image__open(const char *filename, bool readonly);
> @@ -72,5 +72,5 @@ ssize_t raw_image__read_sector_mmap(struct disk_image *disk, u64 sector,
> ssize_t raw_image__write_sector_mmap(struct disk_image *disk, u64 sector,
> 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_req_cb)(void *param));
> +void disk_image__set_callback(struct disk_image *disk, void (*disk_req_cb)(void *param, long len));
> #endif /* KVM__DISK_IMAGE_H */
> diff --git a/tools/kvm/include/kvm/virtio-blk.h b/tools/kvm/include/kvm/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);
>
> #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)
>
> -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;
> };
>
> struct blk_dev {
> pthread_mutex_t mutex;
> + pthread_mutex_t req_mutex;
> +
> struct list_head list;
> + struct list_head req_list;
>
> struct virtio_pci vpci;
> struct virtio_blk_config blk_config;
> @@ -47,41 +51,76 @@ struct blk_dev {
> u32 features;
>
> 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];
> };
>
> static LIST_HEAD(bdevs);
> static int compat_id;
>
> -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 = 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 = param;
> + struct blk_dev *bdev = req->bdev;
> + int queueid = req->vq - bdev->vqs;
> u8 *status;
> +
> + /* status */
> + status = req->iov[req->out + req->in - 1].iov_base;
> + *status = (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;
>
> block_cnt = -1;
> - job = param;
> - bdev = job->bdev;
> - queue = job->vq;
> - iov = job->iov;
> - out = job->out;
> - in = job->in;
> - head = job->head;
> - req = iov[0].iov_base;
> -
> - switch (req->type) {
> + bdev = req->bdev;
> + queue = req->vq;
> + iov = req->iov;
> + out = req->out;
> + in = req->in;
> + head = req->head;
Hi, Sasha
Please drop these queue and head local variables.
virtio/blk.c: In function ‘virtio_blk_do_io_request’:
virtio/blk.c:106:15: error: variable ‘head’ set but not used
[-Werror=unused-but-set-variable]
virtio/blk.c:104:21: error: variable ‘queue’ set but not used
[-Werror=unused-but-set-variable]
cc1: all warnings being treated as errors
> + req_hdr = iov[0].iov_base;
> +
> + switch (req_hdr->type) {
> case VIRTIO_BLK_T_IN:
> - block_cnt = disk_image__read(bdev->disk, req->sector, iov + 1,
> + block_cnt = disk_image__read(bdev->disk, req_hdr->sector, iov + 1,
> in + out - 2, NULL);
> break;
> case VIRTIO_BLK_T_OUT:
> - block_cnt = disk_image__write(bdev->disk, req->sector, iov + 1,
> + block_cnt = 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 = -1;
> break;
> }
>
> - /* status */
> - status = iov[out + in - 1].iov_base;
> - *status = (block_cnt < 0) ? VIRTIO_BLK_S_IOERR : VIRTIO_BLK_S_OK;
> -
> - 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);
> }
>
> 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 = &bdev->jobs[bdev->job_idx++ % VIRTIO_BLK_QUEUE_SIZE];
> + struct blk_dev_req *req = virtio_blk_req_pop(bdev);
>
> - *job = (struct blk_dev_job) {
> - .vq = vq,
> - .bdev = bdev,
> + *req = (struct blk_dev_req) {
> + .vq = vq,
> + .bdev = bdev,
> + .kvm = kvm,
> };
> - job->head = virt_queue__get_iov(vq, job->iov, &job->out, &job->in, kvm);
> + req->head = virt_queue__get_iov(vq, req->iov, &req->out, &req->in, kvm);
>
> - 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);
> }
> }
>
> @@ -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;
>
> if (!disk)
> return;
> @@ -201,6 +233,7 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
>
> *bdev = (struct blk_dev) {
> .mutex = PTHREAD_MUTEX_INITIALIZER,
> + .req_mutex = PTHREAD_MUTEX_INITIALIZER,
> .disk = disk,
> .blk_config = (struct virtio_blk_config) {
> .capacity = disk->size / SECTOR_SIZE,
> @@ -222,6 +255,10 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
>
> list_add_tail(&bdev->list, &bdevs);
>
> + INIT_LIST_HEAD(&bdev->req_list);
> + for (i = 0; i < ARRAY_SIZE(bdev->reqs); i++)
> + list_add(&bdev->reqs[i].list, &bdev->req_list);
> +
> if (compat_id != -1)
> compat_id = compat__add_message("virtio-blk device was not detected",
> "While you have requested a virtio-blk device, "
--
Asias He
next prev parent reply other threads:[~2011-10-31 10:43 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-10-30 17:35 [PATCH 00/11] kvm tools: Support native vectored AIO Sasha Levin
2011-10-30 17:35 ` [PATCH 01/11] kvm tools: Switch to using an enum for disk image types Sasha Levin
2011-10-30 17:35 ` [PATCH 02/11] kvm tools: Hold a copy of ops struct inside disk_image Sasha Levin
2011-11-01 6:44 ` Pekka Enberg
2011-11-01 11:16 ` Sasha Levin
2011-11-01 11:30 ` Pekka Enberg
2011-11-01 11:43 ` Sasha Levin
2011-11-01 11:52 ` Pekka Enberg
2011-10-30 17:35 ` [PATCH 03/11] kvm tools: Remove the non-iov interface from disk image ops Sasha Levin
2011-10-30 17:35 ` [PATCH 04/11] kvm tools: Modify disk ops usage Sasha Levin
2011-10-30 17:35 ` [PATCH 05/11] kvm tools: Modify behaviour on missing ops ptr Sasha Levin
2011-10-30 17:35 ` [PATCH 06/11] kvm tools: Add optional callback on disk op completion Sasha Levin
2011-10-30 17:35 ` [PATCH 07/11] kvm tools: Remove qcow nowrite function Sasha Levin
2011-10-30 17:35 ` [PATCH 08/11] kvm tools: Split io request from completion Sasha Levin
2011-10-31 10:42 ` Asias He [this message]
2011-10-30 17:35 ` [PATCH 09/11] kvm tools: Hook virtio-blk completion to disk op completion Sasha Levin
2011-10-30 17:35 ` [PATCH 10/11] kvm tools: Add aio read write functions Sasha Levin
2011-10-30 17:35 ` [PATCH 11/11] kvm tools: Use native vectored AIO in virtio-blk Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4EAE7BAA.5070407@gmail.com \
--to=asias.hejun@gmail.com \
--cc=gorcunov@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=levinsasha928@gmail.com \
--cc=mingo@elte.hu \
--cc=penberg@cs.helsinki.fi \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).