Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1387653 > unrolled thread
| Started by | Jens Axboe <axboe@fb.com> |
|---|---|
| First post | 2016-04-26 18:00 +0200 |
| Last post | 2016-05-11 18:40 +0200 |
| Articles | 17 — 6 participants |
Back to article view | Back to linux.kernel
[PATCHSET v5] Make background writeback great again for the first time Jens Axboe <axboe@fb.com> - 2016-04-26 18:00 +0200
[PATCH 6/8] block: add scalable completion tracking of requests Jens Axboe <axboe@fb.com> - 2016-04-26 18:00 +0200
Re: [PATCH 6/8] block: add scalable completion tracking of requests Ming Lei <tom.leiming@gmail.com> - 2016-05-05 10:00 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jan Kara <jack@suse.cz> - 2016-04-27 20:10 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jens Axboe <axboe@fb.com> - 2016-04-27 20:20 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jens Axboe <axboe@kernel.dk> - 2016-04-27 22:40 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jens Axboe <axboe@kernel.dk> - 2016-04-27 23:00 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time xiakaixu <xiakaixu@huawei.com> - 2016-04-28 06:10 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jens Axboe <axboe@kernel.dk> - 2016-04-28 20:40 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jan Kara <jack@suse.cz> - 2016-04-28 14:00 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jens Axboe <axboe@kernel.dk> - 2016-04-28 20:50 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jan Kara <jack@suse.cz> - 2016-05-03 14:20 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Chris Mason <clm@fb.com> - 2016-05-03 14:50 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jan Kara <jack@suse.cz> - 2016-05-03 15:10 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Chris Mason <clm@fb.com> - 2016-05-03 15:50 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jan Kara <jack@suse.cz> - 2016-05-03 16:00 +0200
Re: [PATCHSET v5] Make background writeback great again for the first time Jan Kara <jack@suse.cz> - 2016-05-11 18:40 +0200
| From | Jens Axboe <axboe@fb.com> |
|---|---|
| Date | 2016-04-26 18:00 +0200 |
| Subject | [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsdz4-1JO-17@gated-at.bofh.it> |
Hi, Since the dawn of time, our background buffered writeback has sucked. When we do background buffered writeback, it should have little impact on foreground activity. That's the definition of background activity... But for as long as I can remember, heavy buffered writers have not behaved like that. For instance, if I do something like this: $ dd if=/dev/zero of=foo bs=1M count=10k on my laptop, and then try and start chrome, it basically won't start before the buffered writeback is done. Or, for server oriented workloads, where installation of a big RPM (or similar) adversely impacts database reads or sync writes. When that happens, I get people yelling at me. I have posted plenty of results previously, I'll keep it shorter this time. Here's a run on my laptop, using read-to-pipe-async for reading a 5g file, and rewriting it. You can find this test program in the fio git repo. 4.6-rc3: $ t/read-to-pipe-async -f ~/5g > 5g-new Latency percentiles (usec) (READERS) 50.0000th: 2 75.0000th: 3 90.0000th: 5 95.0000th: 7 99.0000th: 43 99.5000th: 77 99.9000th: 9008 99.9900th: 91008 99.9990th: 286208 99.9999th: 347648 Over=1251, min=0, max=358081 Latency percentiles (usec) (WRITERS) 50.0000th: 4 75.0000th: 8 90.0000th: 13 95.0000th: 15 99.0000th: 32 99.5000th: 43 99.9000th: 81 99.9900th: 2372 99.9990th: 104320 99.9999th: 349696 Over=63, min=1, max=358321 Read rate (KB/sec) : 91859 Write rate (KB/sec): 91859 4.6-rc3 + wb-buf-throttle Latency percentiles (usec) (READERS) 50.0000th: 2 75.0000th: 3 90.0000th: 5 95.0000th: 8 99.0000th: 48 99.5000th: 79 99.9000th: 5304 99.9900th: 22496 99.9990th: 29408 99.9999th: 33728 Over=860, min=0, max=37599 Latency percentiles (usec) (WRITERS) 50.0000th: 4 75.0000th: 9 90.0000th: 14 95.0000th: 16 99.0000th: 34 99.5000th: 45 99.9000th: 87 99.9900th: 1342 99.9990th: 13648 99.9999th: 21280 Over=29, min=1, max=30457 Read rate (KB/sec) : 95832 Write rate (KB/sec): 95832 Better throughput and tighter latencies, for both reads and writes. That's hard not to like. The above was the why. The how is basically throttling background writeback. We still want to issue big writes from the vm side of things, so we get nice and big extents on the file system end. But we don't need to flood the device with THOUSANDS of requests for background writeback. For most devices, we don't need a whole lot to get decent throughput. This adds some simple blk-wb code that keeps limits how much buffered writeback we keep in flight on the device end. It's all about managing the queues on the hardware side. The big change in this version is that it should be pretty much auto-tuning - you no longer have to set a given percentage of writeback bandwidth. I've implemented something similar to CoDel to manage the writeback queue. See the last patch for a full description, but the tldr is that we monitor min latencies over a window of time, and scale up/down the queue based on that. This needs a minimum of tunables, and it stays out of the way, if your device is fast enough. There's a single tunable now, wb_last_usec, that simply sets this latency target. Most people won't have to touch this, it'll work pretty well just being in the ballpark. I welcome testing. If you are sick of Linux bogging down when buffered writes are happening, then this is for you, laptop or server. The patchset is fully stable, I have not observed problems. It passes full xfstest runs, and a variety of benchmarks as well. It works equally well on blk-mq/scsi-mq, and "classic" setups. You can also find this in a branch in the block git repo: git://git.kernel.dk/linux-block.git wb-buf-throttle Note that I rebase this branch when I collapse patches. The wb-buf-throttle-v5 will remain the same as this version. I've folded the device write cache changes into my 4.7 branches, so they are not a part of this posting. Get the full wb-buf-throttle branch, or apply the patches here on top of my for-next. A full patch against Linus' current tree can also be downloaded here: http://brick.kernel.dk/snaps/wb-buf-throttle-v5.patch Changes since v4 - Add some documentation for the two queue sysfs files - Kill off wb_stats sysfs file. Use the trace points to get this info now. - Various work around making this block layer agnostic. The main code now resides in lib/wbt.c and can be plugged into NFS as well, for instance. - Fix an issue with double completions on the block layer side. - Fix an issue where a long sync issue was disregarded, if the stat sample weren't valid. - Speed up the division in rwb_arm_timer(). - Add logic to scale back up for 'unknown' latency events. - Don't track sync issue timestamp of wbt is disabled. - Drop the dirty/writeback page inc/dec patch. We don't need it, and it was racy. - Move block/blk-wb.c to lib/wbt.c Changes since v3 - Re-do the mm/ writheback parts. Add REQ_BG for background writes, and don't overload the wbc 'reason' for writeback decisions. - Add tracking for when apps are sleeping waiting for a page to complete. - Change wbc_to_write() to wbc_to_write_cmd(). - Use atomic_t for the balance_dirty_pages() sleep count. - Add a basic scalable block stats tracking framework. - Rewrite blk-wb core as described above, to dynamically adapt. This is a big change, see the last patch for a full description of it. - Add tracing to blk-wb, instead of using debug printk's. - Rebased to 4.6-rc3 (ish) Changes since v2 - Switch from wb_depth to wb_percent, as that's an easier tunable. - Add the patch to track device depth on the block layer side. - Cleanup the limiting code. - Don't use a fixed limit in the wb wait, since it can change between wakeups. - Minor tweaks, fixups, cleanups. Changes since v1 - Drop sync() WB_SYNC_NONE -> WB_SYNC_ALL change - wb_start_writeback() fills in background/reclaim/sync info in the writeback work, based on writeback reason. - Use WRITE_SYNC for reclaim/sync IO - Split balance_dirty_pages() sleep change into separate patch - Drop get_request() u64 flag change, set the bit on the request directly after-the-fact. - Fix wrong sysfs return value - Various small cleanups Documentation/block/queue-sysfs.txt | 22 + Documentation/block/writeback_cache_control.txt | 4 arch/um/drivers/ubd_kern.c | 2 block/Kconfig | 1 block/Makefile | 2 block/blk-core.c | 26 + block/blk-flush.c | 11 block/blk-mq-sysfs.c | 47 ++ block/blk-mq.c | 44 +- block/blk-mq.h | 3 block/blk-settings.c | 59 +- block/blk-stat.c | 185 ++++++++ block/blk-stat.h | 17 block/blk-sysfs.c | 184 ++++++++ drivers/block/drbd/drbd_main.c | 2 drivers/block/loop.c | 2 drivers/block/mtip32xx/mtip32xx.c | 6 drivers/block/nbd.c | 4 drivers/block/osdblk.c | 2 drivers/block/ps3disk.c | 2 drivers/block/skd_main.c | 2 drivers/block/virtio_blk.c | 6 drivers/block/xen-blkback/xenbus.c | 2 drivers/block/xen-blkfront.c | 3 drivers/ide/ide-disk.c | 6 drivers/md/bcache/super.c | 2 drivers/md/dm-table.c | 20 drivers/md/md.c | 2 drivers/md/raid5-cache.c | 3 drivers/mmc/card/block.c | 2 drivers/mtd/mtd_blkdevs.c | 2 drivers/nvme/host/core.c | 7 drivers/scsi/scsi.c | 3 drivers/scsi/sd.c | 8 drivers/target/target_core_iblock.c | 6 fs/block_dev.c | 2 fs/buffer.c | 2 fs/f2fs/data.c | 2 fs/f2fs/node.c | 2 fs/gfs2/meta_io.c | 3 fs/mpage.c | 9 fs/xfs/xfs_aops.c | 2 include/linux/backing-dev-defs.h | 2 include/linux/blk_types.h | 12 include/linux/blkdev.h | 28 + include/linux/fs.h | 4 include/linux/wbt.h | 95 ++++ include/linux/writeback.h | 10 include/trace/events/wbt.h | 122 +++++ lib/Kconfig | 3 lib/Makefile | 1 lib/wbt.c | 524 ++++++++++++++++++++++++ mm/backing-dev.c | 1 mm/page-writeback.c | 2 54 files changed, 1429 insertions(+), 96 deletions(-) -- Jens Axboe
[toc] | [next] | [standalone]
| From | Jens Axboe <axboe@fb.com> |
|---|---|
| Date | 2016-04-26 18:00 +0200 |
| Subject | [PATCH 6/8] block: add scalable completion tracking of requests |
| Message-ID | <rsdz5-1JO-45@gated-at.bofh.it> |
| In reply to | #1387653 |
For legacy block, we simply track them in the request queue. For
blk-mq, we track them on a per-sw queue basis, which we can then
sum up through the hardware queues and finally to a per device
state.
The stats are tracked in, roughly, 0.1s interval windows.
Add sysfs files to display the stats.
Signed-off-by: Jens Axboe <axboe@fb.com>
---
block/Makefile | 2 +-
block/blk-core.c | 4 +
block/blk-mq-sysfs.c | 47 ++++++++++++
block/blk-mq.c | 14 ++++
block/blk-mq.h | 3 +
block/blk-stat.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++
block/blk-stat.h | 17 +++++
block/blk-sysfs.c | 26 +++++++
include/linux/blk_types.h | 8 ++
include/linux/blkdev.h | 4 +
10 files changed, 308 insertions(+), 1 deletion(-)
create mode 100644 block/blk-stat.c
create mode 100644 block/blk-stat.h
diff --git a/block/Makefile b/block/Makefile
index 9eda2322b2d4..3446e0472df0 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -5,7 +5,7 @@
obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o blk-sysfs.o \
blk-flush.o blk-settings.o blk-ioc.o blk-map.o \
blk-exec.o blk-merge.o blk-softirq.o blk-timeout.o \
- blk-lib.o blk-mq.o blk-mq-tag.o \
+ blk-lib.o blk-mq.o blk-mq-tag.o blk-stat.o \
blk-mq-sysfs.o blk-mq-cpu.o blk-mq-cpumap.o ioctl.o \
genhd.o scsi_ioctl.o partition-generic.o ioprio.o \
badblocks.o partitions/
diff --git a/block/blk-core.c b/block/blk-core.c
index 74c16fd8995d..40b57bf4852c 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2514,6 +2514,8 @@ void blk_start_request(struct request *req)
{
blk_dequeue_request(req);
+ req->issue_time = ktime_to_ns(ktime_get());
+
/*
* We are now handing the request to the hardware, initialize
* resid_len to full count and add the timeout handler.
@@ -2581,6 +2583,8 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
trace_block_rq_complete(req->q, req, nr_bytes);
+ blk_stat_add(&req->q->rq_stats[rq_data_dir(req)], req);
+
if (!req->bio)
return false;
diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 4ea4dd8a1eed..2f68015f8616 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -247,6 +247,47 @@ static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char *page)
return ret;
}
+static void blk_mq_stat_clear(struct blk_mq_hw_ctx *hctx)
+{
+ struct blk_mq_ctx *ctx;
+ unsigned int i;
+
+ hctx_for_each_ctx(hctx, ctx, i) {
+ blk_stat_init(&ctx->stat[0]);
+ blk_stat_init(&ctx->stat[1]);
+ }
+}
+
+static ssize_t blk_mq_hw_sysfs_stat_store(struct blk_mq_hw_ctx *hctx,
+ const char *page, size_t count)
+{
+ blk_mq_stat_clear(hctx);
+ return count;
+}
+
+static ssize_t print_stat(char *page, struct blk_rq_stat *stat, const char *pre)
+{
+ return sprintf(page, "%s samples=%llu, mean=%lld, min=%lld, max=%lld\n",
+ pre, (long long) stat->nr_samples,
+ (long long) stat->mean, (long long) stat->min,
+ (long long) stat->max);
+}
+
+static ssize_t blk_mq_hw_sysfs_stat_show(struct blk_mq_hw_ctx *hctx, char *page)
+{
+ struct blk_rq_stat stat[2];
+ ssize_t ret;
+
+ blk_stat_init(&stat[0]);
+ blk_stat_init(&stat[1]);
+
+ blk_hctx_stat_get(hctx, stat);
+
+ ret = print_stat(page, &stat[0], "read :");
+ ret += print_stat(page + ret, &stat[1], "write:");
+ return ret;
+}
+
static struct blk_mq_ctx_sysfs_entry blk_mq_sysfs_dispatched = {
.attr = {.name = "dispatched", .mode = S_IRUGO },
.show = blk_mq_sysfs_dispatched_show,
@@ -304,6 +345,11 @@ static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_poll = {
.attr = {.name = "io_poll", .mode = S_IRUGO },
.show = blk_mq_hw_sysfs_poll_show,
};
+static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_stat = {
+ .attr = {.name = "stats", .mode = S_IRUGO | S_IWUSR },
+ .show = blk_mq_hw_sysfs_stat_show,
+ .store = blk_mq_hw_sysfs_stat_store,
+};
static struct attribute *default_hw_ctx_attrs[] = {
&blk_mq_hw_sysfs_queued.attr,
@@ -314,6 +360,7 @@ static struct attribute *default_hw_ctx_attrs[] = {
&blk_mq_hw_sysfs_cpus.attr,
&blk_mq_hw_sysfs_active.attr,
&blk_mq_hw_sysfs_poll.attr,
+ &blk_mq_hw_sysfs_stat.attr,
NULL,
};
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 1699baf39b78..71b4a13fbf94 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -29,6 +29,7 @@
#include "blk.h"
#include "blk-mq.h"
#include "blk-mq-tag.h"
+#include "blk-stat.h"
static DEFINE_MUTEX(all_q_mutex);
static LIST_HEAD(all_q_list);
@@ -356,10 +357,19 @@ static void blk_mq_ipi_complete_request(struct request *rq)
put_cpu();
}
+static void blk_mq_stat_add(struct request *rq)
+{
+ struct blk_rq_stat *stat = &rq->mq_ctx->stat[rq_data_dir(rq)];
+
+ blk_stat_add(stat, rq);
+}
+
static void __blk_mq_complete_request(struct request *rq)
{
struct request_queue *q = rq->q;
+ blk_mq_stat_add(rq);
+
if (!q->softirq_done_fn)
blk_mq_end_request(rq, rq->errors);
else
@@ -403,6 +413,8 @@ void blk_mq_start_request(struct request *rq)
if (unlikely(blk_bidi_rq(rq)))
rq->next_rq->resid_len = blk_rq_bytes(rq->next_rq);
+ rq->issue_time = ktime_to_ns(ktime_get());
+
blk_add_timer(rq);
/*
@@ -1761,6 +1773,8 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
spin_lock_init(&__ctx->lock);
INIT_LIST_HEAD(&__ctx->rq_list);
__ctx->queue = q;
+ blk_stat_init(&__ctx->stat[0]);
+ blk_stat_init(&__ctx->stat[1]);
/* If the cpu isn't online, the cpu is mapped to first hctx */
if (!cpu_online(i))
diff --git a/block/blk-mq.h b/block/blk-mq.h
index 9087b11037b7..e107f700ff17 100644
--- a/block/blk-mq.h
+++ b/block/blk-mq.h
@@ -1,6 +1,8 @@
#ifndef INT_BLK_MQ_H
#define INT_BLK_MQ_H
+#include "blk-stat.h"
+
struct blk_mq_tag_set;
struct blk_mq_ctx {
@@ -20,6 +22,7 @@ struct blk_mq_ctx {
/* incremented at completion time */
unsigned long ____cacheline_aligned_in_smp rq_completed[2];
+ struct blk_rq_stat stat[2];
struct request_queue *queue;
struct kobject kobj;
diff --git a/block/blk-stat.c b/block/blk-stat.c
new file mode 100644
index 000000000000..b38776a83173
--- /dev/null
+++ b/block/blk-stat.c
@@ -0,0 +1,184 @@
+/*
+ * Block stat tracking code
+ *
+ * Copyright (C) 2016 Jens Axboe
+ */
+#include <linux/kernel.h>
+#include <linux/blk-mq.h>
+
+#include "blk-stat.h"
+#include "blk-mq.h"
+
+void blk_stat_sum(struct blk_rq_stat *dst, struct blk_rq_stat *src)
+{
+ if (!src->nr_samples)
+ return;
+
+ dst->min = min(dst->min, src->min);
+ dst->max = max(dst->max, src->max);
+
+ if (!dst->nr_samples)
+ dst->mean = src->mean;
+ else {
+ dst->mean = div64_s64((src->mean * src->nr_samples) +
+ (dst->mean * dst->nr_samples),
+ dst->nr_samples + src->nr_samples);
+ }
+ dst->nr_samples += src->nr_samples;
+}
+
+static void blk_mq_stat_get(struct request_queue *q, struct blk_rq_stat *dst)
+{
+ struct blk_mq_hw_ctx *hctx;
+ struct blk_mq_ctx *ctx;
+ int i, j, nr;
+
+ blk_stat_init(&dst[0]);
+ blk_stat_init(&dst[1]);
+
+ nr = 0;
+ do {
+ uint64_t newest = 0;
+
+ queue_for_each_hw_ctx(q, hctx, i) {
+ hctx_for_each_ctx(hctx, ctx, j) {
+ if (!ctx->stat[0].nr_samples &&
+ !ctx->stat[1].nr_samples)
+ continue;
+ if (ctx->stat[0].time > newest)
+ newest = ctx->stat[0].time;
+ if (ctx->stat[1].time > newest)
+ newest = ctx->stat[1].time;
+ }
+ }
+
+ /*
+ * No samples
+ */
+ if (!newest)
+ break;
+
+ queue_for_each_hw_ctx(q, hctx, i) {
+ hctx_for_each_ctx(hctx, ctx, j) {
+ if (ctx->stat[0].time == newest) {
+ blk_stat_sum(&dst[0], &ctx->stat[0]);
+ nr++;
+ }
+ if (ctx->stat[1].time == newest) {
+ blk_stat_sum(&dst[1], &ctx->stat[1]);
+ nr++;
+ }
+ }
+ }
+ /*
+ * If we race on finding an entry, just loop back again.
+ * Should be very rare.
+ */
+ } while (!nr);
+}
+
+void blk_queue_stat_get(struct request_queue *q, struct blk_rq_stat *dst)
+{
+ if (q->mq_ops)
+ blk_mq_stat_get(q, dst);
+ else {
+ memcpy(&dst[0], &q->rq_stats[0], sizeof(struct blk_rq_stat));
+ memcpy(&dst[1], &q->rq_stats[1], sizeof(struct blk_rq_stat));
+ }
+}
+
+void blk_hctx_stat_get(struct blk_mq_hw_ctx *hctx, struct blk_rq_stat *dst)
+{
+ struct blk_mq_ctx *ctx;
+ unsigned int i, nr;
+
+ nr = 0;
+ do {
+ uint64_t newest = 0;
+
+ hctx_for_each_ctx(hctx, ctx, i) {
+ if (!ctx->stat[0].nr_samples &&
+ !ctx->stat[1].nr_samples)
+ continue;
+
+ if (ctx->stat[0].time > newest)
+ newest = ctx->stat[0].time;
+ if (ctx->stat[1].time > newest)
+ newest = ctx->stat[1].time;
+ }
+
+ if (!newest)
+ break;
+
+ hctx_for_each_ctx(hctx, ctx, i) {
+ if (ctx->stat[0].time == newest) {
+ blk_stat_sum(&dst[0], &ctx->stat[0]);
+ nr++;
+ }
+ if (ctx->stat[1].time == newest) {
+ blk_stat_sum(&dst[1], &ctx->stat[1]);
+ nr++;
+ }
+ }
+ /*
+ * If we race on finding an entry, just loop back again.
+ * Should be very rare, as the window is only updated
+ * occasionally
+ */
+ } while (!nr);
+}
+
+static void __blk_stat_init(struct blk_rq_stat *stat, s64 time_now)
+{
+ stat->min = -1ULL;
+ stat->max = stat->nr_samples = stat->mean = 0;
+ stat->time = time_now & BLK_STAT_MASK;
+}
+
+void blk_stat_init(struct blk_rq_stat *stat)
+{
+ __blk_stat_init(stat, ktime_to_ns(ktime_get()));
+}
+
+void blk_stat_add(struct blk_rq_stat *stat, struct request *rq)
+{
+ s64 delta, now, value;
+
+ now = ktime_to_ns(ktime_get());
+ if (now < rq->issue_time)
+ return;
+
+ if ((now & BLK_STAT_MASK) != (stat->time & BLK_STAT_MASK))
+ __blk_stat_init(stat, now);
+
+ value = now - rq->issue_time;
+ if (value > stat->max)
+ stat->max = value;
+ if (value < stat->min)
+ stat->min = value;
+
+ delta = value - stat->mean;
+ if (delta)
+ stat->mean += div64_s64(delta, stat->nr_samples + 1);
+
+ stat->nr_samples++;
+}
+
+void blk_stat_clear(struct request_queue *q)
+{
+ if (q->mq_ops) {
+ struct blk_mq_hw_ctx *hctx;
+ struct blk_mq_ctx *ctx;
+ int i, j;
+
+ queue_for_each_hw_ctx(q, hctx, i) {
+ hctx_for_each_ctx(hctx, ctx, j) {
+ blk_stat_init(&ctx->stat[0]);
+ blk_stat_init(&ctx->stat[1]);
+ }
+ }
+ } else {
+ blk_stat_init(&q->rq_stats[0]);
+ blk_stat_init(&q->rq_stats[1]);
+ }
+}
diff --git a/block/blk-stat.h b/block/blk-stat.h
new file mode 100644
index 000000000000..d77548dbf196
--- /dev/null
+++ b/block/blk-stat.h
@@ -0,0 +1,17 @@
+#ifndef BLK_STAT_H
+#define BLK_STAT_H
+
+/*
+ * ~0.13s window as a power-of-2 (2^27 nsecs)
+ */
+#define BLK_STAT_NSEC 134217728ULL
+#define BLK_STAT_MASK ~(BLK_STAT_NSEC - 1)
+
+void blk_stat_add(struct blk_rq_stat *, struct request *);
+void blk_hctx_stat_get(struct blk_mq_hw_ctx *, struct blk_rq_stat *);
+void blk_queue_stat_get(struct request_queue *, struct blk_rq_stat *);
+void blk_stat_clear(struct request_queue *q);
+void blk_stat_init(struct blk_rq_stat *);
+void blk_stat_sum(struct blk_rq_stat *, struct blk_rq_stat *);
+
+#endif
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 99205965f559..6e516cc0d3d0 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -379,6 +379,26 @@ static ssize_t queue_wc_store(struct request_queue *q, const char *page,
return count;
}
+static ssize_t print_stat(char *page, struct blk_rq_stat *stat, const char *pre)
+{
+ return sprintf(page, "%s samples=%llu, mean=%lld, min=%lld, max=%lld\n",
+ pre, (long long) stat->nr_samples,
+ (long long) stat->mean, (long long) stat->min,
+ (long long) stat->max);
+}
+
+static ssize_t queue_stats_show(struct request_queue *q, char *page)
+{
+ struct blk_rq_stat stat[2];
+ ssize_t ret;
+
+ blk_queue_stat_get(q, stat);
+
+ ret = print_stat(page, &stat[0], "read :");
+ ret += print_stat(page + ret, &stat[1], "write:");
+ return ret;
+}
+
static struct queue_sysfs_entry queue_requests_entry = {
.attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
.show = queue_requests_show,
@@ -516,6 +536,11 @@ static struct queue_sysfs_entry queue_wc_entry = {
.store = queue_wc_store,
};
+static struct queue_sysfs_entry queue_stats_entry = {
+ .attr = {.name = "stats", .mode = S_IRUGO },
+ .show = queue_stats_show,
+};
+
static struct attribute *default_attrs[] = {
&queue_requests_entry.attr,
&queue_ra_entry.attr,
@@ -542,6 +567,7 @@ static struct attribute *default_attrs[] = {
&queue_random_entry.attr,
&queue_poll_entry.attr,
&queue_wc_entry.attr,
+ &queue_stats_entry.attr,
NULL,
};
diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
index 223012451c7a..2b4414fb4d8e 100644
--- a/include/linux/blk_types.h
+++ b/include/linux/blk_types.h
@@ -268,4 +268,12 @@ static inline unsigned int blk_qc_t_to_tag(blk_qc_t cookie)
return cookie & ((1u << BLK_QC_T_SHIFT) - 1);
}
+struct blk_rq_stat {
+ s64 mean;
+ u64 min;
+ u64 max;
+ s64 nr_samples;
+ s64 time;
+};
+
#endif /* __LINUX_BLK_TYPES_H */
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index eee94bd6de52..87f6703ced71 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -153,6 +153,7 @@ struct request {
struct gendisk *rq_disk;
struct hd_struct *part;
unsigned long start_time;
+ s64 issue_time;
#ifdef CONFIG_BLK_CGROUP
struct request_list *rl; /* rl this rq is alloced from */
unsigned long long start_time_ns;
@@ -402,6 +403,9 @@ struct request_queue {
unsigned int nr_sorted;
unsigned int in_flight[2];
+
+ struct blk_rq_stat rq_stats[2];
+
/*
* Number of active block driver functions for which blk_drain_queue()
* must wait. Must be incremented around functions that unlock the
--
2.8.0.rc4.6.g7e4ba36
[toc] | [prev] | [next] | [standalone]
| From | Ming Lei <tom.leiming@gmail.com> |
|---|---|
| Date | 2016-05-05 10:00 +0200 |
| Subject | Re: [PATCH 6/8] block: add scalable completion tracking of requests |
| Message-ID | <rvmmt-3Sg-7@gated-at.bofh.it> |
| In reply to | #1387654 |
On Tue, Apr 26, 2016 at 11:55 PM, Jens Axboe <axboe@fb.com> wrote:
> For legacy block, we simply track them in the request queue. For
> blk-mq, we track them on a per-sw queue basis, which we can then
> sum up through the hardware queues and finally to a per device
> state.
>
> The stats are tracked in, roughly, 0.1s interval windows.
>
> Add sysfs files to display the stats.
>
> Signed-off-by: Jens Axboe <axboe@fb.com>
> ---
> block/Makefile | 2 +-
> block/blk-core.c | 4 +
> block/blk-mq-sysfs.c | 47 ++++++++++++
> block/blk-mq.c | 14 ++++
> block/blk-mq.h | 3 +
> block/blk-stat.c | 184 ++++++++++++++++++++++++++++++++++++++++++++++
> block/blk-stat.h | 17 +++++
> block/blk-sysfs.c | 26 +++++++
> include/linux/blk_types.h | 8 ++
> include/linux/blkdev.h | 4 +
> 10 files changed, 308 insertions(+), 1 deletion(-)
> create mode 100644 block/blk-stat.c
> create mode 100644 block/blk-stat.h
>
> diff --git a/block/Makefile b/block/Makefile
> index 9eda2322b2d4..3446e0472df0 100644
> --- a/block/Makefile
> +++ b/block/Makefile
> @@ -5,7 +5,7 @@
> obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o blk-sysfs.o \
> blk-flush.o blk-settings.o blk-ioc.o blk-map.o \
> blk-exec.o blk-merge.o blk-softirq.o blk-timeout.o \
> - blk-lib.o blk-mq.o blk-mq-tag.o \
> + blk-lib.o blk-mq.o blk-mq-tag.o blk-stat.o \
> blk-mq-sysfs.o blk-mq-cpu.o blk-mq-cpumap.o ioctl.o \
> genhd.o scsi_ioctl.o partition-generic.o ioprio.o \
> badblocks.o partitions/
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 74c16fd8995d..40b57bf4852c 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -2514,6 +2514,8 @@ void blk_start_request(struct request *req)
> {
> blk_dequeue_request(req);
>
> + req->issue_time = ktime_to_ns(ktime_get());
> +
> /*
> * We are now handing the request to the hardware, initialize
> * resid_len to full count and add the timeout handler.
> @@ -2581,6 +2583,8 @@ bool blk_update_request(struct request *req, int error, unsigned int nr_bytes)
>
> trace_block_rq_complete(req->q, req, nr_bytes);
>
> + blk_stat_add(&req->q->rq_stats[rq_data_dir(req)], req);
blk_update_request() is often run lockless, so it might be a problem
to add into queue's status here in case of non-blk-mq. Maybe it is
better to do it in blk_finish_request()?
For blk-mq, blk_stat_add() should be avoided here.
> +
> if (!req->bio)
> return false;
>
> diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
> index 4ea4dd8a1eed..2f68015f8616 100644
> --- a/block/blk-mq-sysfs.c
> +++ b/block/blk-mq-sysfs.c
> @@ -247,6 +247,47 @@ static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char *page)
> return ret;
> }
>
> +static void blk_mq_stat_clear(struct blk_mq_hw_ctx *hctx)
> +{
> + struct blk_mq_ctx *ctx;
> + unsigned int i;
> +
> + hctx_for_each_ctx(hctx, ctx, i) {
> + blk_stat_init(&ctx->stat[0]);
> + blk_stat_init(&ctx->stat[1]);
> + }
> +}
> +
> +static ssize_t blk_mq_hw_sysfs_stat_store(struct blk_mq_hw_ctx *hctx,
> + const char *page, size_t count)
> +{
> + blk_mq_stat_clear(hctx);
> + return count;
> +}
> +
> +static ssize_t print_stat(char *page, struct blk_rq_stat *stat, const char *pre)
> +{
> + return sprintf(page, "%s samples=%llu, mean=%lld, min=%lld, max=%lld\n",
> + pre, (long long) stat->nr_samples,
> + (long long) stat->mean, (long long) stat->min,
> + (long long) stat->max);
> +}
> +
> +static ssize_t blk_mq_hw_sysfs_stat_show(struct blk_mq_hw_ctx *hctx, char *page)
> +{
> + struct blk_rq_stat stat[2];
> + ssize_t ret;
> +
> + blk_stat_init(&stat[0]);
> + blk_stat_init(&stat[1]);
> +
> + blk_hctx_stat_get(hctx, stat);
> +
> + ret = print_stat(page, &stat[0], "read :");
> + ret += print_stat(page + ret, &stat[1], "write:");
> + return ret;
> +}
> +
> static struct blk_mq_ctx_sysfs_entry blk_mq_sysfs_dispatched = {
> .attr = {.name = "dispatched", .mode = S_IRUGO },
> .show = blk_mq_sysfs_dispatched_show,
> @@ -304,6 +345,11 @@ static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_poll = {
> .attr = {.name = "io_poll", .mode = S_IRUGO },
> .show = blk_mq_hw_sysfs_poll_show,
> };
> +static struct blk_mq_hw_ctx_sysfs_entry blk_mq_hw_sysfs_stat = {
> + .attr = {.name = "stats", .mode = S_IRUGO | S_IWUSR },
> + .show = blk_mq_hw_sysfs_stat_show,
> + .store = blk_mq_hw_sysfs_stat_store,
> +};
>
> static struct attribute *default_hw_ctx_attrs[] = {
> &blk_mq_hw_sysfs_queued.attr,
> @@ -314,6 +360,7 @@ static struct attribute *default_hw_ctx_attrs[] = {
> &blk_mq_hw_sysfs_cpus.attr,
> &blk_mq_hw_sysfs_active.attr,
> &blk_mq_hw_sysfs_poll.attr,
> + &blk_mq_hw_sysfs_stat.attr,
> NULL,
> };
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 1699baf39b78..71b4a13fbf94 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -29,6 +29,7 @@
> #include "blk.h"
> #include "blk-mq.h"
> #include "blk-mq-tag.h"
> +#include "blk-stat.h"
>
> static DEFINE_MUTEX(all_q_mutex);
> static LIST_HEAD(all_q_list);
> @@ -356,10 +357,19 @@ static void blk_mq_ipi_complete_request(struct request *rq)
> put_cpu();
> }
>
> +static void blk_mq_stat_add(struct request *rq)
> +{
> + struct blk_rq_stat *stat = &rq->mq_ctx->stat[rq_data_dir(rq)];
> +
> + blk_stat_add(stat, rq);
> +}
> +
> static void __blk_mq_complete_request(struct request *rq)
> {
> struct request_queue *q = rq->q;
>
> + blk_mq_stat_add(rq);
> +
> if (!q->softirq_done_fn)
> blk_mq_end_request(rq, rq->errors);
> else
> @@ -403,6 +413,8 @@ void blk_mq_start_request(struct request *rq)
> if (unlikely(blk_bidi_rq(rq)))
> rq->next_rq->resid_len = blk_rq_bytes(rq->next_rq);
>
> + rq->issue_time = ktime_to_ns(ktime_get());
> +
> blk_add_timer(rq);
>
> /*
> @@ -1761,6 +1773,8 @@ static void blk_mq_init_cpu_queues(struct request_queue *q,
> spin_lock_init(&__ctx->lock);
> INIT_LIST_HEAD(&__ctx->rq_list);
> __ctx->queue = q;
> + blk_stat_init(&__ctx->stat[0]);
> + blk_stat_init(&__ctx->stat[1]);
>
> /* If the cpu isn't online, the cpu is mapped to first hctx */
> if (!cpu_online(i))
> diff --git a/block/blk-mq.h b/block/blk-mq.h
> index 9087b11037b7..e107f700ff17 100644
> --- a/block/blk-mq.h
> +++ b/block/blk-mq.h
> @@ -1,6 +1,8 @@
> #ifndef INT_BLK_MQ_H
> #define INT_BLK_MQ_H
>
> +#include "blk-stat.h"
> +
> struct blk_mq_tag_set;
>
> struct blk_mq_ctx {
> @@ -20,6 +22,7 @@ struct blk_mq_ctx {
>
> /* incremented at completion time */
> unsigned long ____cacheline_aligned_in_smp rq_completed[2];
> + struct blk_rq_stat stat[2];
>
> struct request_queue *queue;
> struct kobject kobj;
> diff --git a/block/blk-stat.c b/block/blk-stat.c
> new file mode 100644
> index 000000000000..b38776a83173
> --- /dev/null
> +++ b/block/blk-stat.c
> @@ -0,0 +1,184 @@
> +/*
> + * Block stat tracking code
> + *
> + * Copyright (C) 2016 Jens Axboe
> + */
> +#include <linux/kernel.h>
> +#include <linux/blk-mq.h>
> +
> +#include "blk-stat.h"
> +#include "blk-mq.h"
> +
> +void blk_stat_sum(struct blk_rq_stat *dst, struct blk_rq_stat *src)
> +{
> + if (!src->nr_samples)
> + return;
> +
> + dst->min = min(dst->min, src->min);
> + dst->max = max(dst->max, src->max);
> +
> + if (!dst->nr_samples)
> + dst->mean = src->mean;
> + else {
> + dst->mean = div64_s64((src->mean * src->nr_samples) +
> + (dst->mean * dst->nr_samples),
> + dst->nr_samples + src->nr_samples);
> + }
> + dst->nr_samples += src->nr_samples;
> +}
> +
> +static void blk_mq_stat_get(struct request_queue *q, struct blk_rq_stat *dst)
> +{
> + struct blk_mq_hw_ctx *hctx;
> + struct blk_mq_ctx *ctx;
> + int i, j, nr;
> +
> + blk_stat_init(&dst[0]);
> + blk_stat_init(&dst[1]);
> +
> + nr = 0;
> + do {
> + uint64_t newest = 0;
> +
> + queue_for_each_hw_ctx(q, hctx, i) {
> + hctx_for_each_ctx(hctx, ctx, j) {
> + if (!ctx->stat[0].nr_samples &&
> + !ctx->stat[1].nr_samples)
> + continue;
> + if (ctx->stat[0].time > newest)
> + newest = ctx->stat[0].time;
> + if (ctx->stat[1].time > newest)
> + newest = ctx->stat[1].time;
> + }
> + }
> +
> + /*
> + * No samples
> + */
> + if (!newest)
> + break;
> +
> + queue_for_each_hw_ctx(q, hctx, i) {
> + hctx_for_each_ctx(hctx, ctx, j) {
> + if (ctx->stat[0].time == newest) {
> + blk_stat_sum(&dst[0], &ctx->stat[0]);
> + nr++;
> + }
> + if (ctx->stat[1].time == newest) {
> + blk_stat_sum(&dst[1], &ctx->stat[1]);
> + nr++;
> + }
> + }
> + }
> + /*
> + * If we race on finding an entry, just loop back again.
> + * Should be very rare.
> + */
> + } while (!nr);
> +}
> +
> +void blk_queue_stat_get(struct request_queue *q, struct blk_rq_stat *dst)
> +{
> + if (q->mq_ops)
> + blk_mq_stat_get(q, dst);
> + else {
> + memcpy(&dst[0], &q->rq_stats[0], sizeof(struct blk_rq_stat));
> + memcpy(&dst[1], &q->rq_stats[1], sizeof(struct blk_rq_stat));
> + }
> +}
> +
> +void blk_hctx_stat_get(struct blk_mq_hw_ctx *hctx, struct blk_rq_stat *dst)
> +{
> + struct blk_mq_ctx *ctx;
> + unsigned int i, nr;
> +
> + nr = 0;
> + do {
> + uint64_t newest = 0;
> +
> + hctx_for_each_ctx(hctx, ctx, i) {
> + if (!ctx->stat[0].nr_samples &&
> + !ctx->stat[1].nr_samples)
> + continue;
> +
> + if (ctx->stat[0].time > newest)
> + newest = ctx->stat[0].time;
> + if (ctx->stat[1].time > newest)
> + newest = ctx->stat[1].time;
> + }
> +
> + if (!newest)
> + break;
> +
> + hctx_for_each_ctx(hctx, ctx, i) {
> + if (ctx->stat[0].time == newest) {
> + blk_stat_sum(&dst[0], &ctx->stat[0]);
> + nr++;
> + }
> + if (ctx->stat[1].time == newest) {
> + blk_stat_sum(&dst[1], &ctx->stat[1]);
> + nr++;
> + }
> + }
> + /*
> + * If we race on finding an entry, just loop back again.
> + * Should be very rare, as the window is only updated
> + * occasionally
> + */
> + } while (!nr);
> +}
> +
> +static void __blk_stat_init(struct blk_rq_stat *stat, s64 time_now)
> +{
> + stat->min = -1ULL;
> + stat->max = stat->nr_samples = stat->mean = 0;
> + stat->time = time_now & BLK_STAT_MASK;
> +}
> +
> +void blk_stat_init(struct blk_rq_stat *stat)
> +{
> + __blk_stat_init(stat, ktime_to_ns(ktime_get()));
> +}
> +
> +void blk_stat_add(struct blk_rq_stat *stat, struct request *rq)
> +{
> + s64 delta, now, value;
> +
> + now = ktime_to_ns(ktime_get());
> + if (now < rq->issue_time)
> + return;
> +
> + if ((now & BLK_STAT_MASK) != (stat->time & BLK_STAT_MASK))
> + __blk_stat_init(stat, now);
> +
> + value = now - rq->issue_time;
> + if (value > stat->max)
> + stat->max = value;
> + if (value < stat->min)
> + stat->min = value;
> +
> + delta = value - stat->mean;
> + if (delta)
> + stat->mean += div64_s64(delta, stat->nr_samples + 1);
> +
> + stat->nr_samples++;
> +}
> +
> +void blk_stat_clear(struct request_queue *q)
> +{
> + if (q->mq_ops) {
> + struct blk_mq_hw_ctx *hctx;
> + struct blk_mq_ctx *ctx;
> + int i, j;
> +
> + queue_for_each_hw_ctx(q, hctx, i) {
> + hctx_for_each_ctx(hctx, ctx, j) {
> + blk_stat_init(&ctx->stat[0]);
> + blk_stat_init(&ctx->stat[1]);
> + }
> + }
> + } else {
> + blk_stat_init(&q->rq_stats[0]);
> + blk_stat_init(&q->rq_stats[1]);
> + }
> +}
> diff --git a/block/blk-stat.h b/block/blk-stat.h
> new file mode 100644
> index 000000000000..d77548dbf196
> --- /dev/null
> +++ b/block/blk-stat.h
> @@ -0,0 +1,17 @@
> +#ifndef BLK_STAT_H
> +#define BLK_STAT_H
> +
> +/*
> + * ~0.13s window as a power-of-2 (2^27 nsecs)
> + */
> +#define BLK_STAT_NSEC 134217728ULL
> +#define BLK_STAT_MASK ~(BLK_STAT_NSEC - 1)
> +
> +void blk_stat_add(struct blk_rq_stat *, struct request *);
> +void blk_hctx_stat_get(struct blk_mq_hw_ctx *, struct blk_rq_stat *);
> +void blk_queue_stat_get(struct request_queue *, struct blk_rq_stat *);
> +void blk_stat_clear(struct request_queue *q);
> +void blk_stat_init(struct blk_rq_stat *);
> +void blk_stat_sum(struct blk_rq_stat *, struct blk_rq_stat *);
> +
> +#endif
> diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
> index 99205965f559..6e516cc0d3d0 100644
> --- a/block/blk-sysfs.c
> +++ b/block/blk-sysfs.c
> @@ -379,6 +379,26 @@ static ssize_t queue_wc_store(struct request_queue *q, const char *page,
> return count;
> }
>
> +static ssize_t print_stat(char *page, struct blk_rq_stat *stat, const char *pre)
> +{
> + return sprintf(page, "%s samples=%llu, mean=%lld, min=%lld, max=%lld\n",
> + pre, (long long) stat->nr_samples,
> + (long long) stat->mean, (long long) stat->min,
> + (long long) stat->max);
> +}
> +
> +static ssize_t queue_stats_show(struct request_queue *q, char *page)
> +{
> + struct blk_rq_stat stat[2];
> + ssize_t ret;
> +
> + blk_queue_stat_get(q, stat);
> +
> + ret = print_stat(page, &stat[0], "read :");
> + ret += print_stat(page + ret, &stat[1], "write:");
> + return ret;
> +}
> +
> static struct queue_sysfs_entry queue_requests_entry = {
> .attr = {.name = "nr_requests", .mode = S_IRUGO | S_IWUSR },
> .show = queue_requests_show,
> @@ -516,6 +536,11 @@ static struct queue_sysfs_entry queue_wc_entry = {
> .store = queue_wc_store,
> };
>
> +static struct queue_sysfs_entry queue_stats_entry = {
> + .attr = {.name = "stats", .mode = S_IRUGO },
> + .show = queue_stats_show,
> +};
> +
> static struct attribute *default_attrs[] = {
> &queue_requests_entry.attr,
> &queue_ra_entry.attr,
> @@ -542,6 +567,7 @@ static struct attribute *default_attrs[] = {
> &queue_random_entry.attr,
> &queue_poll_entry.attr,
> &queue_wc_entry.attr,
> + &queue_stats_entry.attr,
> NULL,
> };
>
> diff --git a/include/linux/blk_types.h b/include/linux/blk_types.h
> index 223012451c7a..2b4414fb4d8e 100644
> --- a/include/linux/blk_types.h
> +++ b/include/linux/blk_types.h
> @@ -268,4 +268,12 @@ static inline unsigned int blk_qc_t_to_tag(blk_qc_t cookie)
> return cookie & ((1u << BLK_QC_T_SHIFT) - 1);
> }
>
> +struct blk_rq_stat {
> + s64 mean;
> + u64 min;
> + u64 max;
> + s64 nr_samples;
> + s64 time;
> +};
> +
> #endif /* __LINUX_BLK_TYPES_H */
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index eee94bd6de52..87f6703ced71 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -153,6 +153,7 @@ struct request {
> struct gendisk *rq_disk;
> struct hd_struct *part;
> unsigned long start_time;
> + s64 issue_time;
io_start_time_ns may be reused for same purpose.
> #ifdef CONFIG_BLK_CGROUP
> struct request_list *rl; /* rl this rq is alloced from */
> unsigned long long start_time_ns;
> @@ -402,6 +403,9 @@ struct request_queue {
>
> unsigned int nr_sorted;
> unsigned int in_flight[2];
> +
> + struct blk_rq_stat rq_stats[2];
> +
> /*
> * Number of active block driver functions for which blk_drain_queue()
> * must wait. Must be incremented around functions that unlock the
> --
> 2.8.0.rc4.6.g7e4ba36
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Ming Lei
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-04-27 20:10 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsC4q-5HB-27@gated-at.bofh.it> |
| In reply to | #1387653 |
Hi, On Tue 26-04-16 09:55:23, Jens Axboe wrote: > Since the dawn of time, our background buffered writeback has sucked. > When we do background buffered writeback, it should have little impact > on foreground activity. That's the definition of background activity... > But for as long as I can remember, heavy buffered writers have not > behaved like that. For instance, if I do something like this: > > $ dd if=/dev/zero of=foo bs=1M count=10k > > on my laptop, and then try and start chrome, it basically won't start > before the buffered writeback is done. Or, for server oriented > workloads, where installation of a big RPM (or similar) adversely > impacts database reads or sync writes. When that happens, I get people > yelling at me. > > I have posted plenty of results previously, I'll keep it shorter > this time. Here's a run on my laptop, using read-to-pipe-async for > reading a 5g file, and rewriting it. You can find this test program > in the fio git repo. I have tested your patchset on my test system. Generally I have observed noticeable drop in average throughput for heavy background writes without any other disk activity and also somewhat increased variance in the runtimes. It is most visible on this simple testcases: dd if=/dev/zero of=/mnt/file bs=1M count=10000 and dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly created before each dd run on a dedicated disk. Without your patches I get pretty stable dd runtimes for both cases: dd if=/dev/zero of=/mnt/file bs=1M count=10000 Runtimes: 87.9611 87.3279 87.2554 dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync Runtimes: 93.3502 93.2086 93.541 With your patches the numbers look like: dd if=/dev/zero of=/mnt/file bs=1M count=10000 Runtimes: 108.183, 97.184, 99.9587 dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync Runtimes: 104.9, 102.775, 102.892 I have checked whether the variance is due to some interaction with CFQ which is used for the disk. When I switched the disk to deadline, I still get some variance although, the throughput is still ~10% lower: dd if=/dev/zero of=/mnt/file bs=1M count=10000 Runtimes: 100.417 100.643 100.866 dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync Runtimes: 104.208 106.341 105.483 The disk is rotational SATA drive with writeback cache, queue depth of the disk reported in /sys/block/sdb/device/queue_depth is 1. So I think we still need some tweaking on the low end of the storage spectrum so that we don't lose 10% of throughput for simple cases like this. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@fb.com> |
|---|---|
| Date | 2016-04-27 20:20 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsCe7-5LS-31@gated-at.bofh.it> |
| In reply to | #1389427 |
On 04/27/2016 12:01 PM, Jan Kara wrote: > Hi, > > On Tue 26-04-16 09:55:23, Jens Axboe wrote: >> Since the dawn of time, our background buffered writeback has sucked. >> When we do background buffered writeback, it should have little impact >> on foreground activity. That's the definition of background activity... >> But for as long as I can remember, heavy buffered writers have not >> behaved like that. For instance, if I do something like this: >> >> $ dd if=/dev/zero of=foo bs=1M count=10k >> >> on my laptop, and then try and start chrome, it basically won't start >> before the buffered writeback is done. Or, for server oriented >> workloads, where installation of a big RPM (or similar) adversely >> impacts database reads or sync writes. When that happens, I get people >> yelling at me. >> >> I have posted plenty of results previously, I'll keep it shorter >> this time. Here's a run on my laptop, using read-to-pipe-async for >> reading a 5g file, and rewriting it. You can find this test program >> in the fio git repo. > > I have tested your patchset on my test system. Generally I have observed > noticeable drop in average throughput for heavy background writes without > any other disk activity and also somewhat increased variance in the > runtimes. It is most visible on this simple testcases: > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 > > and > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync > > The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly > created before each dd run on a dedicated disk. > > Without your patches I get pretty stable dd runtimes for both cases: > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 > Runtimes: 87.9611 87.3279 87.2554 > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync > Runtimes: 93.3502 93.2086 93.541 > > With your patches the numbers look like: > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 > Runtimes: 108.183, 97.184, 99.9587 > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync > Runtimes: 104.9, 102.775, 102.892 > > I have checked whether the variance is due to some interaction with CFQ > which is used for the disk. When I switched the disk to deadline, I still > get some variance although, the throughput is still ~10% lower: > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 > Runtimes: 100.417 100.643 100.866 > > dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync > Runtimes: 104.208 106.341 105.483 > > The disk is rotational SATA drive with writeback cache, queue depth of the > disk reported in /sys/block/sdb/device/queue_depth is 1. > > So I think we still need some tweaking on the low end of the storage > spectrum so that we don't lose 10% of throughput for simple cases like > this. Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if you are seeing smaller requests, and that is why it both varies and you get lower throughput? I'll try and setup a test here similar to yours. -- Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2016-04-27 22:40 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsEpA-7BA-5@gated-at.bofh.it> |
| In reply to | #1389445 |
On Wed, Apr 27 2016, Jens Axboe wrote:
> On 04/27/2016 12:01 PM, Jan Kara wrote:
> >Hi,
> >
> >On Tue 26-04-16 09:55:23, Jens Axboe wrote:
> >>Since the dawn of time, our background buffered writeback has sucked.
> >>When we do background buffered writeback, it should have little impact
> >>on foreground activity. That's the definition of background activity...
> >>But for as long as I can remember, heavy buffered writers have not
> >>behaved like that. For instance, if I do something like this:
> >>
> >>$ dd if=/dev/zero of=foo bs=1M count=10k
> >>
> >>on my laptop, and then try and start chrome, it basically won't start
> >>before the buffered writeback is done. Or, for server oriented
> >>workloads, where installation of a big RPM (or similar) adversely
> >>impacts database reads or sync writes. When that happens, I get people
> >>yelling at me.
> >>
> >>I have posted plenty of results previously, I'll keep it shorter
> >>this time. Here's a run on my laptop, using read-to-pipe-async for
> >>reading a 5g file, and rewriting it. You can find this test program
> >>in the fio git repo.
> >
> >I have tested your patchset on my test system. Generally I have observed
> >noticeable drop in average throughput for heavy background writes without
> >any other disk activity and also somewhat increased variance in the
> >runtimes. It is most visible on this simple testcases:
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> >
> >and
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> >
> >The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly
> >created before each dd run on a dedicated disk.
> >
> >Without your patches I get pretty stable dd runtimes for both cases:
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> >Runtimes: 87.9611 87.3279 87.2554
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> >Runtimes: 93.3502 93.2086 93.541
> >
> >With your patches the numbers look like:
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> >Runtimes: 108.183, 97.184, 99.9587
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> >Runtimes: 104.9, 102.775, 102.892
> >
> >I have checked whether the variance is due to some interaction with CFQ
> >which is used for the disk. When I switched the disk to deadline, I still
> >get some variance although, the throughput is still ~10% lower:
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> >Runtimes: 100.417 100.643 100.866
> >
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> >Runtimes: 104.208 106.341 105.483
> >
> >The disk is rotational SATA drive with writeback cache, queue depth of the
> >disk reported in /sys/block/sdb/device/queue_depth is 1.
> >
> >So I think we still need some tweaking on the low end of the storage
> >spectrum so that we don't lose 10% of throughput for simple cases like
> >this.
>
> Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if
> you are seeing smaller requests, and that is why it both varies and
> you get lower throughput? I'll try and setup a test here similar to
> yours.
Jan, care to try the below patch? I can't fully reproduce your issue on
a SCSI disk limited to QD=1, but I have a feeling this might help. It's
a bit of a hack, but the general idea is to allow one more request to
build up for QD=1 devices. That eliminates wait time between one request
finishing, and the next being submitted.
diff --git a/lib/wbt.c b/lib/wbt.c
index 650da911f24f..6b24c8525ace 100644
--- a/lib/wbt.c
+++ b/lib/wbt.c
@@ -93,23 +93,30 @@ void __wbt_done(struct rq_wb *rwb)
* If the device does write back caching, drop further down
* before we wake people up.
*/
- if (rwb->wc && !atomic_read(&rwb->bdi->wb.dirty_sleeping))
+ if (rwb->queue_depth == 1)
+ limit = 2;
+ else if (rwb->wc && !atomic_read(&rwb->bdi->wb.dirty_sleeping))
limit = 0;
else
limit = rwb->wb_normal;
+ inflight = atomic_dec_return(&rwb->inflight);
+
/*
- * Don't wake anyone up if we are above the normal limit. If
- * throttling got disabled (limit == 0) with waiters, ensure
- * that we wake them up.
+ * wbt got disabled with IO in flight. Wake up any potential
+ * waiters, we don't have to do more than that.
*/
- inflight = atomic_dec_return(&rwb->inflight);
- if (limit && inflight >= limit) {
- if (!rwb->wb_max)
- wake_up_all(&rwb->wait);
+ if (!rwb_enabled(rwb)) {
+ wake_up_all(&rwb->wait);
return;
}
+ /*
+ * Don't wake anyone up if we are above the normal limit.
+ */
+ if (inflight >= limit)
+ return;
+
if (waitqueue_active(&rwb->wait)) {
int diff = limit - inflight;
@@ -366,6 +373,9 @@ static inline unsigned int get_limit(struct rq_wb *rwb, unsigned long rw)
} else
limit = rwb->wb_normal;
+ if (rwb->queue_depth == 1)
+ limit = 2;
+
return limit;
}
--
Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2016-04-27 23:00 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsEIW-7JR-3@gated-at.bofh.it> |
| In reply to | #1389573 |
On Wed, Apr 27 2016, Jens Axboe wrote:
> On Wed, Apr 27 2016, Jens Axboe wrote:
> > On 04/27/2016 12:01 PM, Jan Kara wrote:
> > >Hi,
> > >
> > >On Tue 26-04-16 09:55:23, Jens Axboe wrote:
> > >>Since the dawn of time, our background buffered writeback has sucked.
> > >>When we do background buffered writeback, it should have little impact
> > >>on foreground activity. That's the definition of background activity...
> > >>But for as long as I can remember, heavy buffered writers have not
> > >>behaved like that. For instance, if I do something like this:
> > >>
> > >>$ dd if=/dev/zero of=foo bs=1M count=10k
> > >>
> > >>on my laptop, and then try and start chrome, it basically won't start
> > >>before the buffered writeback is done. Or, for server oriented
> > >>workloads, where installation of a big RPM (or similar) adversely
> > >>impacts database reads or sync writes. When that happens, I get people
> > >>yelling at me.
> > >>
> > >>I have posted plenty of results previously, I'll keep it shorter
> > >>this time. Here's a run on my laptop, using read-to-pipe-async for
> > >>reading a 5g file, and rewriting it. You can find this test program
> > >>in the fio git repo.
> > >
> > >I have tested your patchset on my test system. Generally I have observed
> > >noticeable drop in average throughput for heavy background writes without
> > >any other disk activity and also somewhat increased variance in the
> > >runtimes. It is most visible on this simple testcases:
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > >
> > >and
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > >
> > >The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly
> > >created before each dd run on a dedicated disk.
> > >
> > >Without your patches I get pretty stable dd runtimes for both cases:
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > >Runtimes: 87.9611 87.3279 87.2554
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > >Runtimes: 93.3502 93.2086 93.541
> > >
> > >With your patches the numbers look like:
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > >Runtimes: 108.183, 97.184, 99.9587
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > >Runtimes: 104.9, 102.775, 102.892
> > >
> > >I have checked whether the variance is due to some interaction with CFQ
> > >which is used for the disk. When I switched the disk to deadline, I still
> > >get some variance although, the throughput is still ~10% lower:
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > >Runtimes: 100.417 100.643 100.866
> > >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > >Runtimes: 104.208 106.341 105.483
> > >
> > >The disk is rotational SATA drive with writeback cache, queue depth of the
> > >disk reported in /sys/block/sdb/device/queue_depth is 1.
> > >
> > >So I think we still need some tweaking on the low end of the storage
> > >spectrum so that we don't lose 10% of throughput for simple cases like
> > >this.
> >
> > Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if
> > you are seeing smaller requests, and that is why it both varies and
> > you get lower throughput? I'll try and setup a test here similar to
> > yours.
>
> Jan, care to try the below patch? I can't fully reproduce your issue on
> a SCSI disk limited to QD=1, but I have a feeling this might help. It's
> a bit of a hack, but the general idea is to allow one more request to
> build up for QD=1 devices. That eliminates wait time between one request
> finishing, and the next being submitted.
That accidentally added a potentially stall, this one is both cleaner
and should have that fixed.
diff --git a/lib/wbt.c b/lib/wbt.c
index 650da911f24f..322f5e04e994 100644
--- a/lib/wbt.c
+++ b/lib/wbt.c
@@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb)
else
limit = rwb->wb_normal;
+ inflight = atomic_dec_return(&rwb->inflight);
+
/*
- * Don't wake anyone up if we are above the normal limit. If
- * throttling got disabled (limit == 0) with waiters, ensure
- * that we wake them up.
+ * wbt got disabled with IO in flight. Wake up any potential
+ * waiters, we don't have to do more than that.
*/
- inflight = atomic_dec_return(&rwb->inflight);
- if (limit && inflight >= limit) {
- if (!rwb->wb_max)
- wake_up_all(&rwb->wait);
+ if (!rwb_enabled(rwb)) {
+ wake_up_all(&rwb->wait);
return;
}
+ /*
+ * Don't wake anyone up if we are above the normal limit.
+ */
+ if (inflight && inflight >= limit)
+ return;
+
if (waitqueue_active(&rwb->wait)) {
int diff = limit - inflight;
@@ -150,14 +155,26 @@ static void calc_wb_limits(struct rq_wb *rwb)
return;
}
- depth = min_t(unsigned int, RWB_MAX_DEPTH, rwb->queue_depth);
-
/*
- * Reduce max depth by 50%, and re-calculate normal/bg based on that
+ * For QD=1 devices, this is a special case. It's important for those
+ * to have one request ready when one completes, so force a depth of
+ * 2 for those devices. On the backend, it'll be a depth of 1 anyway,
+ * since the device can't have more than that in flight.
*/
- rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
- rwb->wb_normal = (rwb->wb_max + 1) / 2;
- rwb->wb_background = (rwb->wb_max + 3) / 4;
+ if (rwb->queue_depth == 1) {
+ rwb->wb_max = rwb->wb_normal = 2;
+ rwb->wb_background = 1;
+ } else {
+ depth = min_t(unsigned int, RWB_MAX_DEPTH, rwb->queue_depth);
+
+ /*
+ * Reduce max depth by 50%, and re-calculate normal/bg based on
+ * that.
+ */
+ rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
+ rwb->wb_normal = (rwb->wb_max + 1) / 2;
+ rwb->wb_background = (rwb->wb_max + 3) / 4;
+ }
}
static bool inline stat_sample_valid(struct blk_rq_stat *stat)
--
Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | xiakaixu <xiakaixu@huawei.com> |
|---|---|
| Date | 2016-04-28 06:10 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsLr4-52R-5@gated-at.bofh.it> |
| In reply to | #1389580 |
δΊ 2016/4/28 4:59, Jens Axboe ει:
> On Wed, Apr 27 2016, Jens Axboe wrote:
>> On Wed, Apr 27 2016, Jens Axboe wrote:
>>> On 04/27/2016 12:01 PM, Jan Kara wrote:
>>>> Hi,
>>>>
>>>> On Tue 26-04-16 09:55:23, Jens Axboe wrote:
>>>>> Since the dawn of time, our background buffered writeback has sucked.
>>>>> When we do background buffered writeback, it should have little impact
>>>>> on foreground activity. That's the definition of background activity...
>>>>> But for as long as I can remember, heavy buffered writers have not
>>>>> behaved like that. For instance, if I do something like this:
>>>>>
>>>>> $ dd if=/dev/zero of=foo bs=1M count=10k
>>>>>
>>>>> on my laptop, and then try and start chrome, it basically won't start
>>>>> before the buffered writeback is done. Or, for server oriented
>>>>> workloads, where installation of a big RPM (or similar) adversely
>>>>> impacts database reads or sync writes. When that happens, I get people
>>>>> yelling at me.
>>>>>
>>>>> I have posted plenty of results previously, I'll keep it shorter
>>>>> this time. Here's a run on my laptop, using read-to-pipe-async for
>>>>> reading a 5g file, and rewriting it. You can find this test program
>>>>> in the fio git repo.
>>>>
>>>> I have tested your patchset on my test system. Generally I have observed
>>>> noticeable drop in average throughput for heavy background writes without
>>>> any other disk activity and also somewhat increased variance in the
>>>> runtimes. It is most visible on this simple testcases:
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>>
>>>> and
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>>
>>>> The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly
>>>> created before each dd run on a dedicated disk.
>>>>
>>>> Without your patches I get pretty stable dd runtimes for both cases:
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>> Runtimes: 87.9611 87.3279 87.2554
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>> Runtimes: 93.3502 93.2086 93.541
>>>>
>>>> With your patches the numbers look like:
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>> Runtimes: 108.183, 97.184, 99.9587
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>> Runtimes: 104.9, 102.775, 102.892
>>>>
>>>> I have checked whether the variance is due to some interaction with CFQ
>>>> which is used for the disk. When I switched the disk to deadline, I still
>>>> get some variance although, the throughput is still ~10% lower:
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>> Runtimes: 100.417 100.643 100.866
>>>>
>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>> Runtimes: 104.208 106.341 105.483
>>>>
>>>> The disk is rotational SATA drive with writeback cache, queue depth of the
>>>> disk reported in /sys/block/sdb/device/queue_depth is 1.
>>>>
>>>> So I think we still need some tweaking on the low end of the storage
>>>> spectrum so that we don't lose 10% of throughput for simple cases like
>>>> this.
>>>
>>> Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if
>>> you are seeing smaller requests, and that is why it both varies and
>>> you get lower throughput? I'll try and setup a test here similar to
>>> yours.
>>
>> Jan, care to try the below patch? I can't fully reproduce your issue on
>> a SCSI disk limited to QD=1, but I have a feeling this might help. It's
>> a bit of a hack, but the general idea is to allow one more request to
>> build up for QD=1 devices. That eliminates wait time between one request
>> finishing, and the next being submitted.
>
> That accidentally added a potentially stall, this one is both cleaner
> and should have that fixed.
>
> diff --git a/lib/wbt.c b/lib/wbt.c
> index 650da911f24f..322f5e04e994 100644
> --- a/lib/wbt.c
> +++ b/lib/wbt.c
> @@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb)
> else
> limit = rwb->wb_normal;
Hi Jens,
This statement 'limit = rwb->wb_normal' is executed twice, maybe once is
enough. It is not a big deal anyway :)
Another question about this if branch:
if (rwb->wc && !atomic_read(&rwb->bdi->wb.dirty_sleeping))
limit = 0;
I can't follow the logic of this if branch. why set limit equal to 0
when the device supports write back caches and there are tasks being
limited in balance_dirty_pages(). Could you pelase give more info
about this ? Thanks!
>
> + inflight = atomic_dec_return(&rwb->inflight);
> +
> /*
> - * Don't wake anyone up if we are above the normal limit. If
> - * throttling got disabled (limit == 0) with waiters, ensure
> - * that we wake them up.
> + * wbt got disabled with IO in flight. Wake up any potential
> + * waiters, we don't have to do more than that.
> */
> - inflight = atomic_dec_return(&rwb->inflight);
> - if (limit && inflight >= limit) {
> - if (!rwb->wb_max)
> - wake_up_all(&rwb->wait);
> + if (!rwb_enabled(rwb)) {
> + wake_up_all(&rwb->wait);
> return;
> }
Maybe it is better that executing this if branch earlier. So we can wake up
potential waiters in time when wbt got disabled.
>
> + /*
> + * Don't wake anyone up if we are above the normal limit.
> + */
> + if (inflight && inflight >= limit)
> + return;
> +
> if (waitqueue_active(&rwb->wait)) {
> int diff = limit - inflight;
>
> @@ -150,14 +155,26 @@ static void calc_wb_limits(struct rq_wb *rwb)
> return;
> }
>
> - depth = min_t(unsigned int, RWB_MAX_DEPTH, rwb->queue_depth);
> -
> /*
> - * Reduce max depth by 50%, and re-calculate normal/bg based on that
> + * For QD=1 devices, this is a special case. It's important for those
> + * to have one request ready when one completes, so force a depth of
> + * 2 for those devices. On the backend, it'll be a depth of 1 anyway,
> + * since the device can't have more than that in flight.
> */
> - rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> - rwb->wb_normal = (rwb->wb_max + 1) / 2;
> - rwb->wb_background = (rwb->wb_max + 3) / 4;
> + if (rwb->queue_depth == 1) {
> + rwb->wb_max = rwb->wb_normal = 2;
> + rwb->wb_background = 1;
> + } else {
> + depth = min_t(unsigned int, RWB_MAX_DEPTH, rwb->queue_depth);
> +
> + /*
> + * Reduce max depth by 50%, and re-calculate normal/bg based on
> + * that.
> + */
> + rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> + rwb->wb_normal = (rwb->wb_max + 1) / 2;
> + rwb->wb_background = (rwb->wb_max + 3) / 4;
> + }
> }
>
> static bool inline stat_sample_valid(struct blk_rq_stat *stat)
>
--
Regards
Kaixu Xia
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2016-04-28 20:40 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsZ0Z-cC-9@gated-at.bofh.it> |
| In reply to | #1389781 |
On 04/27/2016 10:06 PM, xiakaixu wrote:
>> diff --git a/lib/wbt.c b/lib/wbt.c
>> index 650da911f24f..322f5e04e994 100644
>> --- a/lib/wbt.c
>> +++ b/lib/wbt.c
>> @@ -98,18 +98,23 @@ void __wbt_done(struct rq_wb *rwb)
>> else
>> limit = rwb->wb_normal;
> Hi Jens,
>
> This statement 'limit = rwb->wb_normal' is executed twice, maybe once is
> enough. It is not a big deal anyway :)
I'll clean that up, thanks for noticing. No functional difference.
> Another question about this if branch:
>
> if (rwb->wc && !atomic_read(&rwb->bdi->wb.dirty_sleeping))
> limit = 0;
>
> I can't follow the logic of this if branch. why set limit equal to 0
> when the device supports write back caches and there are tasks being
> limited in balance_dirty_pages(). Could you pelase give more info
> about this ? Thanks!
Sure. So for write back caching, we have to try a bit harder to ensure
that the device doesn't build up long internal queues with a lot of
dirty data in the cache. So for the case where we have write back
caching AND we don't have anyone waiting for the IO, allow the queue
depth to drain to zero before building it back up again.
Does that make sense?
>>
>> + inflight = atomic_dec_return(&rwb->inflight);
>> +
>> /*
>> - * Don't wake anyone up if we are above the normal limit. If
>> - * throttling got disabled (limit == 0) with waiters, ensure
>> - * that we wake them up.
>> + * wbt got disabled with IO in flight. Wake up any potential
>> + * waiters, we don't have to do more than that.
>> */
>> - inflight = atomic_dec_return(&rwb->inflight);
>> - if (limit && inflight >= limit) {
>> - if (!rwb->wb_max)
>> - wake_up_all(&rwb->wait);
>> + if (!rwb_enabled(rwb)) {
>> + wake_up_all(&rwb->wait);
>> return;
>> }
>
> Maybe it is better that executing this if branch earlier. So we can wake up
> potential waiters in time when wbt got disabled.
The !rwb_enabled() case will only happen if someone disabled wbt while
we had tracked IO in flight. We have to it below the
atomic_dec_return(), so we could reorder that to be at the front.
Ideally we just want it out-of-line instead, as it's the unexpected
slower path.
--
Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-04-28 14:00 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsSLV-2rx-19@gated-at.bofh.it> |
| In reply to | #1389580 |
On Wed 27-04-16 14:59:15, Jens Axboe wrote:
> On Wed, Apr 27 2016, Jens Axboe wrote:
> > On Wed, Apr 27 2016, Jens Axboe wrote:
> > > On 04/27/2016 12:01 PM, Jan Kara wrote:
> > > >Hi,
> > > >
> > > >On Tue 26-04-16 09:55:23, Jens Axboe wrote:
> > > >>Since the dawn of time, our background buffered writeback has sucked.
> > > >>When we do background buffered writeback, it should have little impact
> > > >>on foreground activity. That's the definition of background activity...
> > > >>But for as long as I can remember, heavy buffered writers have not
> > > >>behaved like that. For instance, if I do something like this:
> > > >>
> > > >>$ dd if=/dev/zero of=foo bs=1M count=10k
> > > >>
> > > >>on my laptop, and then try and start chrome, it basically won't start
> > > >>before the buffered writeback is done. Or, for server oriented
> > > >>workloads, where installation of a big RPM (or similar) adversely
> > > >>impacts database reads or sync writes. When that happens, I get people
> > > >>yelling at me.
> > > >>
> > > >>I have posted plenty of results previously, I'll keep it shorter
> > > >>this time. Here's a run on my laptop, using read-to-pipe-async for
> > > >>reading a 5g file, and rewriting it. You can find this test program
> > > >>in the fio git repo.
> > > >
> > > >I have tested your patchset on my test system. Generally I have observed
> > > >noticeable drop in average throughput for heavy background writes without
> > > >any other disk activity and also somewhat increased variance in the
> > > >runtimes. It is most visible on this simple testcases:
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > > >
> > > >and
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > >
> > > >The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly
> > > >created before each dd run on a dedicated disk.
> > > >
> > > >Without your patches I get pretty stable dd runtimes for both cases:
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > > >Runtimes: 87.9611 87.3279 87.2554
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > >Runtimes: 93.3502 93.2086 93.541
> > > >
> > > >With your patches the numbers look like:
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > > >Runtimes: 108.183, 97.184, 99.9587
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > >Runtimes: 104.9, 102.775, 102.892
> > > >
> > > >I have checked whether the variance is due to some interaction with CFQ
> > > >which is used for the disk. When I switched the disk to deadline, I still
> > > >get some variance although, the throughput is still ~10% lower:
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000
> > > >Runtimes: 100.417 100.643 100.866
> > > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > >Runtimes: 104.208 106.341 105.483
> > > >
> > > >The disk is rotational SATA drive with writeback cache, queue depth of the
> > > >disk reported in /sys/block/sdb/device/queue_depth is 1.
> > > >
> > > >So I think we still need some tweaking on the low end of the storage
> > > >spectrum so that we don't lose 10% of throughput for simple cases like
> > > >this.
> > >
> > > Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if
> > > you are seeing smaller requests, and that is why it both varies and
> > > you get lower throughput? I'll try and setup a test here similar to
> > > yours.
> >
> > Jan, care to try the below patch? I can't fully reproduce your issue on
> > a SCSI disk limited to QD=1, but I have a feeling this might help. It's
> > a bit of a hack, but the general idea is to allow one more request to
> > build up for QD=1 devices. That eliminates wait time between one request
> > finishing, and the next being submitted.
>
> That accidentally added a potentially stall, this one is both cleaner
> and should have that fixed.
>
..
> - rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> - rwb->wb_normal = (rwb->wb_max + 1) / 2;
> - rwb->wb_background = (rwb->wb_max + 3) / 4;
> + if (rwb->queue_depth == 1) {
> + rwb->wb_max = rwb->wb_normal = 2;
> + rwb->wb_background = 1;
This breaks the detection of too big scale_step in scale_up() where we key
of wb_max == 1 value. However even with that fixed no luck :(:
dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
Runtime: 105.126 107.125 105.641
So about the same as before. I'll try to debug this later today...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Jens Axboe <axboe@kernel.dk> |
|---|---|
| Date | 2016-04-28 20:50 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rsZaG-hy-27@gated-at.bofh.it> |
| In reply to | #1390077 |
On 04/28/2016 05:54 AM, Jan Kara wrote:
> On Wed 27-04-16 14:59:15, Jens Axboe wrote:
>> On Wed, Apr 27 2016, Jens Axboe wrote:
>>> On Wed, Apr 27 2016, Jens Axboe wrote:
>>>> On 04/27/2016 12:01 PM, Jan Kara wrote:
>>>>> Hi,
>>>>>
>>>>> On Tue 26-04-16 09:55:23, Jens Axboe wrote:
>>>>>> Since the dawn of time, our background buffered writeback has sucked.
>>>>>> When we do background buffered writeback, it should have little impact
>>>>>> on foreground activity. That's the definition of background activity...
>>>>>> But for as long as I can remember, heavy buffered writers have not
>>>>>> behaved like that. For instance, if I do something like this:
>>>>>>
>>>>>> $ dd if=/dev/zero of=foo bs=1M count=10k
>>>>>>
>>>>>> on my laptop, and then try and start chrome, it basically won't start
>>>>>> before the buffered writeback is done. Or, for server oriented
>>>>>> workloads, where installation of a big RPM (or similar) adversely
>>>>>> impacts database reads or sync writes. When that happens, I get people
>>>>>> yelling at me.
>>>>>>
>>>>>> I have posted plenty of results previously, I'll keep it shorter
>>>>>> this time. Here's a run on my laptop, using read-to-pipe-async for
>>>>>> reading a 5g file, and rewriting it. You can find this test program
>>>>>> in the fio git repo.
>>>>>
>>>>> I have tested your patchset on my test system. Generally I have observed
>>>>> noticeable drop in average throughput for heavy background writes without
>>>>> any other disk activity and also somewhat increased variance in the
>>>>> runtimes. It is most visible on this simple testcases:
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>>>
>>>>> and
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>>>
>>>>> The machine has 4GB of ram, /mnt is an ext3 filesystem that is freshly
>>>>> created before each dd run on a dedicated disk.
>>>>>
>>>>> Without your patches I get pretty stable dd runtimes for both cases:
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>>> Runtimes: 87.9611 87.3279 87.2554
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>>> Runtimes: 93.3502 93.2086 93.541
>>>>>
>>>>> With your patches the numbers look like:
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>>> Runtimes: 108.183, 97.184, 99.9587
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>>> Runtimes: 104.9, 102.775, 102.892
>>>>>
>>>>> I have checked whether the variance is due to some interaction with CFQ
>>>>> which is used for the disk. When I switched the disk to deadline, I still
>>>>> get some variance although, the throughput is still ~10% lower:
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000
>>>>> Runtimes: 100.417 100.643 100.866
>>>>>
>>>>> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
>>>>> Runtimes: 104.208 106.341 105.483
>>>>>
>>>>> The disk is rotational SATA drive with writeback cache, queue depth of the
>>>>> disk reported in /sys/block/sdb/device/queue_depth is 1.
>>>>>
>>>>> So I think we still need some tweaking on the low end of the storage
>>>>> spectrum so that we don't lose 10% of throughput for simple cases like
>>>>> this.
>>>>
>>>> Thanks for testing, Jan! I haven't tried old QD=1 SATA. I wonder if
>>>> you are seeing smaller requests, and that is why it both varies and
>>>> you get lower throughput? I'll try and setup a test here similar to
>>>> yours.
>>>
>>> Jan, care to try the below patch? I can't fully reproduce your issue on
>>> a SCSI disk limited to QD=1, but I have a feeling this might help. It's
>>> a bit of a hack, but the general idea is to allow one more request to
>>> build up for QD=1 devices. That eliminates wait time between one request
>>> finishing, and the next being submitted.
>>
>> That accidentally added a potentially stall, this one is both cleaner
>> and should have that fixed.
>>
> ..
>> - rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
>> - rwb->wb_normal = (rwb->wb_max + 1) / 2;
>> - rwb->wb_background = (rwb->wb_max + 3) / 4;
>> + if (rwb->queue_depth == 1) {
>> + rwb->wb_max = rwb->wb_normal = 2;
>> + rwb->wb_background = 1;
>
> This breaks the detection of too big scale_step in scale_up() where we key
> of wb_max == 1 value. However even with that fixed no luck :(:
Yeah, I need to look at that. For QD=1, I think the only sensible values
for max/normal/bg is 2/2/1 and 1/1/1 if we step down.
> dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> Runtime: 105.126 107.125 105.641
>
> So about the same as before. I'll try to debug this later today...
Thanks, I'm very interested in what you find!
--
Jens Axboe
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-05-03 14:20 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <ruHt1-82C-9@gated-at.bofh.it> |
| In reply to | #1390455 |
On Thu 28-04-16 12:46:41, Jens Axboe wrote:
> >>- rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> >>- rwb->wb_normal = (rwb->wb_max + 1) / 2;
> >>- rwb->wb_background = (rwb->wb_max + 3) / 4;
> >>+ if (rwb->queue_depth == 1) {
> >>+ rwb->wb_max = rwb->wb_normal = 2;
> >>+ rwb->wb_background = 1;
> >
> >This breaks the detection of too big scale_step in scale_up() where we key
> >of wb_max == 1 value. However even with that fixed no luck :(:
>
> Yeah, I need to look at that. For QD=1, I think the only sensible values for
> max/normal/bg is 2/2/1 and 1/1/1 if we step down.
>
> >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> >Runtime: 105.126 107.125 105.641
> >
> >So about the same as before. I'll try to debug this later today...
>
> Thanks, I'm very interested in what you find!
OK, so the reason was relatively standard in the end. I was using ext3 (or
more exactly ext4 without delayed allocation) for the test. The throttling
of background writes gave more priority to writes from the journalling
thread which happen with WRITE_SYNC and thus are not throttled. Thus the
journalling thread ended up having to do more data writeback to be able to
commit a transaction (due to requirements of data=ordered mode) and it is
less efficient at that than the normal flusher thread.
So this is an example where throttling background writeback effectively
just pushes more work into another context which does it less efficiently
and indirectly makes everyone wait for it. ext3 has been always sensitive to
issues like this. ext4 is using delayed allocation and thus only data
writes into holes end up being part of a transaction -> simple dd test case
doesn't hit that path. And indeed when I repeat the same test with ext4,
the numbers with and without your patch are exactly the same.
The question remains how common a pattern where throttling of background
writeback delays also something else is. I'll schedule a couple of
benchmarks to measure impact of your patches for a wider range of workloads
(but sadly pretty limited set of hw). If ext3 is the only one seeing
issues, I would be willing to accept that ext3 takes the hit since it is
doing something rather stupid (but inherent in its journal design) and we
have a way to deal with this either by enabling delayed allocation or by
turning off the writeback throttling...
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Chris Mason <clm@fb.com> |
|---|---|
| Date | 2016-05-03 14:50 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <ruHW3-8fC-15@gated-at.bofh.it> |
| In reply to | #1393372 |
On Tue, May 03, 2016 at 02:17:19PM +0200, Jan Kara wrote:
> On Thu 28-04-16 12:46:41, Jens Axboe wrote:
> > >>- rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> > >>- rwb->wb_normal = (rwb->wb_max + 1) / 2;
> > >>- rwb->wb_background = (rwb->wb_max + 3) / 4;
> > >>+ if (rwb->queue_depth == 1) {
> > >>+ rwb->wb_max = rwb->wb_normal = 2;
> > >>+ rwb->wb_background = 1;
> > >
> > >This breaks the detection of too big scale_step in scale_up() where we key
> > >of wb_max == 1 value. However even with that fixed no luck :(:
> >
> > Yeah, I need to look at that. For QD=1, I think the only sensible values for
> > max/normal/bg is 2/2/1 and 1/1/1 if we step down.
> >
> > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > >Runtime: 105.126 107.125 105.641
> > >
> > >So about the same as before. I'll try to debug this later today...
> >
> > Thanks, I'm very interested in what you find!
>
> OK, so the reason was relatively standard in the end. I was using ext3 (or
> more exactly ext4 without delayed allocation) for the test. The throttling
> of background writes gave more priority to writes from the journalling
> thread which happen with WRITE_SYNC and thus are not throttled. Thus the
> journalling thread ended up having to do more data writeback to be able to
> commit a transaction (due to requirements of data=ordered mode) and it is
> less efficient at that than the normal flusher thread.
>
> So this is an example where throttling background writeback effectively
> just pushes more work into another context which does it less efficiently
> and indirectly makes everyone wait for it. ext3 has been always sensitive to
> issues like this. ext4 is using delayed allocation and thus only data
> writes into holes end up being part of a transaction -> simple dd test case
> doesn't hit that path. And indeed when I repeat the same test with ext4,
> the numbers with and without your patch are exactly the same.
>
> The question remains how common a pattern where throttling of background
> writeback delays also something else is. I'll schedule a couple of
> benchmarks to measure impact of your patches for a wider range of workloads
> (but sadly pretty limited set of hw). If ext3 is the only one seeing
> issues, I would be willing to accept that ext3 takes the hit since it is
> doing something rather stupid (but inherent in its journal design) and we
> have a way to deal with this either by enabling delayed allocation or by
> turning off the writeback throttling...
At least in the case of io that we know is going to be data=ordered, we
can bump the prio of those pages?
-chris
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-05-03 15:10 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <ruIfo-nG-5@gated-at.bofh.it> |
| In reply to | #1393394 |
On Tue 03-05-16 08:40:11, Chris Mason wrote:
> On Tue, May 03, 2016 at 02:17:19PM +0200, Jan Kara wrote:
> > On Thu 28-04-16 12:46:41, Jens Axboe wrote:
> > > >>- rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> > > >>- rwb->wb_normal = (rwb->wb_max + 1) / 2;
> > > >>- rwb->wb_background = (rwb->wb_max + 3) / 4;
> > > >>+ if (rwb->queue_depth == 1) {
> > > >>+ rwb->wb_max = rwb->wb_normal = 2;
> > > >>+ rwb->wb_background = 1;
> > > >
> > > >This breaks the detection of too big scale_step in scale_up() where we key
> > > >of wb_max == 1 value. However even with that fixed no luck :(:
> > >
> > > Yeah, I need to look at that. For QD=1, I think the only sensible values for
> > > max/normal/bg is 2/2/1 and 1/1/1 if we step down.
> > >
> > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > >Runtime: 105.126 107.125 105.641
> > > >
> > > >So about the same as before. I'll try to debug this later today...
> > >
> > > Thanks, I'm very interested in what you find!
> >
> > OK, so the reason was relatively standard in the end. I was using ext3 (or
> > more exactly ext4 without delayed allocation) for the test. The throttling
> > of background writes gave more priority to writes from the journalling
> > thread which happen with WRITE_SYNC and thus are not throttled. Thus the
> > journalling thread ended up having to do more data writeback to be able to
> > commit a transaction (due to requirements of data=ordered mode) and it is
> > less efficient at that than the normal flusher thread.
> >
> > So this is an example where throttling background writeback effectively
> > just pushes more work into another context which does it less efficiently
> > and indirectly makes everyone wait for it. ext3 has been always sensitive to
> > issues like this. ext4 is using delayed allocation and thus only data
> > writes into holes end up being part of a transaction -> simple dd test case
> > doesn't hit that path. And indeed when I repeat the same test with ext4,
> > the numbers with and without your patch are exactly the same.
> >
> > The question remains how common a pattern where throttling of background
> > writeback delays also something else is. I'll schedule a couple of
> > benchmarks to measure impact of your patches for a wider range of workloads
> > (but sadly pretty limited set of hw). If ext3 is the only one seeing
> > issues, I would be willing to accept that ext3 takes the hit since it is
> > doing something rather stupid (but inherent in its journal design) and we
> > have a way to deal with this either by enabling delayed allocation or by
> > turning off the writeback throttling...
>
> At least in the case of io that we know is going to be data=ordered, we
> can bump the prio of those pages?
But how would flusher thread, which is submitting IO, know that? We would
have to somehow mark inodes that are part of the running transaction and
flusher thread could give more priority to such writeback - e.g. by using
WRITE_SYNC or at least plain writes. Hmm, if we use an inode flag for that,
it could be doable.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Chris Mason <clm@fb.com> |
|---|---|
| Date | 2016-05-03 15:50 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <ruIS7-Hw-29@gated-at.bofh.it> |
| In reply to | #1393404 |
On Tue, May 03, 2016 at 03:06:09PM +0200, Jan Kara wrote:
> On Tue 03-05-16 08:40:11, Chris Mason wrote:
> > On Tue, May 03, 2016 at 02:17:19PM +0200, Jan Kara wrote:
> > > On Thu 28-04-16 12:46:41, Jens Axboe wrote:
> > > > >>- rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> > > > >>- rwb->wb_normal = (rwb->wb_max + 1) / 2;
> > > > >>- rwb->wb_background = (rwb->wb_max + 3) / 4;
> > > > >>+ if (rwb->queue_depth == 1) {
> > > > >>+ rwb->wb_max = rwb->wb_normal = 2;
> > > > >>+ rwb->wb_background = 1;
> > > > >
> > > > >This breaks the detection of too big scale_step in scale_up() where we key
> > > > >of wb_max == 1 value. However even with that fixed no luck :(:
> > > >
> > > > Yeah, I need to look at that. For QD=1, I think the only sensible values for
> > > > max/normal/bg is 2/2/1 and 1/1/1 if we step down.
> > > >
> > > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > > >Runtime: 105.126 107.125 105.641
> > > > >
> > > > >So about the same as before. I'll try to debug this later today...
> > > >
> > > > Thanks, I'm very interested in what you find!
> > >
> > > OK, so the reason was relatively standard in the end. I was using ext3 (or
> > > more exactly ext4 without delayed allocation) for the test. The throttling
> > > of background writes gave more priority to writes from the journalling
> > > thread which happen with WRITE_SYNC and thus are not throttled. Thus the
> > > journalling thread ended up having to do more data writeback to be able to
> > > commit a transaction (due to requirements of data=ordered mode) and it is
> > > less efficient at that than the normal flusher thread.
> > >
> > > So this is an example where throttling background writeback effectively
> > > just pushes more work into another context which does it less efficiently
> > > and indirectly makes everyone wait for it. ext3 has been always sensitive to
> > > issues like this. ext4 is using delayed allocation and thus only data
> > > writes into holes end up being part of a transaction -> simple dd test case
> > > doesn't hit that path. And indeed when I repeat the same test with ext4,
> > > the numbers with and without your patch are exactly the same.
> > >
> > > The question remains how common a pattern where throttling of background
> > > writeback delays also something else is. I'll schedule a couple of
> > > benchmarks to measure impact of your patches for a wider range of workloads
> > > (but sadly pretty limited set of hw). If ext3 is the only one seeing
> > > issues, I would be willing to accept that ext3 takes the hit since it is
> > > doing something rather stupid (but inherent in its journal design) and we
> > > have a way to deal with this either by enabling delayed allocation or by
> > > turning off the writeback throttling...
> >
> > At least in the case of io that we know is going to be data=ordered, we
> > can bump the prio of those pages?
>
> But how would flusher thread, which is submitting IO, know that? We would
> have to somehow mark inodes that are part of the running transaction and
> flusher thread could give more priority to such writeback - e.g. by using
> WRITE_SYNC or at least plain writes. Hmm, if we use an inode flag for that,
> it could be doable.
This would be specific to the data=ordered code in the FS. If there's
some way to test for an inode or a page's status in the data=ordered
list, the FS writepages call could flag the IO as higher prio?
-chris
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-05-03 16:00 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <ruJ1M-LG-1@gated-at.bofh.it> |
| In reply to | #1393429 |
On Tue 03-05-16 09:42:40, Chris Mason wrote:
> On Tue, May 03, 2016 at 03:06:09PM +0200, Jan Kara wrote:
> > On Tue 03-05-16 08:40:11, Chris Mason wrote:
> > > On Tue, May 03, 2016 at 02:17:19PM +0200, Jan Kara wrote:
> > > > On Thu 28-04-16 12:46:41, Jens Axboe wrote:
> > > > > >>- rwb->wb_max = 1 + ((depth - 1) >> min(31U, rwb->scale_step));
> > > > > >>- rwb->wb_normal = (rwb->wb_max + 1) / 2;
> > > > > >>- rwb->wb_background = (rwb->wb_max + 3) / 4;
> > > > > >>+ if (rwb->queue_depth == 1) {
> > > > > >>+ rwb->wb_max = rwb->wb_normal = 2;
> > > > > >>+ rwb->wb_background = 1;
> > > > > >
> > > > > >This breaks the detection of too big scale_step in scale_up() where we key
> > > > > >of wb_max == 1 value. However even with that fixed no luck :(:
> > > > >
> > > > > Yeah, I need to look at that. For QD=1, I think the only sensible values for
> > > > > max/normal/bg is 2/2/1 and 1/1/1 if we step down.
> > > > >
> > > > > >dd if=/dev/zero of=/mnt/file bs=1M count=10000 conv=fsync
> > > > > >Runtime: 105.126 107.125 105.641
> > > > > >
> > > > > >So about the same as before. I'll try to debug this later today...
> > > > >
> > > > > Thanks, I'm very interested in what you find!
> > > >
> > > > OK, so the reason was relatively standard in the end. I was using ext3 (or
> > > > more exactly ext4 without delayed allocation) for the test. The throttling
> > > > of background writes gave more priority to writes from the journalling
> > > > thread which happen with WRITE_SYNC and thus are not throttled. Thus the
> > > > journalling thread ended up having to do more data writeback to be able to
> > > > commit a transaction (due to requirements of data=ordered mode) and it is
> > > > less efficient at that than the normal flusher thread.
> > > >
> > > > So this is an example where throttling background writeback effectively
> > > > just pushes more work into another context which does it less efficiently
> > > > and indirectly makes everyone wait for it. ext3 has been always sensitive to
> > > > issues like this. ext4 is using delayed allocation and thus only data
> > > > writes into holes end up being part of a transaction -> simple dd test case
> > > > doesn't hit that path. And indeed when I repeat the same test with ext4,
> > > > the numbers with and without your patch are exactly the same.
> > > >
> > > > The question remains how common a pattern where throttling of background
> > > > writeback delays also something else is. I'll schedule a couple of
> > > > benchmarks to measure impact of your patches for a wider range of workloads
> > > > (but sadly pretty limited set of hw). If ext3 is the only one seeing
> > > > issues, I would be willing to accept that ext3 takes the hit since it is
> > > > doing something rather stupid (but inherent in its journal design) and we
> > > > have a way to deal with this either by enabling delayed allocation or by
> > > > turning off the writeback throttling...
> > >
> > > At least in the case of io that we know is going to be data=ordered, we
> > > can bump the prio of those pages?
> >
> > But how would flusher thread, which is submitting IO, know that? We would
> > have to somehow mark inodes that are part of the running transaction and
> > flusher thread could give more priority to such writeback - e.g. by using
> > WRITE_SYNC or at least plain writes. Hmm, if we use an inode flag for that,
> > it could be doable.
>
> This would be specific to the data=ordered code in the FS. If there's
> some way to test for an inode or a page's status in the data=ordered
> list, the FS writepages call could flag the IO as higher prio?
Oh, right, we could do that. I can experiment with that later.
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[toc] | [prev] | [next] | [standalone]
| From | Jan Kara <jack@suse.cz> |
|---|---|
| Date | 2016-05-11 18:40 +0200 |
| Subject | Re: [PATCHSET v5] Make background writeback great again for the first time |
| Message-ID | <rxFl1-3u1-41@gated-at.bofh.it> |
| In reply to | #1393372 |
On Tue 03-05-16 14:17:19, Jan Kara wrote: > The question remains how common a pattern where throttling of background > writeback delays also something else is. I'll schedule a couple of > benchmarks to measure impact of your patches for a wider range of workloads > (but sadly pretty limited set of hw). If ext3 is the only one seeing > issues, I would be willing to accept that ext3 takes the hit since it is > doing something rather stupid (but inherent in its journal design) and we > have a way to deal with this either by enabling delayed allocation or by > turning off the writeback throttling... So I've run some benchmarks on a machine with 6 GB of RAM and SSD with queue depth 32. The filesystem on the disk was XFS this time. I've found couple of regressions. A clear one is with dbench (version 4). The average throughput numbers look like: Baseline WBT Hmean mb/sec-1 30.26 ( 0.00%) 18.67 (-38.28%) Hmean mb/sec-2 40.71 ( 0.00%) 31.25 (-23.23%) Hmean mb/sec-4 52.67 ( 0.00%) 46.83 (-11.09%) Hmean mb/sec-8 69.51 ( 0.00%) 64.35 ( -7.42%) Hmean mb/sec-16 91.07 ( 0.00%) 86.46 ( -5.07%) Hmean mb/sec-32 115.10 ( 0.00%) 110.29 ( -4.18%) Hmean mb/sec-64 145.14 ( 0.00%) 134.97 ( -7.00%) Hmean mb/sec-512 93.99 ( 0.00%) 133.85 ( 42.41%) There were also some losses in a filebench webproxy workload (I can give you exact details of the settings if you want to reproduce it). Also, and this really puzzles me, I've seen higher read latencies in some cases (I've verified they are not just noise by rerunning the test for kernel with writeback throttling patches). For example with the following fio job file: [global] direct=0 ioengine=sync runtime=300 time_based invalidate=1 blocksize=4096 size=10g # Just random value, we are running time based workload log_avg_msec=10 group_reporting=1 [writer] nrfiles=1 filesize=1g fdatasync=256 readwrite=randwrite numjobs=4 [reader] # Simulate random reading from different files, switching to different file # after 16 ios. This somewhat simulates application startup. new_group filesize=100m nrfiles=20 file_service_type=random:16 readwrite=randread I get the following results: Throughput Baseline WBT Hmean kb/sec-writer-write 591.60 ( 0.00%) 507.00 (-14.30%) Hmean kb/sec-reader-read 211.81 ( 0.00%) 137.53 (-35.07%) So both read and write throughput have suffered. And latencies don't offset for the loss either: FIO read latency Min latency-read 1383.00 ( 0.00%) 1519.00 ( -9.83%) 1st-qrtle latency-read 3485.00 ( 0.00%) 5235.00 (-50.22%) 2nd-qrtle latency-read 4708.00 ( 0.00%) 15028.00 (-219.20%) 3rd-qrtle latency-read 10286.00 ( 0.00%) 57622.00 (-460.20%) Max-90% latency-read 195834.00 ( 0.00%) 167149.00 ( 14.65%) Max-93% latency-read 273145.00 ( 0.00%) 200319.00 ( 26.66%) Max-95% latency-read 335434.00 ( 0.00%) 220695.00 ( 34.21%) Max-99% latency-read 537017.00 ( 0.00%) 347174.00 ( 35.35%) Max latency-read 991101.00 ( 0.00%) 485835.00 ( 50.98%) Mean latency-read 51282.79 ( 0.00%) 49953.95 ( 2.59%) So we have reduced the extra high read latencies which is nice but on average there is no change. And another fio jobfile which doesn't look great: [global] direct=0 ioengine=sync runtime=300 blocksize=4096 invalidate=1 time_based ramp_time=5 # Let the flusher thread start before taking measurements log_avg_msec=10 group_reporting=1 [writer] nrfiles=1 filesize=$((MEMTOTAL_BYTES*2)) readwrite=randwrite [reader] # Simulate random reading from different files, switching to different file # after 16 ios. This somewhat simulates application startup. new_group filesize=100m nrfiles=20 file_service_type=random:16 readwrite=randread The throughput numbers look like: Hmean kb/sec-writer-write 24707.22 ( 0.00%) 19912.23 (-19.41%) Hmean kb/sec-reader-read 886.65 ( 0.00%) 905.71 ( 2.15%) So we've got significant hit in writes not really offset by a big increase in reads. Read latency numbers look like (I show the WBT numbers for two runs just so that one can see how variable the latency numbers are because I was puzzled by very high max latency for WBT kernels - quartiles seem rather stable higher percentiles and min/max are rather variable): Baseline WBT WBT Min latency-read 1230.00 ( 0.00%) 1560.00 (-26.83%) 1100.00 ( 10.57%) 1st-qrtle latency-read 3357.00 ( 0.00%) 3351.00 ( 0.18%) 3351.00 ( 0.18%) 2nd-qrtle latency-read 4074.00 ( 0.00%) 4056.00 ( 0.44%) 4022.00 ( 1.28%) 3rd-qrtle latency-read 5198.00 ( 0.00%) 5145.00 ( 1.02%) 5095.00 ( 1.98%) Max-90% latency-read 6594.00 ( 0.00%) 6370.00 ( 3.40%) 6130.00 ( 7.04%) Max-93% latency-read 11251.00 ( 0.00%) 9410.00 ( 16.36%) 6654.00 ( 40.86%) Max-95% latency-read 14769.00 ( 0.00%) 13231.00 ( 10.41%) 10306.00 ( 30.22%) Max-99% latency-read 27826.00 ( 0.00%) 28728.00 ( -3.24%) 25077.00 ( 9.88%) Max latency-read 80202.00 ( 0.00%) 186491.00 (-132.53%) 141346.00 (-76.24%) Mean latency-read 5356.12 ( 0.00%) 5229.00 ( 2.37%) 4927.23 ( 8.01%) I have run also other tests but they have mostly shown no significant difference. Honza -- Jan Kara <jack@suse.com> SUSE Labs, CR
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web