Re: [PATCH 5/5] block/io: auto-no-fallback for write-zeroes
14.03.2020 0:56, Eric Blake wrote:
On 3/2/20 4:05 AM, Vladimir Sementsov-Ogievskiy wrote:
NBD driver may has max_pwrite_zeroes but doesn't has
max_pwrite_zeroes_no_fallback limit. This means, that (when
BDRV_REQ_NO_FALLBACK is supported) it is beneficial to try send request
with BDRV_REQ_NO_FALLBACK instead of splitting the request accordingly
to max_pwrite_zeroes.
If failed, fallback to old behavior.
Grammar:
When BDRV_REQ_NO_FALLBACK is supported, the NBD driver supports a larger
request size. Add code to try large zero requests with a NO_FALLBACK request
prior to having to split a request into chunks according to max_pwrite_zeroes.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index c64566b4cf..48d71b0883 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1752,17 +1752,28 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int head = 0;
int tail = 0;
- int max_write_zeroes = MIN_NON_ZERO((flags & BDRV_REQ_NO_FALLBACK) ?
- bs->bl.max_pwrite_zeroes_no_fallback :
- bs->bl.max_pwrite_zeroes, INT_MAX);
+ int max_write_zeroes;
int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
bs->bl.request_alignment);
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
+ bool auto_no_fallback;
if (!drv) {
return -ENOMEDIUM;
}
+ if (!(flags & BDRV_REQ_NO_FALLBACK) &&
+ (bs->supported_zero_flags & BDRV_REQ_NO_FALLBACK) &&
+ bs->bl.max_pwrite_zeroes &&
+ bs->bl.max_pwrite_zeroes < bytes &&
+ (bs->bl.max_pwrite_zeroes < bs->bl.max_pwrite_zeroes_no_fallback ||
+ bs->bl.max_pwrite_zeroes_no_fallback == 0))
Why are we letting max_pwrite_zeroes_no_fallback ever be 0? It might be more
convenient if it is always guaranteed to be >= max_pwrite_zeroes by the block
layer.
All other limits may be 0 too, which means some default.. So, if we want to set
the default for max_pwrite_zeroes_no_fallback explicitly, I think we should do
it for all other limits too. And it should be separate thing..
+ {
+ assert(drv->bdrv_co_pwrite_zeroes);
+ flags |= BDRV_REQ_NO_FALLBACK;
+ auto_no_fallback = true;
+ }
+
if ((flags & ~bs->supported_zero_flags) & BDRV_REQ_NO_FALLBACK) {
return -ENOTSUP;
}
@@ -1770,7 +1781,11 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
assert(alignment % bs->bl.request_alignment == 0);
head = offset % alignment;
tail = (offset + bytes) % alignment;
- max_write_zeroes = QEMU_ALIGN_DOWN(max_write_zeroes, alignment);
+ max_write_zeroes =
+ QEMU_ALIGN_DOWN(MIN_NON_ZERO((flags & BDRV_REQ_NO_FALLBACK) ?
+ bs->bl.max_pwrite_zeroes_no_fallback :
+ bs->bl.max_pwrite_zeroes, INT_MAX),
+ alignment);
assert(max_write_zeroes >= bs->bl.request_alignment);
while (bytes > 0 && !ret) {
@@ -1801,6 +1816,13 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
if (drv->bdrv_co_pwrite_zeroes) {
ret = drv->bdrv_co_pwrite_zeroes(bs, offset, num,
flags &
bs->supported_zero_flags);
+ if (ret == -ENOTSUP && auto_no_fallback) {
+ flags &= ~BDRV_REQ_NO_FALLBACK;
+ max_write_zeroes =
+ QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pwrite_zeroes,
+ INT_MAX), alignment);
+ continue;
+ }
if (ret != -ENOTSUP && (flags & BDRV_REQ_FUA) &&
!(bs->supported_zero_flags & BDRV_REQ_FUA)) {
need_flush = true;
Otherwise makes sense.
--
Best regards,
Vladimir
Re: [PATCH 5/5] block/io: auto-no-fallback for write-zeroes
On 3/2/20 4:05 AM, Vladimir Sementsov-Ogievskiy wrote:
NBD driver may has max_pwrite_zeroes but doesn't has
max_pwrite_zeroes_no_fallback limit. This means, that (when
BDRV_REQ_NO_FALLBACK is supported) it is beneficial to try send request
with BDRV_REQ_NO_FALLBACK instead of splitting the request accordingly
to max_pwrite_zeroes.
If failed, fallback to old behavior.
Grammar:
When BDRV_REQ_NO_FALLBACK is supported, the NBD driver supports a larger
request size. Add code to try large zero requests with a NO_FALLBACK
request prior to having to split a request into chunks according to
max_pwrite_zeroes.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index c64566b4cf..48d71b0883 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1752,17 +1752,28 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int head = 0;
int tail = 0;
-int max_write_zeroes = MIN_NON_ZERO((flags & BDRV_REQ_NO_FALLBACK) ?
-bs->bl.max_pwrite_zeroes_no_fallback :
-bs->bl.max_pwrite_zeroes, INT_MAX);
+int max_write_zeroes;
int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
bs->bl.request_alignment);
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
+bool auto_no_fallback;
if (!drv) {
return -ENOMEDIUM;
}
+if (!(flags & BDRV_REQ_NO_FALLBACK) &&
+(bs->supported_zero_flags & BDRV_REQ_NO_FALLBACK) &&
+bs->bl.max_pwrite_zeroes &&
+bs->bl.max_pwrite_zeroes < bytes &&
+(bs->bl.max_pwrite_zeroes < bs->bl.max_pwrite_zeroes_no_fallback ||
+ bs->bl.max_pwrite_zeroes_no_fallback == 0))
Why are we letting max_pwrite_zeroes_no_fallback ever be 0? It might be
more convenient if it is always guaranteed to be >= max_pwrite_zeroes by
the block layer.
+{
+assert(drv->bdrv_co_pwrite_zeroes);
+flags |= BDRV_REQ_NO_FALLBACK;
+auto_no_fallback = true;
+}
+
if ((flags & ~bs->supported_zero_flags) & BDRV_REQ_NO_FALLBACK) {
return -ENOTSUP;
}
@@ -1770,7 +1781,11 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
assert(alignment % bs->bl.request_alignment == 0);
head = offset % alignment;
tail = (offset + bytes) % alignment;
-max_write_zeroes = QEMU_ALIGN_DOWN(max_write_zeroes, alignment);
+max_write_zeroes =
+QEMU_ALIGN_DOWN(MIN_NON_ZERO((flags & BDRV_REQ_NO_FALLBACK) ?
+ bs->bl.max_pwrite_zeroes_no_fallback :
+ bs->bl.max_pwrite_zeroes, INT_MAX),
+alignment);
assert(max_write_zeroes >= bs->bl.request_alignment);
while (bytes > 0 && !ret) {
@@ -1801,6 +1816,13 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
if (drv->bdrv_co_pwrite_zeroes) {
ret = drv->bdrv_co_pwrite_zeroes(bs, offset, num,
flags &
bs->supported_zero_flags);
+if (ret == -ENOTSUP && auto_no_fallback) {
+flags &= ~BDRV_REQ_NO_FALLBACK;
+max_write_zeroes =
+QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pwrite_zeroes,
+ INT_MAX), alignment);
+continue;
+}
if (ret != -ENOTSUP && (flags & BDRV_REQ_FUA) &&
!(bs->supported_zero_flags & BDRV_REQ_FUA)) {
need_flush = true;
Otherwise makes sense.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
[PATCH 5/5] block/io: auto-no-fallback for write-zeroes
NBD driver may has max_pwrite_zeroes but doesn't has
max_pwrite_zeroes_no_fallback limit. This means, that (when
BDRV_REQ_NO_FALLBACK is supported) it is beneficial to try send request
with BDRV_REQ_NO_FALLBACK instead of splitting the request accordingly
to max_pwrite_zeroes.
If failed, fallback to old behavior.
Signed-off-by: Vladimir Sementsov-Ogievskiy
---
block/io.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index c64566b4cf..48d71b0883 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1752,17 +1752,28 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
int head = 0;
int tail = 0;
-int max_write_zeroes = MIN_NON_ZERO((flags & BDRV_REQ_NO_FALLBACK) ?
-bs->bl.max_pwrite_zeroes_no_fallback :
-bs->bl.max_pwrite_zeroes, INT_MAX);
+int max_write_zeroes;
int alignment = MAX(bs->bl.pwrite_zeroes_alignment,
bs->bl.request_alignment);
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
+bool auto_no_fallback;
if (!drv) {
return -ENOMEDIUM;
}
+if (!(flags & BDRV_REQ_NO_FALLBACK) &&
+(bs->supported_zero_flags & BDRV_REQ_NO_FALLBACK) &&
+bs->bl.max_pwrite_zeroes &&
+bs->bl.max_pwrite_zeroes < bytes &&
+(bs->bl.max_pwrite_zeroes < bs->bl.max_pwrite_zeroes_no_fallback ||
+ bs->bl.max_pwrite_zeroes_no_fallback == 0))
+{
+assert(drv->bdrv_co_pwrite_zeroes);
+flags |= BDRV_REQ_NO_FALLBACK;
+auto_no_fallback = true;
+}
+
if ((flags & ~bs->supported_zero_flags) & BDRV_REQ_NO_FALLBACK) {
return -ENOTSUP;
}
@@ -1770,7 +1781,11 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
assert(alignment % bs->bl.request_alignment == 0);
head = offset % alignment;
tail = (offset + bytes) % alignment;
-max_write_zeroes = QEMU_ALIGN_DOWN(max_write_zeroes, alignment);
+max_write_zeroes =
+QEMU_ALIGN_DOWN(MIN_NON_ZERO((flags & BDRV_REQ_NO_FALLBACK) ?
+ bs->bl.max_pwrite_zeroes_no_fallback :
+ bs->bl.max_pwrite_zeroes, INT_MAX),
+alignment);
assert(max_write_zeroes >= bs->bl.request_alignment);
while (bytes > 0 && !ret) {
@@ -1801,6 +1816,13 @@ static int coroutine_fn
bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
if (drv->bdrv_co_pwrite_zeroes) {
ret = drv->bdrv_co_pwrite_zeroes(bs, offset, num,
flags & bs->supported_zero_flags);
+if (ret == -ENOTSUP && auto_no_fallback) {
+flags &= ~BDRV_REQ_NO_FALLBACK;
+max_write_zeroes =
+QEMU_ALIGN_DOWN(MIN_NON_ZERO(bs->bl.max_pwrite_zeroes,
+ INT_MAX), alignment);
+continue;
+}
if (ret != -ENOTSUP && (flags & BDRV_REQ_FUA) &&
!(bs->supported_zero_flags & BDRV_REQ_FUA)) {
need_flush = true;
--
2.21.0
