On 4/13/20 6:12 AM, Denis Plotnikov wrote:
The patch adds ability to qemu-file to write the data
asynchronously to improve the performance on writing.
Before, only synchronous writing was supported.
Enabling of the asyncronous mode is managed by new
asynchronous
"enabled_buffered" callback.
The term "enabled_buffered" does not appear in the patch. Did you
mean...
Signed-off-by: Denis Plotnikov <dplotni...@virtuozzo.com>
---
include/qemu/typedefs.h | 1 +
migration/qemu-file.c | 351
+++++++++++++++++++++++++++++++++++++++++++++---
migration/qemu-file.h | 9 ++
3 files changed, 339 insertions(+), 22 deletions(-)
@@ -60,6 +66,22 @@ struct QEMUFile {
bool shutdown;
/* currently used buffer */
QEMUFileBuffer *current_buf;
+ /*
+ * with buffered_mode enabled all the data copied to 512 byte
+ * aligned buffer, including iov data. Then the buffer is passed
+ * to writev_buffer callback.
+ */
+ bool buffered_mode;
..."Asynchronous mode is managed by setting the new buffered_mode
flag"? ...
+ /* for async buffer writing */
+ AioTaskPool *pool;
+ /* the list of free buffers, currently used on is NOT there */
s/on/one/
+ QLIST_HEAD(, QEMUFileBuffer) free_buffers;
+};
+
+struct QEMUFileAioTask {
+ AioTask task;
+ QEMUFile *f;
+ QEMUFileBuffer *fb;
};
/*
@@ -115,10 +137,42 @@ QEMUFile *qemu_fopen_ops(void *opaque, const
QEMUFileOps *ops)
f->opaque = opaque;
f->ops = ops;
- f->current_buf = g_new0(QEMUFileBuffer, 1);
- f->current_buf->buf = g_malloc(IO_BUF_SIZE);
- f->current_buf->iov = g_new0(struct iovec, MAX_IOV_SIZE);
- f->current_buf->may_free = bitmap_new(MAX_IOV_SIZE);
+ if (f->ops->enable_buffered) {
+ f->buffered_mode = f->ops->enable_buffered(f->opaque);
...ah, you meant 'enable_buffered'. But still, why do we need a
callback function? Is it not sufficient to just have a bool flag?
+static size_t get_buf_free_size(QEMUFile *f)
+{
+ QEMUFileBuffer *fb = f->current_buf;
+ /* buf_index can't be greated than buf_size */
greater
+ assert(fb->buf_size >= fb->buf_index);
+ return fb->buf_size - fb->buf_index;
+}
+
+static int write_task_fn(AioTask *task)
+{
+ /*
+ * Increment file position.
+ * This needs to be here before calling writev_buffer, because
+ * writev_buffer is asynchronous and there could be more than one
+ * writev_buffer started simultaniously. Each writev_buffer should
simultaneously
+ * use its own file pos to write to. writev_buffer may write less
+ * than buf_index bytes but we treat this situation as an error.
+ * If error appeared, further file using is meaningless.
s/using/use/
+ * We expect that, the most of the time the full buffer is written,
+ * (when buf_size == buf_index). The only case when the non-full
+ * buffer is written (buf_size != buf_index) is file close,
+ * when we need to flush the rest of the buffer content.
We expect that most of the time, the full buffer will be written
(buf_size == buf_index), with the exception at file close where we
need to flush the final partial buffer.
+ */
+ f->pos += fb->buf_index;
+
+ ret = f->ops->writev_buffer(f->opaque, &v, 1, pos, &local_error);
+
+ /* return the just written buffer to the free list */
+ QLIST_INSERT_HEAD(&f->free_buffers, fb, link);
+
+ /* check that we have written everything */
+ if (ret != fb->buf_index) {
+ qemu_file_set_error_obj(f, ret < 0 ? ret : -EIO, local_error);
+ }
+
+ /*
+ * always return 0 - don't use task error handling, relay on
rely
+ * qemu file error handling
+ */
+ return 0;
+}
+
+static void qemu_file_switch_current_buf(QEMUFile *f)
+{
+ /*
+ * if the list is empty, wait until some task returns a buffer
+ * to the list of free buffers.
+ */
+ if (QLIST_EMPTY(&f->free_buffers)) {
+ aio_task_pool_wait_slot(f->pool);
+ }
+
+ /*
+ * sanity check that the list isn't empty
+ * if the free list was empty, we waited for a task complition,
completion
+ * and the pompleted task must return a buffer to a list of free
buffers
completed
+ */
+ assert(!QLIST_EMPTY(&f->free_buffers));
+
+ /* set the current buffer for using from the free list */
+ f->current_buf = QLIST_FIRST(&f->free_buffers);
+ reset_buf(f);
+
+ QLIST_REMOVE(f->current_buf, link);
+}
+
/*
+ * Copy an external buffer to the intenal current buffer.
internal
+ */
+static void copy_buf(QEMUFile *f, const uint8_t *buf, size_t size,
+ bool may_free)
+{
+++ b/migration/qemu-file.h
@@ -103,6 +103,14 @@ typedef QEMUFile *(QEMURetPathFunc)(void *opaque);
typedef int (QEMUFileShutdownFunc)(void *opaque, bool rd, bool wr,
Error **errp);
+/*
+ * Enables or disables the buffered mode
+ * Existing blocking reads/writes must be woken
+ * Returns true if the buffered mode has to be enabled,
+ * false if it has to be disabled.
+ */
+typedef bool (QEMUFileEnableBufferedFunc)(void *opaque);
If this never gets called outside of initial creation of the QemuFile
(that is, it is not dynamic), then making it a straight flag instead
of a callback function is simpler.
