* [PATCH v3 1/6] block/block-copy: allocate buffer in block_copy_with_bounce_buffer
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
@ 2019-10-22 11:18 ` Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 2/6] block/block-copy: limit copy_range_size to 16 MiB Vladimir Sementsov-Ogievskiy
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-22 11:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, den, jsnow
Move bounce_buffer allocation block_copy_with_bounce_buffer. This
commit simplifies further work on implementing copying by larger chunks
(of different size) and further asynchronous handling of block_copy
iterations (with help of block/aio_task API).
Allocation works fast, a lot faster than disk io, so it's not a problem
that we now allocate/free bounce_buffer more times. And we anyway will
have to allocate several bounce_buffers for parallel execution of loop
iterations in future.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/block-copy.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index 066e3a7274..ecd086010e 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -126,20 +126,17 @@ void block_copy_set_callbacks(
static int coroutine_fn block_copy_with_bounce_buffer(BlockCopyState *s,
int64_t start,
int64_t end,
- bool *error_is_read,
- void **bounce_buffer)
+ bool *error_is_read)
{
int ret;
int nbytes;
+ void *bounce_buffer = qemu_blockalign(s->source->bs, s->cluster_size);
assert(QEMU_IS_ALIGNED(start, s->cluster_size));
bdrv_reset_dirty_bitmap(s->copy_bitmap, start, s->cluster_size);
nbytes = MIN(s->cluster_size, s->len - start);
- if (!*bounce_buffer) {
- *bounce_buffer = qemu_blockalign(s->source->bs, s->cluster_size);
- }
- ret = bdrv_co_pread(s->source, start, nbytes, *bounce_buffer, 0);
+ ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0);
if (ret < 0) {
trace_block_copy_with_bounce_buffer_read_fail(s, start, ret);
if (error_is_read) {
@@ -148,7 +145,7 @@ static int coroutine_fn block_copy_with_bounce_buffer(BlockCopyState *s,
goto fail;
}
- ret = bdrv_co_pwrite(s->target, start, nbytes, *bounce_buffer,
+ ret = bdrv_co_pwrite(s->target, start, nbytes, bounce_buffer,
s->write_flags);
if (ret < 0) {
trace_block_copy_with_bounce_buffer_write_fail(s, start, ret);
@@ -158,8 +155,11 @@ static int coroutine_fn block_copy_with_bounce_buffer(BlockCopyState *s,
goto fail;
}
+ qemu_vfree(bounce_buffer);
+
return nbytes;
fail:
+ qemu_vfree(bounce_buffer);
bdrv_set_dirty_bitmap(s->copy_bitmap, start, s->cluster_size);
return ret;
@@ -271,7 +271,6 @@ int coroutine_fn block_copy(BlockCopyState *s,
{
int ret = 0;
int64_t end = bytes + start; /* bytes */
- void *bounce_buffer = NULL;
int64_t status_bytes;
BlockCopyInFlightReq req;
@@ -324,7 +323,7 @@ int coroutine_fn block_copy(BlockCopyState *s,
}
if (!s->use_copy_range) {
ret = block_copy_with_bounce_buffer(s, start, dirty_end,
- error_is_read, &bounce_buffer);
+ error_is_read);
}
if (ret < 0) {
break;
@@ -335,10 +334,6 @@ int coroutine_fn block_copy(BlockCopyState *s,
ret = 0;
}
- if (bounce_buffer) {
- qemu_vfree(bounce_buffer);
- }
-
block_copy_inflight_req_end(&req);
return ret;
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 2/6] block/block-copy: limit copy_range_size to 16 MiB
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 1/6] block/block-copy: allocate buffer in block_copy_with_bounce_buffer Vladimir Sementsov-Ogievskiy
@ 2019-10-22 11:18 ` Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 3/6] block/block-copy: refactor copying Vladimir Sementsov-Ogievskiy
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-22 11:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, den, jsnow
Large copy range may imply memory allocation and large io effort, so
using 2G copy range request may be bad idea. Let's limit it to 16 MiB.
It also helps the following patch to refactor copy-with-offload
fallback to copy-with-bounce-buffer.
Note, that total memory usage of backup is still not limited, it will
be fixed in further commit.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/block-copy.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index ecd086010e..cd45b373a9 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -18,6 +18,9 @@
#include "qapi/error.h"
#include "block/block-copy.h"
#include "sysemu/block-backend.h"
+#include "qemu/units.h"
+
+#define BLOCK_COPY_MAX_COPY_RANGE (16 * MiB)
static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
int64_t start,
@@ -70,9 +73,12 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
{
BlockCopyState *s;
BdrvDirtyBitmap *copy_bitmap;
+
+ /* Ignore BLOCK_COPY_MAX_COPY_RANGE if requested cluster_size is larger */
uint32_t max_transfer =
- MIN_NON_ZERO(INT_MAX, MIN_NON_ZERO(source->bs->bl.max_transfer,
- target->bs->bl.max_transfer));
+ MIN_NON_ZERO(MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
+ MIN_NON_ZERO(source->bs->bl.max_transfer,
+ target->bs->bl.max_transfer));
copy_bitmap = bdrv_create_dirty_bitmap(source->bs, cluster_size, NULL,
errp);
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 3/6] block/block-copy: refactor copying
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 1/6] block/block-copy: allocate buffer in block_copy_with_bounce_buffer Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 2/6] block/block-copy: limit copy_range_size to 16 MiB Vladimir Sementsov-Ogievskiy
@ 2019-10-22 11:18 ` Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 4/6] util: introduce SharedResource Vladimir Sementsov-Ogievskiy
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-22 11:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, den, jsnow
Merge copying code into one function block_copy_do_copy, which only
calls bdrv_ io functions and don't do any synchronization (like dirty
bitmap set/reset).
Refactor block_copy() function so that it takes full decision about
size of chunk to be copied and does all the synchronization (checking
intersecting requests, set/reset dirty bitmaps).
It will help:
- introduce parallel processing of block_copy iterations: we need to
calculate chunk size, start async chunk copying and go to the next
iteration
- simplify synchronization improvement (like memory limiting in
further commit and reducing critical section (now we lock the whole
requested range, when actually we need to lock only dirty region
which we handle at the moment))
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
block/block-copy.c | 118 ++++++++++++++++++++-------------------------
block/trace-events | 6 +--
2 files changed, 54 insertions(+), 70 deletions(-)
diff --git a/block/block-copy.c b/block/block-copy.c
index cd45b373a9..845b2423dc 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -126,79 +126,64 @@ void block_copy_set_callbacks(
}
/*
- * Copy range to target with a bounce buffer and return the bytes copied. If
- * error occurred, return a negative error number
+ * block_copy_do_copy
+ *
+ * Do copy of cluser-aligned chunk. @end is allowed to exceed s->len only to
+ * cover last cluster when s->len is not aligned to clusters.
+ *
+ * No sync here: nor bitmap neighter intersecting requests handling, only copy.
+ *
+ * Returns 0 on success.
*/
-static int coroutine_fn block_copy_with_bounce_buffer(BlockCopyState *s,
- int64_t start,
- int64_t end,
- bool *error_is_read)
+static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
+ int64_t start, int64_t end,
+ bool *error_is_read)
{
int ret;
- int nbytes;
- void *bounce_buffer = qemu_blockalign(s->source->bs, s->cluster_size);
+ int nbytes = MIN(end, s->len) - start;
+ void *bounce_buffer = NULL;
assert(QEMU_IS_ALIGNED(start, s->cluster_size));
- bdrv_reset_dirty_bitmap(s->copy_bitmap, start, s->cluster_size);
- nbytes = MIN(s->cluster_size, s->len - start);
+ assert(QEMU_IS_ALIGNED(end, s->cluster_size));
+ assert(end < s->len || end == QEMU_ALIGN_UP(s->len, s->cluster_size));
+
+ if (s->use_copy_range) {
+ ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
+ 0, s->write_flags);
+ if (ret < 0) {
+ trace_block_copy_copy_range_fail(s, start, ret);
+ s->use_copy_range = false;
+ /* Fallback to read+write with allocated buffer */
+ } else {
+ goto out;
+ }
+ }
+
+ bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0);
if (ret < 0) {
- trace_block_copy_with_bounce_buffer_read_fail(s, start, ret);
+ trace_block_copy_read_fail(s, start, ret);
if (error_is_read) {
*error_is_read = true;
}
- goto fail;
+ goto out;
}
ret = bdrv_co_pwrite(s->target, start, nbytes, bounce_buffer,
s->write_flags);
if (ret < 0) {
- trace_block_copy_with_bounce_buffer_write_fail(s, start, ret);
+ trace_block_copy_write_fail(s, start, ret);
if (error_is_read) {
*error_is_read = false;
}
- goto fail;
+ goto out;
}
+out:
qemu_vfree(bounce_buffer);
- return nbytes;
-fail:
- qemu_vfree(bounce_buffer);
- bdrv_set_dirty_bitmap(s->copy_bitmap, start, s->cluster_size);
return ret;
-
-}
-
-/*
- * Copy range to target and return the bytes copied. If error occurred, return a
- * negative error number.
- */
-static int coroutine_fn block_copy_with_offload(BlockCopyState *s,
- int64_t start,
- int64_t end)
-{
- int ret;
- int nr_clusters;
- int nbytes;
-
- assert(QEMU_IS_ALIGNED(s->copy_range_size, s->cluster_size));
- assert(QEMU_IS_ALIGNED(start, s->cluster_size));
- nbytes = MIN(s->copy_range_size, MIN(end, s->len) - start);
- nr_clusters = DIV_ROUND_UP(nbytes, s->cluster_size);
- bdrv_reset_dirty_bitmap(s->copy_bitmap, start,
- s->cluster_size * nr_clusters);
- ret = bdrv_co_copy_range(s->source, start, s->target, start, nbytes,
- 0, s->write_flags);
- if (ret < 0) {
- trace_block_copy_with_offload_fail(s, start, ret);
- bdrv_set_dirty_bitmap(s->copy_bitmap, start,
- s->cluster_size * nr_clusters);
- return ret;
- }
-
- return nbytes;
}
/*
@@ -294,7 +279,7 @@ int coroutine_fn block_copy(BlockCopyState *s,
block_copy_inflight_req_begin(s, &req, start, end);
while (start < end) {
- int64_t dirty_end;
+ int64_t next_zero, chunk_end;
if (!bdrv_dirty_bitmap_get(s->copy_bitmap, start)) {
trace_block_copy_skip(s, start);
@@ -302,10 +287,15 @@ int coroutine_fn block_copy(BlockCopyState *s,
continue; /* already copied */
}
- dirty_end = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
- (end - start));
- if (dirty_end < 0) {
- dirty_end = end;
+ chunk_end = MIN(end, start + (s->use_copy_range ?
+ s->copy_range_size : s->cluster_size));
+
+ next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
+ chunk_end - start);
+ if (next_zero >= 0) {
+ assert(next_zero > start); /* start is dirty */
+ assert(next_zero < chunk_end); /* no need to do MIN() */
+ chunk_end = next_zero;
}
if (s->skip_unallocated) {
@@ -316,27 +306,21 @@ int coroutine_fn block_copy(BlockCopyState *s,
continue;
}
/* Clamp to known allocated region */
- dirty_end = MIN(dirty_end, start + status_bytes);
+ chunk_end = MIN(chunk_end, start + status_bytes);
}
trace_block_copy_process(s, start);
- if (s->use_copy_range) {
- ret = block_copy_with_offload(s, start, dirty_end);
- if (ret < 0) {
- s->use_copy_range = false;
- }
- }
- if (!s->use_copy_range) {
- ret = block_copy_with_bounce_buffer(s, start, dirty_end,
- error_is_read);
- }
+ bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
+
+ ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
if (ret < 0) {
+ bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
break;
}
- start += ret;
- s->progress_bytes_callback(ret, s->progress_opaque);
+ s->progress_bytes_callback(chunk_end - start, s->progress_opaque);
+ start = chunk_end;
ret = 0;
}
diff --git a/block/trace-events b/block/trace-events
index b8d70f5242..ccde15a14c 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -45,9 +45,9 @@ backup_do_cow_return(void *job, int64_t offset, uint64_t bytes, int ret) "job %p
block_copy_skip(void *bcs, int64_t start) "bcs %p start %"PRId64
block_copy_skip_range(void *bcs, int64_t start, uint64_t bytes) "bcs %p start %"PRId64" bytes %"PRId64
block_copy_process(void *bcs, int64_t start) "bcs %p start %"PRId64
-block_copy_with_bounce_buffer_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
-block_copy_with_bounce_buffer_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
-block_copy_with_offload_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_copy_range_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_read_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
+block_copy_write_fail(void *bcs, int64_t start, int ret) "bcs %p start %"PRId64" ret %d"
# ../blockdev.c
qmp_block_job_cancel(void *job) "job %p"
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 4/6] util: introduce SharedResource
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
` (2 preceding siblings ...)
2019-10-22 11:18 ` [PATCH v3 3/6] block/block-copy: refactor copying Vladimir Sementsov-Ogievskiy
@ 2019-10-22 11:18 ` Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 5/6] block/block-copy: add memory limit Vladimir Sementsov-Ogievskiy
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-22 11:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, den, jsnow
Introduce an API for some shared splittable resource, like memory.
It's going to be used by backup. Backup uses both read/write io and
copy_range. copy_range may consume memory implictly, so the new API is
abstract: it doesn't allocate any real memory but only hands out
tickets.
The idea is that we have some total amount of something and callers
should wait in coroutine queue if there is not enough of the resource
at the moment.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/qemu/co-shared-resource.h | 71 +++++++++++++++++++++++++++++
util/qemu-co-shared-resource.c | 76 +++++++++++++++++++++++++++++++
util/Makefile.objs | 1 +
3 files changed, 148 insertions(+)
create mode 100644 include/qemu/co-shared-resource.h
create mode 100644 util/qemu-co-shared-resource.c
diff --git a/include/qemu/co-shared-resource.h b/include/qemu/co-shared-resource.h
new file mode 100644
index 0000000000..4e4503004c
--- /dev/null
+++ b/include/qemu/co-shared-resource.h
@@ -0,0 +1,71 @@
+/*
+ * Helper functionality for distributing a fixed total amount of
+ * an abstract resource among multiple coroutines.
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef QEMU_CO_SHARED_RESOURCE_H
+#define QEMU_CO_SHARED_RESOURCE_H
+
+
+typedef struct SharedResource SharedResource;
+
+/*
+ * Create SharedResource structure
+ *
+ * @total: total amount of some resource to be shared between clients
+ *
+ * Note: this API is not thread-safe.
+ */
+SharedResource *shres_create(uint64_t total);
+
+/*
+ * Release SharedResource structure
+ *
+ * This function may only be called once everything allocated by all
+ * clients has been deallocated.
+ */
+void shres_destroy(SharedResource *s);
+
+/*
+ * Try to allocate an amount of @n. Return true on success, and false
+ * if there is too little left of the collective resource to fulfill
+ * the request.
+ */
+bool co_try_get_from_shres(SharedResource *s, uint64_t n);
+
+/*
+ * Allocate an amount of @n, and, if necessary, yield until
+ * that becomes possible.
+ */
+void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n);
+
+/*
+ * Deallocate an amount of @n. The total amount allocated by a caller
+ * does not need to be deallocated/released with a single call, but may
+ * be split over several calls. For example, get(4), get(3), and then
+ * put(5), put(2).
+ */
+void coroutine_fn co_put_to_shres(SharedResource *s, uint64_t n);
+
+
+#endif /* QEMU_CO_SHARED_RESOURCE_H */
diff --git a/util/qemu-co-shared-resource.c b/util/qemu-co-shared-resource.c
new file mode 100644
index 0000000000..1c83cd9d29
--- /dev/null
+++ b/util/qemu-co-shared-resource.c
@@ -0,0 +1,76 @@
+/*
+ * Helper functionality for distributing a fixed total amount of
+ * an abstract resource among multiple coroutines.
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/coroutine.h"
+#include "qemu/co-shared-resource.h"
+
+struct SharedResource {
+ uint64_t total;
+ uint64_t available;
+
+ CoQueue queue;
+};
+
+SharedResource *shres_create(uint64_t total)
+{
+ SharedResource *s = g_new0(SharedResource, 1);
+
+ s->total = s->available = total;
+ qemu_co_queue_init(&s->queue);
+
+ return s;
+}
+
+void shres_destroy(SharedResource *s)
+{
+ assert(s->available == s->total);
+ g_free(s);
+}
+
+bool co_try_get_from_shres(SharedResource *s, uint64_t n)
+{
+ if (s->available >= n) {
+ s->available -= n;
+ return true;
+ }
+
+ return false;
+}
+
+void coroutine_fn co_get_from_shres(SharedResource *s, uint64_t n)
+{
+ assert(n <= s->total);
+ while (!co_try_get_from_shres(s, n)) {
+ qemu_co_queue_wait(&s->queue, NULL);
+ }
+}
+
+void coroutine_fn co_put_to_shres(SharedResource *s, uint64_t n)
+{
+ assert(s->total - s->available >= n);
+ s->available += n;
+ qemu_co_queue_restart_all(&s->queue);
+}
diff --git a/util/Makefile.objs b/util/Makefile.objs
index 41bf59d127..df124af1c5 100644
--- a/util/Makefile.objs
+++ b/util/Makefile.objs
@@ -37,6 +37,7 @@ util-obj-y += rcu.o
util-obj-$(CONFIG_MEMBARRIER) += sys_membarrier.o
util-obj-y += qemu-coroutine.o qemu-coroutine-lock.o qemu-coroutine-io.o
util-obj-y += qemu-coroutine-sleep.o
+util-obj-y += qemu-co-shared-resource.o
util-obj-y += coroutine-$(CONFIG_COROUTINE_BACKEND).o
util-obj-y += buffer.o
util-obj-y += timed-average.o
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 5/6] block/block-copy: add memory limit
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
` (3 preceding siblings ...)
2019-10-22 11:18 ` [PATCH v3 4/6] util: introduce SharedResource Vladimir Sementsov-Ogievskiy
@ 2019-10-22 11:18 ` Vladimir Sementsov-Ogievskiy
2019-10-22 11:18 ` [PATCH v3 6/6] block/block-copy: increase buffered copy request Vladimir Sementsov-Ogievskiy
2019-10-25 12:31 ` [PATCH v3 0/6] block-copy: memory limit Max Reitz
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-22 11:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, den, jsnow
Currently total allocation for parallel requests to block-copy instance
is unlimited. Let's limit it to 128 MiB.
For now block-copy is used only in backup, so actually we limit total
allocation for backup job.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/block-copy.h | 3 +++
block/block-copy.c | 5 +++++
2 files changed, 8 insertions(+)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index e2e135ff1b..edcdf0072d 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -16,6 +16,7 @@
#define BLOCK_COPY_H
#include "block/block.h"
+#include "qemu/co-shared-resource.h"
typedef struct BlockCopyInFlightReq {
int64_t start_byte;
@@ -69,6 +70,8 @@ typedef struct BlockCopyState {
*/
ProgressResetCallbackFunc progress_reset_callback;
void *progress_opaque;
+
+ SharedResource *mem;
} BlockCopyState;
BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
diff --git a/block/block-copy.c b/block/block-copy.c
index 845b2423dc..7f0ebb58f8 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -21,6 +21,7 @@
#include "qemu/units.h"
#define BLOCK_COPY_MAX_COPY_RANGE (16 * MiB)
+#define BLOCK_COPY_MAX_MEM (128 * MiB)
static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
int64_t start,
@@ -64,6 +65,7 @@ void block_copy_state_free(BlockCopyState *s)
}
bdrv_release_dirty_bitmap(s->copy_bitmap);
+ shres_destroy(s->mem);
g_free(s);
}
@@ -95,6 +97,7 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
.cluster_size = cluster_size,
.len = bdrv_dirty_bitmap_size(copy_bitmap),
.write_flags = write_flags,
+ .mem = shres_create(BLOCK_COPY_MAX_MEM),
};
s->copy_range_size = QEMU_ALIGN_DOWN(max_transfer, cluster_size),
@@ -313,7 +316,9 @@ int coroutine_fn block_copy(BlockCopyState *s,
bdrv_reset_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
+ co_get_from_shres(s->mem, chunk_end - start);
ret = block_copy_do_copy(s, start, chunk_end, error_is_read);
+ co_put_to_shres(s->mem, chunk_end - start);
if (ret < 0) {
bdrv_set_dirty_bitmap(s->copy_bitmap, start, chunk_end - start);
break;
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 6/6] block/block-copy: increase buffered copy request
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
` (4 preceding siblings ...)
2019-10-22 11:18 ` [PATCH v3 5/6] block/block-copy: add memory limit Vladimir Sementsov-Ogievskiy
@ 2019-10-22 11:18 ` Vladimir Sementsov-Ogievskiy
2019-10-25 12:31 ` [PATCH v3 0/6] block-copy: memory limit Max Reitz
6 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-22 11:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, vsementsov, qemu-devel, mreitz, den, jsnow
No reason to limit buffered copy to one cluster. Let's allow up to 1
MiB.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
include/block/block-copy.h | 2 +-
block/block-copy.c | 48 +++++++++++++++++++++++++-------------
2 files changed, 33 insertions(+), 17 deletions(-)
diff --git a/include/block/block-copy.h b/include/block/block-copy.h
index edcdf0072d..0a161724d7 100644
--- a/include/block/block-copy.h
+++ b/include/block/block-copy.h
@@ -38,7 +38,7 @@ typedef struct BlockCopyState {
BdrvDirtyBitmap *copy_bitmap;
int64_t cluster_size;
bool use_copy_range;
- int64_t copy_range_size;
+ int64_t copy_size;
uint64_t len;
QLIST_HEAD(, BlockCopyInFlightReq) inflight_reqs;
diff --git a/block/block-copy.c b/block/block-copy.c
index 7f0ebb58f8..c39cc9cffe 100644
--- a/block/block-copy.c
+++ b/block/block-copy.c
@@ -21,6 +21,7 @@
#include "qemu/units.h"
#define BLOCK_COPY_MAX_COPY_RANGE (16 * MiB)
+#define BLOCK_COPY_MAX_BUFFER (1 * MiB)
#define BLOCK_COPY_MAX_MEM (128 * MiB)
static void coroutine_fn block_copy_wait_inflight_reqs(BlockCopyState *s,
@@ -75,10 +76,8 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
{
BlockCopyState *s;
BdrvDirtyBitmap *copy_bitmap;
-
- /* Ignore BLOCK_COPY_MAX_COPY_RANGE if requested cluster_size is larger */
uint32_t max_transfer =
- MIN_NON_ZERO(MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
+ MIN_NON_ZERO(INT_MAX,
MIN_NON_ZERO(source->bs->bl.max_transfer,
target->bs->bl.max_transfer));
@@ -100,17 +99,28 @@ BlockCopyState *block_copy_state_new(BdrvChild *source, BdrvChild *target,
.mem = shres_create(BLOCK_COPY_MAX_MEM),
};
- s->copy_range_size = QEMU_ALIGN_DOWN(max_transfer, cluster_size),
- /*
- * Set use_copy_range, consider the following:
- * 1. Compression is not supported for copy_range.
- * 2. copy_range does not respect max_transfer (it's a TODO), so we factor
- * that in here. If max_transfer is smaller than the job->cluster_size,
- * we do not use copy_range (in that case it's zero after aligning down
- * above).
- */
- s->use_copy_range =
- !(write_flags & BDRV_REQ_WRITE_COMPRESSED) && s->copy_range_size > 0;
+ if (max_transfer < cluster_size) {
+ /*
+ * copy_range does not respect max_transfer. We don't want to bother
+ * with requests smaller than block-copy cluster size, so fallback to
+ * buffered copying (read and write respect max_transfer on their
+ * behalf).
+ */
+ s->use_copy_range = false;
+ s->copy_size = cluster_size;
+ } else if (write_flags & BDRV_REQ_WRITE_COMPRESSED) {
+ /* Compression is not supported for copy_range */
+ s->use_copy_range = false;
+ s->copy_size = MAX(cluster_size, BLOCK_COPY_MAX_BUFFER);
+ } else {
+ /*
+ * copy_range does not respect max_transfer (it's a TODO), so we factor
+ * that in here.
+ */
+ s->use_copy_range = true;
+ s->copy_size = MIN(MAX(cluster_size, BLOCK_COPY_MAX_COPY_RANGE),
+ QEMU_ALIGN_DOWN(max_transfer, cluster_size));
+ }
QLIST_INIT(&s->inflight_reqs);
@@ -156,12 +166,19 @@ static int coroutine_fn block_copy_do_copy(BlockCopyState *s,
if (ret < 0) {
trace_block_copy_copy_range_fail(s, start, ret);
s->use_copy_range = false;
+ s->copy_size = MAX(s->cluster_size, BLOCK_COPY_MAX_BUFFER);
/* Fallback to read+write with allocated buffer */
} else {
goto out;
}
}
+ /*
+ * In case of failed copy_range request above, we may proceed with buffered
+ * request larger than BLOCK_COPY_MAX_BUFFER. Still, further requests will
+ * be properly limited, so don't care too much.
+ */
+
bounce_buffer = qemu_blockalign(s->source->bs, nbytes);
ret = bdrv_co_pread(s->source, start, nbytes, bounce_buffer, 0);
@@ -290,8 +307,7 @@ int coroutine_fn block_copy(BlockCopyState *s,
continue; /* already copied */
}
- chunk_end = MIN(end, start + (s->use_copy_range ?
- s->copy_range_size : s->cluster_size));
+ chunk_end = MIN(end, start + s->copy_size);
next_zero = bdrv_dirty_bitmap_next_zero(s->copy_bitmap, start,
chunk_end - start);
--
2.21.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/6] block-copy: memory limit
2019-10-22 11:17 [PATCH v3 0/6] block-copy: memory limit Vladimir Sementsov-Ogievskiy
` (5 preceding siblings ...)
2019-10-22 11:18 ` [PATCH v3 6/6] block/block-copy: increase buffered copy request Vladimir Sementsov-Ogievskiy
@ 2019-10-25 12:31 ` Max Reitz
2019-10-25 12:32 ` Vladimir Sementsov-Ogievskiy
6 siblings, 1 reply; 9+ messages in thread
From: Max Reitz @ 2019-10-25 12:31 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, den, jsnow, qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1724 bytes --]
On 22.10.19 13:17, Vladimir Sementsov-Ogievskiy wrote:
> I'm going to bring block-status driven, async copying process to
> block-copy, to make it fast. The first step is to limit memory usage of
> backup, here is it.
>
> v3:
> 03: add Max's r-b
> 04: fix commit message and include guards, add Max's r-b
> 05-06: add Max's r-b
>
> v2: [mostly by Max's comments]
> Now based on master (Thank you Max!)
> 01: add Max's r-b
> 02: add Max's r-b
> 03: - refactor block_copy_do_copy goto/return
> - add small comment to block_copy_do_copy
> 04: - a lot of renaming and wording fixes
> - refactor to use "available" instead of "taken"
> - refactor co_get_from_shres
> 05: rebase on 04 changes
> 06: drop extra things from max_transfer calculation
>
> Vladimir Sementsov-Ogievskiy (6):
> block/block-copy: allocate buffer in block_copy_with_bounce_buffer
> block/block-copy: limit copy_range_size to 16 MiB
> block/block-copy: refactor copying
> util: introduce SharedResource
> block/block-copy: add memory limit
> block/block-copy: increase buffered copy request
>
> include/block/block-copy.h | 5 +-
> include/qemu/co-shared-resource.h | 71 ++++++++++++
> block/block-copy.c | 182 +++++++++++++++---------------
> util/qemu-co-shared-resource.c | 76 +++++++++++++
> block/trace-events | 6 +-
> util/Makefile.objs | 1 +
> 6 files changed, 249 insertions(+), 92 deletions(-)
> create mode 100644 include/qemu/co-shared-resource.h
> create mode 100644 util/qemu-co-shared-resource.c
Thanks, applied to my block branch:
https://git.xanclic.moe/XanClic/qemu/commits/branch/block
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 0/6] block-copy: memory limit
2019-10-25 12:31 ` [PATCH v3 0/6] block-copy: memory limit Max Reitz
@ 2019-10-25 12:32 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 9+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-10-25 12:32 UTC (permalink / raw)
To: Max Reitz, qemu-block@nongnu.org
Cc: kwolf@redhat.com, jsnow@redhat.com, qemu-devel@nongnu.org,
Denis Lunev
25.10.2019 15:31, Max Reitz wrote:
> On 22.10.19 13:17, Vladimir Sementsov-Ogievskiy wrote:
>> I'm going to bring block-status driven, async copying process to
>> block-copy, to make it fast. The first step is to limit memory usage of
>> backup, here is it.
>>
>> v3:
>> 03: add Max's r-b
>> 04: fix commit message and include guards, add Max's r-b
>> 05-06: add Max's r-b
>>
>> v2: [mostly by Max's comments]
>> Now based on master (Thank you Max!)
>> 01: add Max's r-b
>> 02: add Max's r-b
>> 03: - refactor block_copy_do_copy goto/return
>> - add small comment to block_copy_do_copy
>> 04: - a lot of renaming and wording fixes
>> - refactor to use "available" instead of "taken"
>> - refactor co_get_from_shres
>> 05: rebase on 04 changes
>> 06: drop extra things from max_transfer calculation
>>
>> Vladimir Sementsov-Ogievskiy (6):
>> block/block-copy: allocate buffer in block_copy_with_bounce_buffer
>> block/block-copy: limit copy_range_size to 16 MiB
>> block/block-copy: refactor copying
>> util: introduce SharedResource
>> block/block-copy: add memory limit
>> block/block-copy: increase buffered copy request
>>
>> include/block/block-copy.h | 5 +-
>> include/qemu/co-shared-resource.h | 71 ++++++++++++
>> block/block-copy.c | 182 +++++++++++++++---------------
>> util/qemu-co-shared-resource.c | 76 +++++++++++++
>> block/trace-events | 6 +-
>> util/Makefile.objs | 1 +
>> 6 files changed, 249 insertions(+), 92 deletions(-)
>> create mode 100644 include/qemu/co-shared-resource.h
>> create mode 100644 util/qemu-co-shared-resource.c
>
> Thanks, applied to my block branch:
>
> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
>
> Max
>
Thank you Max!
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 9+ messages in thread