2022-07-06 09:51:19

by Yu Kuai

[permalink] [raw]
Subject: [PATCH] dm writecache: fix inaccurate reads/writes stats

From: Yu Kuai <[email protected]>

Test procedures:

1) format a dm writecache device with 4k blocksize.
2) flush cache.
3) cache 1G data through write.
4) clear stats.
5) read 2G data with bs=1m.
6) read stats.

Expected result:
cache hit ratio is 50%.

Test result:
stats: 0, 1011345, 749201, 0, 263168, 262144, 0, 0, 0, 0, 0, 0, 0, 0
ratio is 99% (262144/263168)

The way that reads is accounted is different between cache hit and cache
miss:

1) If cache hit, reads will be accounted for each entry, which means reads
and read_hits will both increase 256 for each io in the above test.

2) If cache miss, reads will only account once, which means reads will only
increase 1 for each io in the above test.

The case that writes_around has the same problem, fix it by adding
appropriate reads/writes in writecache_map_remap_origin().

Fixes: e3a35d03407c ("dm writecache: add event counters")
Signed-off-by: Yu Kuai <[email protected]>
---
drivers/md/dm-writecache.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c
index d74c5a7a0ab4..c2c6c3a023dd 100644
--- a/drivers/md/dm-writecache.c
+++ b/drivers/md/dm-writecache.c
@@ -1329,16 +1329,29 @@ enum wc_map_op {
WC_MAP_ERROR,
};

-static enum wc_map_op writecache_map_remap_origin(struct dm_writecache *wc, struct bio *bio,
- struct wc_entry *e)
+static enum wc_map_op writecache_map_remap_origin(struct dm_writecache *wc,
+ struct bio *bio,
+ struct wc_entry *e, bool read)
{
+ sector_t next_boundary;
+ unsigned long long miss_count;
+
if (e) {
- sector_t next_boundary =
+ next_boundary =
read_original_sector(wc, e) - bio->bi_iter.bi_sector;
if (next_boundary < bio->bi_iter.bi_size >> SECTOR_SHIFT)
dm_accept_partial_bio(bio, next_boundary);
+ } else {
+ next_boundary = bio->bi_iter.bi_size;
}

+ miss_count = (round_up(next_boundary, wc->block_size) >>
+ wc->block_size_bits) - 1;
+ if (read)
+ wc->stats.reads += miss_count;
+ else
+ wc->stats.writes += miss_count;
+
return WC_MAP_REMAP_ORIGIN;
}

@@ -1366,7 +1379,7 @@ static enum wc_map_op writecache_map_read(struct dm_writecache *wc, struct bio *
map_op = WC_MAP_REMAP;
}
} else {
- map_op = writecache_map_remap_origin(wc, bio, e);
+ map_op = writecache_map_remap_origin(wc, bio, e, true);
}

return map_op;
@@ -1458,7 +1471,8 @@ static enum wc_map_op writecache_map_write(struct dm_writecache *wc, struct bio
direct_write:
wc->stats.writes_around++;
e = writecache_find_entry(wc, bio->bi_iter.bi_sector, WFE_RETURN_FOLLOWING);
- return writecache_map_remap_origin(wc, bio, e);
+ return writecache_map_remap_origin(wc, bio, e,
+ false);
}
wc->stats.writes_blocked_on_freelist++;
writecache_wait_on_freelist(wc);
--
2.31.1


2022-07-11 21:00:06

by Mikulas Patocka

[permalink] [raw]
Subject: Re: [PATCH] dm writecache: fix inaccurate reads/writes stats



On Wed, 6 Jul 2022, Yu Kuai wrote:

> From: Yu Kuai <[email protected]>
>
> Test procedures:
>
> 1) format a dm writecache device with 4k blocksize.
> 2) flush cache.
> 3) cache 1G data through write.
> 4) clear stats.
> 5) read 2G data with bs=1m.
> 6) read stats.
>
> Expected result:
> cache hit ratio is 50%.
>
> Test result:
> stats: 0, 1011345, 749201, 0, 263168, 262144, 0, 0, 0, 0, 0, 0, 0, 0
> ratio is 99% (262144/263168)

Hi

Here I'm providing patches that change the dm-writecache counting from
requests to blocks.

Mike, you can queue them for the next merge window.

Mikulas

2022-07-11 21:01:30

by Mikulas Patocka

[permalink] [raw]
Subject: [PATCH 3/4] dm-writecache: count the number of blocks written, not the number of write bios

Change dm-writecache, so that it counts the number of blocks written
instead of the number of write bios. Bios can be split and requeued using
the dm_accept_partial_bio function, so counting of bios provided
inaccurate results.

Signed-off-by: Mikulas Patocka <[email protected]>
Reported-by: Yu Kuai <[email protected]>

Index: linux-2.6/drivers/md/dm-writecache.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-writecache.c
+++ linux-2.6/drivers/md/dm-writecache.c
@@ -1413,6 +1413,9 @@ static void writecache_bio_copy_ssd(stru
bio->bi_iter.bi_sector = start_cache_sec;
dm_accept_partial_bio(bio, bio_size >> SECTOR_SHIFT);

+ wc->stats.writes += bio->bi_iter.bi_size >> wc->block_size_bits;
+ wc->stats.writes_allocate += (bio->bi_iter.bi_size - wc->block_size) >> wc->block_size_bits;
+
if (unlikely(wc->uncommitted_blocks >= wc->autocommit_blocks)) {
wc->uncommitted_blocks = 0;
queue_work(wc->writeback_wq, &wc->flush_work);
@@ -1428,9 +1431,10 @@ static enum wc_map_op writecache_map_wri
do {
bool found_entry = false;
bool search_used = false;
- wc->stats.writes++;
- if (writecache_has_error(wc))
+ if (writecache_has_error(wc)) {
+ wc->stats.writes += bio->bi_iter.bi_size >> wc->block_size_bits;
return WC_MAP_ERROR;
+ }
e = writecache_find_entry(wc, bio->bi_iter.bi_sector, 0);
if (e) {
if (!writecache_entry_is_committed(wc, e)) {
@@ -1454,9 +1458,10 @@ static enum wc_map_op writecache_map_wri
if (unlikely(!e)) {
if (!WC_MODE_PMEM(wc) && !found_entry) {
direct_write:
- wc->stats.writes_around++;
e = writecache_find_entry(wc, bio->bi_iter.bi_sector, WFE_RETURN_FOLLOWING);
writecache_map_remap_origin(wc, bio, e);
+ wc->stats.writes_around += bio->bi_iter.bi_size >> wc->block_size_bits;
+ wc->stats.writes += bio->bi_iter.bi_size >> wc->block_size_bits;
return WC_MAP_REMAP_ORIGIN;
}
wc->stats.writes_blocked_on_freelist++;
@@ -1470,6 +1475,7 @@ direct_write:
bio_copy:
if (WC_MODE_PMEM(wc)) {
bio_copy_block(wc, bio, memory_data(wc, e));
+ wc->stats.writes++;
} else {
writecache_bio_copy_ssd(wc, bio, e, search_used);
return WC_MAP_REMAP;
Index: linux-2.6/Documentation/admin-guide/device-mapper/writecache.rst
===================================================================
--- linux-2.6.orig/Documentation/admin-guide/device-mapper/writecache.rst
+++ linux-2.6/Documentation/admin-guide/device-mapper/writecache.rst
@@ -80,11 +80,11 @@ Status:
4. the number of blocks under writeback
5. the number of read blocks
6. the number of read blocks that hit the cache
-7. the number of write requests
-8. the number of write requests that hit uncommitted block
-9. the number of write requests that hit committed block
-10. the number of write requests that bypass the cache
-11. the number of write requests that are allocated in the cache
+7. the number of write blocks
+8. the number of write blocks that hit uncommitted block
+9. the number of write blocks that hit committed block
+10. the number of write blocks that bypass the cache
+11. the number of write blocks that are allocated in the cache
12. the number of write requests that are blocked on the freelist
13. the number of flush requests
14. the number of discard requests

2022-07-11 21:07:14

by Mikulas Patocka

[permalink] [raw]
Subject: [PATCH 2/4] dm-writecache: count the number of blocks read, not the number of read bios

Change dm-writecache, so that it counts the number of blocks read instead
of the number of read bios. Bios can be split and requeued using the
dm_accept_partial_bio function, so counting of bios provided inaccurate
results.

Signed-off-by: Mikulas Patocka <[email protected]>
Reported-by: Yu Kuai <[email protected]>

Index: linux-2.6/drivers/md/dm-writecache.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-writecache.c
+++ linux-2.6/drivers/md/dm-writecache.c
@@ -1365,6 +1365,7 @@ read_next_block:
}
} else {
writecache_map_remap_origin(wc, bio, e);
+ wc->stats.reads += (bio->bi_iter.bi_size - wc->block_size) >> wc->block_size_bits;
map_op = WC_MAP_REMAP_ORIGIN;
}

Index: linux-2.6/Documentation/admin-guide/device-mapper/writecache.rst
===================================================================
--- linux-2.6.orig/Documentation/admin-guide/device-mapper/writecache.rst
+++ linux-2.6/Documentation/admin-guide/device-mapper/writecache.rst
@@ -78,8 +78,8 @@ Status:
2. the number of blocks
3. the number of free blocks
4. the number of blocks under writeback
-5. the number of read requests
-6. the number of read requests that hit the cache
+5. the number of read blocks
+6. the number of read blocks that hit the cache
7. the number of write requests
8. the number of write requests that hit uncommitted block
9. the number of write requests that hit committed block