[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/9] block/write-threshold: don't use write notifiers
From: |
Max Reitz |
Subject: |
Re: [PATCH v2 1/9] block/write-threshold: don't use write notifiers |
Date: |
Wed, 5 May 2021 14:37:51 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 |
On 04.05.21 10:25, Vladimir Sementsov-Ogievskiy wrote:
write-notifiers are used only for write-threshold. New code for such
purpose should create filters.
Let's better special-case write-threshold and drop write notifiers at
all. (Actually, write-threshold is special-cased anyway, as the only
user of write-notifiers)
Not noted here: That write-threshold.c is also reorganized. (Doesn’t
seem entirely necessary to do right in this patch, but why not.)
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
include/block/block_int.h | 1 -
include/block/write-threshold.h | 9 +++++
block/io.c | 5 ++-
block/write-threshold.c | 68 +++++++--------------------------
4 files changed, 25 insertions(+), 58 deletions(-)
[...]
diff --git a/include/block/write-threshold.h b/include/block/write-threshold.h
index c646f267a4..7942dcc368 100644
--- a/include/block/write-threshold.h
+++ b/include/block/write-threshold.h
@@ -59,4 +59,13 @@ bool bdrv_write_threshold_is_set(const BlockDriverState *bs);
uint64_t bdrv_write_threshold_exceeded(const BlockDriverState *bs,
const BdrvTrackedRequest *req);
+/*
+ * bdrv_write_threshold_check_write
+ *
+ * Check, does specified request exceeds write threshold. If it is, send
I’d prefer either “Check: Does the specified request exceed the write
threshold?” or “Check whether the specified request exceeds the write
threshold.”
+ * corresponding event and unset write threshold.
I’d call it “disable write threshold checking” instead of “unset write
threshold”, because I don’t it immediately clear what unsetting the
threshold means.
+ */
+void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
+ int64_t bytes);
+
#endif
[...]
@@ -122,3 +68,15 @@ void qmp_block_set_write_threshold(const char *node_name,
aio_context_release(aio_context);
}
+
+void bdrv_write_threshold_check_write(BlockDriverState *bs, int64_t offset,
+ int64_t bytes)
+{
+ int64_t end = offset + bytes;
+ uint64_t wtr = bs->write_threshold_offset;
+
+ if (wtr > 0 && end > wtr) {
+ qapi_event_send_block_write_threshold(bs->node_name, end - wtr, wtr);
OK, don’t understand why bdrv_write_threshold_exceeded() had two cases
(one for offset > wtr, one for end > wtr). Perhaps overflow
considerations? Shouldn’t matter though.
+ bdrv_write_threshold_set(bs, 0);
I’d keep the comment from before_write_notify() here (i.e. “autodisable
to avoid flooding the monitor”).
But other than that, I have no complaints:
Reviewed-by: Max Reitz <mreitz@redhat.com>
+ }
+}