[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] block: simplify write-threshold and drop write notifiers
From: |
Max Reitz |
Subject: |
Re: [PATCH] block: simplify write-threshold and drop write notifiers |
Date: |
Fri, 30 Apr 2021 12:04:15 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 22.04.21 00:09, Vladimir Sementsov-Ogievskiy wrote:
write-notifiers are used only for write-threshold. New code for such
purpose should create filters.
Let's handle write-threshold simply in generic code and drop write
notifiers at all.
Also move part of write-threshold API that is used only for testing to
the test.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
I agree that this could be split into 2-3 parts and not combining
everything into one. But I'm tired now.
Er... You should have put it off until the next day then? O:)
It should be multiple patches. At least one to move the write threshold
update to block/io.c, and then another to drop write notifiers.
I can send v2 if needed, so
consider it as RFC. Or take as is if you think it's not too much-in-one.
I also suggest this as a prepartion (and partly substitution) for
"[PATCH v2 0/8] Block layer thread-safety, continued"
include/block/block_int.h | 12 -----
include/block/write-threshold.h | 24 ---------
block.c | 1 -
block/io.c | 21 +++++---
block/write-threshold.c | 87 ++-----------------------------
tests/unit/test-write-threshold.c | 38 ++++++++++++++
6 files changed, 54 insertions(+), 129 deletions(-)
[...]
diff --git a/block/io.c b/block/io.c
index ca2dca3007..e0aa775952 100644
--- a/block/io.c
+++ b/block/io.c
@@ -36,6 +36,8 @@
#include "qemu/main-loop.h"
#include "sysemu/replay.h"
+#include "qapi/qapi-events-block-core.h"
+
/* Maximum bounce buffer for copy-on-read and write zeroes, in bytes */
#define MAX_BOUNCE_BUFFER (32768 << BDRV_SECTOR_BITS)
@@ -1974,6 +1976,8 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t offset, int64_t bytes,
child->perm & BLK_PERM_RESIZE);
switch (req->type) {
+ uint64_t write_threshold;
+
Works, but I think this is the first time I see a variable declared in a
switch block. What I usually do for such cases is to put a block after
the label. (i.e. case X: { uint64_t write_threshold; ... })
But it wouldn’t hurt to just declare this at the beginning of
bdrv_co_write_req_prepare(), I think.
case BDRV_TRACKED_WRITE:
case BDRV_TRACKED_DISCARD:
if (flags & BDRV_REQ_WRITE_UNCHANGED) {
@@ -1981,8 +1985,15 @@ bdrv_co_write_req_prepare(BdrvChild *child, int64_t
offset, int64_t bytes,
} else {
assert(child->perm & BLK_PERM_WRITE);
}
- return notifier_with_return_list_notify(&bs->before_write_notifiers,
- req);
+ write_threshold = qatomic_read(&bs->write_threshold_offset);
+ if (write_threshold > 0 && offset + bytes > write_threshold) {
+ qapi_event_send_block_write_threshold(
+ bs->node_name,
+ offset + bytes - write_threshold,
+ write_threshold);
+ qatomic_set(&bs->write_threshold_offset, 0);
+ }
I’d put all of this into a function in block/write-threshold.c that’s
called from here.
Max
+ return 0;
case BDRV_TRACKED_TRUNCATE:
assert(child->perm & BLK_PERM_RESIZE);
return 0;
@@ -3137,12 +3148,6 @@ bool bdrv_qiov_is_aligned(BlockDriverState *bs,
QEMUIOVector *qiov)
return true;
}
-void bdrv_add_before_write_notifier(BlockDriverState *bs,
- NotifierWithReturn *notifier)
-{
- notifier_with_return_list_add(&bs->before_write_notifiers, notifier);
-}
-
void bdrv_io_plug(BlockDriverState *bs)
{
BdrvChild *child;