Re: vmd vioblk start/finish
On Tue, May 30, 2017 at 07:00:08AM -0400, Ted Unangst wrote:
> This splits the read/write functions into top and bottom halves. It doesn't
> change much yet, but this is a requirement for async IO. The start funtion
> turns the request into an ioinfo (to be completed eventually by a thread) and
> the finish function retrives the result. (for now, we just do the work in
> finish.)
>
> seems to work, but could probably use a little more load testing.
reads ok to me, go for it (and thanks)
>
> Index: virtio.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 virtio.c
> --- virtio.c 27 May 2017 23:58:16 - 1.47
> +++ virtio.c 30 May 2017 10:56:47 -
> @@ -62,6 +62,14 @@ int nr_vioblk;
> #define VMMCI_F_ACK (1<<1)
> #define VMMCI_F_SYNCRTC (1<<2)
>
> +struct ioinfo {
> + uint8_t *buf;
> + ssize_t len;
> + off_t offset;
> + int fd;
> + int error;
> +};
> +
> const char *
> vioblk_cmd_name(uint32_t type)
> {
> @@ -324,35 +332,87 @@ vioblk_update_qs(struct vioblk_dev *dev)
> dev->cfg.queue_size = dev->vq[dev->cfg.queue_select].qs;
> }
>
> -static char *
> -vioblk_do_read(struct vioblk_dev *dev, off_t sector, ssize_t sz)
> +static void
> +vioblk_free_info(struct ioinfo *info)
> {
> - char *buf;
> + if (!info)
> + return;
> + free(info->buf);
> + free(info);
> +}
>
> - buf = malloc(sz);
> - if (buf == NULL) {
> - log_warn("malloc errror vioblk read");
> - return (NULL);
> - }
> +static struct ioinfo *
> +vioblk_start_read(struct vioblk_dev *dev, off_t sector, ssize_t sz)
> +{
> + struct ioinfo *info;
> +
> + info = calloc(1, sizeof(*info));
> + if (!info)
> + goto nomem;
> + info->buf = malloc(sz);
> + if (info->buf == NULL)
> + goto nomem;
> + info->len = sz;
> + info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
> + info->fd = dev->fd;
> +
> + return info;
>
> - if (pread(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
> +nomem:
> + free(info);
> + log_warn("malloc errror vioblk read");
> + return (NULL);
> +}
> +
> +
> +static const uint8_t *
> +vioblk_finish_read(struct ioinfo *info)
> +{
> + if (pread(info->fd, info->buf, info->len, info->offset) != info->len) {
> + info->error = errno;
> log_warn("vioblk read error");
> - free(buf);
> - return (NULL);
> + return NULL;
> }
>
> - return buf;
> + return info->buf;
> +}
> +
> +static struct ioinfo *
> +vioblk_start_write(struct vioblk_dev *dev, off_t sector, paddr_t addr,
> size_t len)
> +{
> + struct ioinfo *info;
> +
> + info = calloc(1, sizeof(*info));
> + if (!info)
> + goto nomem;
> + info->buf = malloc(len);
> + if (info->buf == NULL)
> + goto nomem;
> + info->len = len;
> + info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
> + info->fd = dev->fd;
> +
> + if (read_mem(addr, info->buf, len)) {
> + vioblk_free_info(info);
> + return NULL;
> + }
> +
> + return info;
> +
> +nomem:
> + free(info);
> + log_warn("malloc errror vioblk write");
> + return (NULL);
> }
>
> static int
> -vioblk_do_write(struct vioblk_dev *dev, off_t sector, char *buf, ssize_t sz)
> +vioblk_finish_write(struct ioinfo *info)
> {
> - if (pwrite(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
> + if (pwrite(info->fd, info->buf, info->len, info->offset) != info->len) {
> log_warn("vioblk write error");
> - return (1);
> + return EIO;
> }
> -
> - return (0);
> + return 0;
> }
>
> /*
> @@ -368,7 +428,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
> uint8_t ds;
> int ret;
> off_t secbias;
> - char *vr, *secdata;
> + char *vr;
> struct vring_desc *desc, *cmd_desc, *secdata_desc, *ds_desc;
> struct vring_avail *avail;
> struct vring_used *used;
> @@ -441,14 +501,16 @@ vioblk_notifyq(struct vioblk_dev *dev)
>
> secbias = 0;
> do {
> - /* read the data (use current data descriptor)
> */
> - /*
> - * XXX waste to malloc secdata in vioblk_do_read
> - * and free it here over and over
> - */
> - secdata = vioblk_do_read(dev, cmd.sector +
> secbias,
> + struct ioinfo *info;
> + const uint8_t *secdata;
> +
> + info = vioblk_start_read(dev, cmd.sector +
> secbias,
> (ssize_t)secdata_desc->len);
> +
> + /* read
vmd vioblk start/finish
This splits the read/write functions into top and bottom halves. It doesn't
change much yet, but this is a requirement for async IO. The start funtion
turns the request into an ioinfo (to be completed eventually by a thread) and
the finish function retrives the result. (for now, we just do the work in
finish.)
seems to work, but could probably use a little more load testing.
Index: virtio.c
===
RCS file: /cvs/src/usr.sbin/vmd/virtio.c,v
retrieving revision 1.47
diff -u -p -r1.47 virtio.c
--- virtio.c27 May 2017 23:58:16 - 1.47
+++ virtio.c30 May 2017 10:56:47 -
@@ -62,6 +62,14 @@ int nr_vioblk;
#define VMMCI_F_ACK(1<<1)
#define VMMCI_F_SYNCRTC(1<<2)
+struct ioinfo {
+ uint8_t *buf;
+ ssize_t len;
+ off_t offset;
+ int fd;
+ int error;
+};
+
const char *
vioblk_cmd_name(uint32_t type)
{
@@ -324,35 +332,87 @@ vioblk_update_qs(struct vioblk_dev *dev)
dev->cfg.queue_size = dev->vq[dev->cfg.queue_select].qs;
}
-static char *
-vioblk_do_read(struct vioblk_dev *dev, off_t sector, ssize_t sz)
+static void
+vioblk_free_info(struct ioinfo *info)
{
- char *buf;
+ if (!info)
+ return;
+ free(info->buf);
+ free(info);
+}
- buf = malloc(sz);
- if (buf == NULL) {
- log_warn("malloc errror vioblk read");
- return (NULL);
- }
+static struct ioinfo *
+vioblk_start_read(struct vioblk_dev *dev, off_t sector, ssize_t sz)
+{
+ struct ioinfo *info;
+
+ info = calloc(1, sizeof(*info));
+ if (!info)
+ goto nomem;
+ info->buf = malloc(sz);
+ if (info->buf == NULL)
+ goto nomem;
+ info->len = sz;
+ info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
+ info->fd = dev->fd;
+
+ return info;
- if (pread(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
+nomem:
+ free(info);
+ log_warn("malloc errror vioblk read");
+ return (NULL);
+}
+
+
+static const uint8_t *
+vioblk_finish_read(struct ioinfo *info)
+{
+ if (pread(info->fd, info->buf, info->len, info->offset) != info->len) {
+ info->error = errno;
log_warn("vioblk read error");
- free(buf);
- return (NULL);
+ return NULL;
}
- return buf;
+ return info->buf;
+}
+
+static struct ioinfo *
+vioblk_start_write(struct vioblk_dev *dev, off_t sector, paddr_t addr, size_t
len)
+{
+ struct ioinfo *info;
+
+ info = calloc(1, sizeof(*info));
+ if (!info)
+ goto nomem;
+ info->buf = malloc(len);
+ if (info->buf == NULL)
+ goto nomem;
+ info->len = len;
+ info->offset = sector * VIRTIO_BLK_SECTOR_SIZE;
+ info->fd = dev->fd;
+
+ if (read_mem(addr, info->buf, len)) {
+ vioblk_free_info(info);
+ return NULL;
+ }
+
+ return info;
+
+nomem:
+ free(info);
+ log_warn("malloc errror vioblk write");
+ return (NULL);
}
static int
-vioblk_do_write(struct vioblk_dev *dev, off_t sector, char *buf, ssize_t sz)
+vioblk_finish_write(struct ioinfo *info)
{
- if (pwrite(dev->fd, buf, sz, sector * VIRTIO_BLK_SECTOR_SIZE) != sz) {
+ if (pwrite(info->fd, info->buf, info->len, info->offset) != info->len) {
log_warn("vioblk write error");
- return (1);
+ return EIO;
}
-
- return (0);
+ return 0;
}
/*
@@ -368,7 +428,7 @@ vioblk_notifyq(struct vioblk_dev *dev)
uint8_t ds;
int ret;
off_t secbias;
- char *vr, *secdata;
+ char *vr;
struct vring_desc *desc, *cmd_desc, *secdata_desc, *ds_desc;
struct vring_avail *avail;
struct vring_used *used;
@@ -441,14 +501,16 @@ vioblk_notifyq(struct vioblk_dev *dev)
secbias = 0;
do {
- /* read the data (use current data descriptor)
*/
- /*
-* XXX waste to malloc secdata in vioblk_do_read
-* and free it here over and over
-*/
- secdata = vioblk_do_read(dev, cmd.sector +
secbias,
+ struct ioinfo *info;
+ const uint8_t *secdata;
+
+ info = vioblk_start_read(dev, cmd.sector +
secbias,
(ssize_t)secdata_desc->len);
+
+ /* read the data (use current data descriptor)
*/
+ secdata = vioblk_finish_read(info);
if (secdata == NULL) {
+ vioblk_free_info(info);
