haraldwolff/qemu-patch-raspberry4
1
0
Fork 0
Code Issues Pull requests Packages Projects Releases Wiki Activity

virtio-blk: embed VirtQueueElement in VirtIOBlockReq

Browse source 
The memory allocation between hw/block/virtio-blk.c,
hw/block/dataplane/virtio-blk.c, and hw/virtio/dataplane/vring.c is
messy.  Structs are allocated in different files than they are freed in.
This is risky and makes memory leaks easier.

Embed VirtQueueElement in VirtIOBlockReq to reduce the amount of memory
allocation we need to juggle.  This also makes vring.c and virtio.c
slightly more similar.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Stefan Hajnoczi 2014-07-09 10:05:49 +02:00 committed by Kevin Wolf
parent 869d66af53
commit f897bf751f
5 changed files with 48 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
hw/block/dataplane/virtio-blk.c
View file

@ -65,7 +65,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status)
{ {
stb_p(&req->in->status, status); stb_p(&req->in->status, status);
vring_push(&req->dev->dataplane->vring, req->elem, vring_push(&req->dev->dataplane->vring, &req->elem,
req->qiov.size + sizeof(*req->in)); req->qiov.size + sizeof(*req->in));
notify_guest(req->dev->dataplane); notify_guest(req->dev->dataplane);
} }
@ -74,33 +74,32 @@ static void handle_notify(EventNotifier *e)
{ {
VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane, VirtIOBlockDataPlane *s = container_of(e, VirtIOBlockDataPlane,
host_notifier); host_notifier);
VirtIOBlock *vblk = VIRTIO_BLK(s->vdev);
VirtQueueElement *elem;
VirtIOBlockReq *req;
int ret;
MultiReqBuffer mrb = {
.num_writes = 0,
};
event_notifier_test_and_clear(&s->host_notifier); event_notifier_test_and_clear(&s->host_notifier);
bdrv_io_plug(s->blk->conf.bs); bdrv_io_plug(s->blk->conf.bs);
for (;;) { for (;;) {
MultiReqBuffer mrb = {
.num_writes = 0,
};
int ret;
/* Disable guest->host notifies to avoid unnecessary vmexits */ /* Disable guest->host notifies to avoid unnecessary vmexits */
vring_disable_notification(s->vdev, &s->vring); vring_disable_notification(s->vdev, &s->vring);
for (;;) { for (;;) {
ret = vring_pop(s->vdev, &s->vring, &elem); VirtIOBlockReq *req = virtio_blk_alloc_request(vblk);
ret = vring_pop(s->vdev, &s->vring, &req->elem);
if (ret < 0) { if (ret < 0) {
assert(elem == NULL); virtio_blk_free_request(req);
break; /* no more requests */ break; /* no more requests */
} }
trace_virtio_blk_data_plane_process_request(s, elem->out_num, trace_virtio_blk_data_plane_process_request(s, req->elem.out_num,
elem->in_num, elem->index); req->elem.in_num,
req->elem.index);
req = g_slice_new(VirtIOBlockReq);
req->dev = VIRTIO_BLK(s->vdev);
req->elem = elem;
virtio_blk_handle_request(req, &mrb); virtio_blk_handle_request(req, &mrb);
} }

46
hw/block/virtio-blk.c
View file

@ -29,20 +29,18 @@
#include "hw/virtio/virtio-bus.h" #include "hw/virtio/virtio-bus.h"
#include "hw/virtio/virtio-access.h" #include "hw/virtio/virtio-access.h"
static VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s) VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s)
{ {
VirtIOBlockReq *req = g_slice_new(VirtIOBlockReq); VirtIOBlockReq *req = g_slice_new(VirtIOBlockReq);
req->dev = s; req->dev = s;
req->qiov.size = 0; req->qiov.size = 0;
req->next = NULL; req->next = NULL;
req->elem = g_slice_new(VirtQueueElement);
return req; return req;
} }
static void virtio_blk_free_request(VirtIOBlockReq *req) void virtio_blk_free_request(VirtIOBlockReq *req)
{ {
if (req) { if (req) {
g_slice_free(VirtQueueElement, req->elem);
g_slice_free(VirtIOBlockReq, req); g_slice_free(VirtIOBlockReq, req);
} }
} }
@ -56,7 +54,7 @@ static void virtio_blk_complete_request(VirtIOBlockReq *req,
trace_virtio_blk_req_complete(req, status); trace_virtio_blk_req_complete(req, status);
stb_p(&req->in->status, status); stb_p(&req->in->status, status);
virtqueue_push(s->vq, req->elem, req->qiov.size + sizeof(*req->in)); virtqueue_push(s->vq, &req->elem, req->qiov.size + sizeof(*req->in));
virtio_notify(vdev, s->vq); virtio_notify(vdev, s->vq);
} }
@ -121,7 +119,7 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s)
{ {
VirtIOBlockReq *req = virtio_blk_alloc_request(s); VirtIOBlockReq *req = virtio_blk_alloc_request(s);
if (!virtqueue_pop(s->vq, req->elem)) { if (!virtqueue_pop(s->vq, &req->elem)) {
virtio_blk_free_request(req); virtio_blk_free_request(req);
return NULL; return NULL;
} }
@ -254,7 +252,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
{ {
int status; int status;
status = virtio_blk_handle_scsi_req(req->dev, req->elem); status = virtio_blk_handle_scsi_req(req->dev, &req->elem);
virtio_blk_req_complete(req, status); virtio_blk_req_complete(req, status);
virtio_blk_free_request(req); virtio_blk_free_request(req);
} }
@ -351,12 +349,12 @@ static void virtio_blk_handle_read(VirtIOBlockReq *req)
void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
{ {
uint32_t type; uint32_t type;
struct iovec *in_iov = req->elem->in_sg; struct iovec *in_iov = req->elem.in_sg;
struct iovec *iov = req->elem->out_sg; struct iovec *iov = req->elem.out_sg;
unsigned in_num = req->elem->in_num; unsigned in_num = req->elem.in_num;
unsigned out_num = req->elem->out_num; unsigned out_num = req->elem.out_num;
if (req->elem->out_num < 1 || req->elem->in_num < 1) { if (req->elem.out_num < 1 || req->elem.in_num < 1) {
error_report("virtio-blk missing headers"); error_report("virtio-blk missing headers");
exit(1); exit(1);
} }
@ -393,19 +391,19 @@ void virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb)
* NB: per existing s/n string convention the string is * NB: per existing s/n string convention the string is
* terminated by '\0' only when shorter than buffer. * terminated by '\0' only when shorter than buffer.
*/ */
strncpy(req->elem->in_sg[0].iov_base, strncpy(req->elem.in_sg[0].iov_base,
s->blk.serial ? s->blk.serial : "", s->blk.serial ? s->blk.serial : "",
MIN(req->elem->in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES)); MIN(req->elem.in_sg[0].iov_len, VIRTIO_BLK_ID_BYTES));
virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); virtio_blk_req_complete(req, VIRTIO_BLK_S_OK);
virtio_blk_free_request(req); virtio_blk_free_request(req);
} else if (type & VIRTIO_BLK_T_OUT) { } else if (type & VIRTIO_BLK_T_OUT) {
qemu_iovec_init_external(&req->qiov, &req->elem->out_sg[1], qemu_iovec_init_external(&req->qiov, &req->elem.out_sg[1],
req->elem->out_num - 1); req->elem.out_num - 1);
virtio_blk_handle_write(req, mrb); virtio_blk_handle_write(req, mrb);
} else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) { } else if (type == VIRTIO_BLK_T_IN || type == VIRTIO_BLK_T_BARRIER) {
/* VIRTIO_BLK_T_IN is 0, so we can't just & it. */ /* VIRTIO_BLK_T_IN is 0, so we can't just & it. */
qemu_iovec_init_external(&req->qiov, &req->elem->in_sg[0], qemu_iovec_init_external(&req->qiov, &req->elem.in_sg[0],
req->elem->in_num - 1); req->elem.in_num - 1);
virtio_blk_handle_read(req); virtio_blk_handle_read(req);
} else { } else {
virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
@ -629,7 +627,7 @@ static void virtio_blk_save_device(VirtIODevice *vdev, QEMUFile *f)
while (req) { while (req) {
qemu_put_sbyte(f, 1); qemu_put_sbyte(f, 1);
qemu_put_buffer(f, (unsigned char *)req->elem, qemu_put_buffer(f, (unsigned char *)&req->elem,
sizeof(VirtQueueElement)); sizeof(VirtQueueElement));
req = req->next; req = req->next;
} }
@ -654,15 +652,15 @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f,
while (qemu_get_sbyte(f)) { while (qemu_get_sbyte(f)) {
VirtIOBlockReq *req = virtio_blk_alloc_request(s); VirtIOBlockReq *req = virtio_blk_alloc_request(s);
qemu_get_buffer(f, (unsigned char *)req->elem, qemu_get_buffer(f, (unsigned char *)&req->elem,
sizeof(VirtQueueElement)); sizeof(VirtQueueElement));
req->next = s->rq; req->next = s->rq;
s->rq = req; s->rq = req;
virtqueue_map_sg(req->elem->in_sg, req->elem->in_addr, virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr,
req->elem->in_num, 1); req->elem.in_num, 1);
virtqueue_map_sg(req->elem->out_sg, req->elem->out_addr, virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr,
req->elem->out_num, 0); req->elem.out_num, 0);
} }
return 0; return 0;

17
hw/virtio/dataplane/vring.c
View file

@ -301,14 +301,16 @@ static void vring_unmap_element(VirtQueueElement *elem)
* Stolen from linux/drivers/vhost/vhost.c. * Stolen from linux/drivers/vhost/vhost.c.
*/ */
int vring_pop(VirtIODevice *vdev, Vring *vring, int vring_pop(VirtIODevice *vdev, Vring *vring,
VirtQueueElement **p_elem) VirtQueueElement *elem)
{ {
struct vring_desc desc; struct vring_desc desc;
unsigned int i, head, found = 0, num = vring->vr.num; unsigned int i, head, found = 0, num = vring->vr.num;
uint16_t avail_idx, last_avail_idx; uint16_t avail_idx, last_avail_idx;
VirtQueueElement *elem = NULL;
int ret; int ret;
/* Initialize elem so it can be safely unmapped */
elem->in_num = elem->out_num = 0;
/* If there was a fatal error then refuse operation */ /* If there was a fatal error then refuse operation */
if (vring->broken) { if (vring->broken) {
ret = -EFAULT; ret = -EFAULT;
@ -340,10 +342,8 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
* the index we've seen. */ * the index we've seen. */
head = vring->vr.avail->ring[last_avail_idx % num]; head = vring->vr.avail->ring[last_avail_idx % num];
elem = g_slice_new(VirtQueueElement);
elem->index = head; elem->index = head;
elem->in_num = elem->out_num = 0;
/* If their number is silly, that's an error. */ /* If their number is silly, that's an error. */
if (unlikely(head >= num)) { if (unlikely(head >= num)) {
error_report("Guest says index %u > %u is available", head, num); error_report("Guest says index %u > %u is available", head, num);
@ -391,7 +391,6 @@ int vring_pop(VirtIODevice *vdev, Vring *vring,
/* On success, increment avail index. */ /* On success, increment avail index. */
vring->last_avail_idx++; vring->last_avail_idx++;
*p_elem = elem;
return head; return head;
out: out:
@ -399,11 +398,7 @@ out:
if (ret == -EFAULT) { if (ret == -EFAULT) {
vring->broken = true; vring->broken = true;
} }
if (elem) { vring_unmap_element(elem);
vring_unmap_element(elem);
g_slice_free(VirtQueueElement, elem);
}
*p_elem = NULL;
return ret; return ret;
} }

2
include/hw/virtio/dataplane/vring.h
View file

@ -53,7 +53,7 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n);
void vring_disable_notification(VirtIODevice *vdev, Vring *vring); void vring_disable_notification(VirtIODevice *vdev, Vring *vring);
bool vring_enable_notification(VirtIODevice *vdev, Vring *vring); bool vring_enable_notification(VirtIODevice *vdev, Vring *vring);
bool vring_should_notify(VirtIODevice *vdev, Vring *vring); bool vring_should_notify(VirtIODevice *vdev, Vring *vring);
int vring_pop(VirtIODevice *vdev, Vring *vring, VirtQueueElement **elem); int vring_pop(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem);
void vring_push(Vring *vring, VirtQueueElement *elem, int len); void vring_push(Vring *vring, VirtQueueElement *elem, int len);
#endif /* VRING_H */ #endif /* VRING_H */

6
include/hw/virtio/virtio-blk.h
View file

@ -144,7 +144,7 @@ typedef struct MultiReqBuffer {
typedef struct VirtIOBlockReq { typedef struct VirtIOBlockReq {
VirtIOBlock *dev; VirtIOBlock *dev;
VirtQueueElement *elem; VirtQueueElement elem;
struct virtio_blk_inhdr *in; struct virtio_blk_inhdr *in;
struct virtio_blk_outhdr out; struct virtio_blk_outhdr out;
QEMUIOVector qiov; QEMUIOVector qiov;
@ -152,6 +152,10 @@ typedef struct VirtIOBlockReq {
BlockAcctCookie acct; BlockAcctCookie acct;
} VirtIOBlockReq; } VirtIOBlockReq;
VirtIOBlockReq *virtio_blk_alloc_request(VirtIOBlock *s);
void virtio_blk_free_request(VirtIOBlockReq *req);
int virtio_blk_handle_scsi_req(VirtIOBlock *blk, int virtio_blk_handle_scsi_req(VirtIOBlock *blk,
VirtQueueElement *elem); VirtQueueElement *elem);