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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.