[PATCH] dm-kcopyd: Delayed unplug of the queues

[PATCH] dm-kcopyd: Delayed unplug of the queues

[Mikulas Patocka]

dm-kcopyd: Delayed unplug of the queues

This patch improves performance aby about 20% when writing to the snapshot
origin. We keep track of two block devices to unplug (one for read and the
other for write) and unplug them when exiting "do_work" thread. If there are
more devices used (in theory it could happen, in practice it is rare), we
unplug immediatelly.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-kcopyd.c |   35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

Index: linux-2.6.36-rc7-fast/drivers/md/dm-kcopyd.c
===================================================================
--- linux-2.6.36-rc7-fast.orig/drivers/md/dm-kcopyd.c	2010-10-14 00:35:28.000000000 +0200
+++ linux-2.6.36-rc7-fast/drivers/md/dm-kcopyd.c	2010-10-14 00:35:31.000000000 +0200
@@ -37,6 +37,8 @@ struct dm_kcopyd_client {
 	unsigned int nr_pages;
 	unsigned int nr_free_pages;
 
+	struct block_device *unplug[2];
+
 	struct dm_io_client *io_client;
 
 	wait_queue_head_t destroyq;
@@ -308,6 +310,23 @@ static int run_complete_job(struct kcopy
 	return 0;
 }
 
+static void unplug(struct dm_kcopyd_client *kc, int queue)
+{
+	if (kc->unplug[queue] != NULL) {
+		blk_unplug(bdev_get_queue(kc->unplug[queue]));
+		kc->unplug[queue] = NULL;
+	}
+}
+
+static void prepare_unplug(struct dm_kcopyd_client *kc, int queue,
+			   struct block_device *bdev)
+{
+	if (likely(kc->unplug[queue] == bdev))
+		return;
+	unplug(kc, queue);
+	kc->unplug[queue] = bdev;
+}
+
 static void complete_io(unsigned long error, void *context)
 {
 	struct kcopyd_job *job = (struct kcopyd_job *) context;
@@ -345,7 +364,7 @@ static int run_io_job(struct kcopyd_job 
 {
 	int r;
 	struct dm_io_request io_req = {
-		.bi_rw = job->rw | REQ_UNPLUG,
+		.bi_rw = job->rw,
 		.mem.type = DM_IO_PAGE_LIST,
 		.mem.ptr.pl = job->pages,
 		.mem.offset = job->offset,
@@ -354,10 +373,16 @@ static int run_io_job(struct kcopyd_job 
 		.client = job->kc->io_client,
 	};
 
-	if (job->rw == READ)
+	if (job->rw == READ) {
 		r = dm_io(&io_req, 1, &job->source, NULL);
-	else
+		prepare_unplug(job->kc, READ, job->source.bdev);
+	} else {
+		if (unlikely(job->num_dests > 1))
+			io_req.bi_rw |= REQ_UNPLUG;
 		r = dm_io(&io_req, job->num_dests, job->dests, NULL);
+		if (likely(!(io_req.bi_rw & REQ_UNPLUG)))
+			prepare_unplug(job->kc, WRITE, job->dests[0].bdev);
+	}
 
 	return r;
 }
@@ -439,6 +464,8 @@ static void do_work(struct work_struct *
 	process_jobs(&kc->complete_jobs, kc, run_complete_job);
 	process_jobs(&kc->pages_jobs, kc, run_pages_job);
 	process_jobs(&kc->io_jobs, kc, run_io_job);
+	unplug(kc, READ);
+	unplug(kc, WRITE);
 }
 
 /*
@@ -619,6 +646,8 @@ int dm_kcopyd_client_create(unsigned int
 	INIT_LIST_HEAD(&kc->io_jobs);
 	INIT_LIST_HEAD(&kc->pages_jobs);
 
+	memset(kc->unplug, 0, sizeof kc->unplug);
+
 	kc->job_pool = mempool_create_slab_pool(MIN_JOBS, _job_cache);
 	if (!kc->job_pool)
 		goto bad_slab;

Re: dm-kcopyd: Delayed unplug of the queues

[Mike Snitzer]

On Sun, Oct 24 2010 at  9:03pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> dm-kcopyd: Delayed unplug of the queues
> 
> This patch improves performance aby about 20% when writing to the snapshot
> origin. We keep track of two block devices to unplug (one for read and the
> other for write) and unplug them when exiting "do_work" thread. If there are
> more devices used (in theory it could happen, in practice it is rare), we
> unplug immediatelly.

Again, more specifics on how you realized this 20% performance would be
appreciated.  I'd imagine this patch is really where the increased
latency comes back (due to removing immediate unplug)?

But does the fact that >2 devices will unplug immediately mitigate the
worst of the higher latency (e.g. for DM mirrors with >2 legs)?

Mike

Re: dm-kcopyd: Delayed unplug of the queues

[Mikulas Patocka]

On Fri, 19 Nov 2010, Mike Snitzer wrote:

> On Sun, Oct 24 2010 at  9:03pm -0400,
> Mikulas Patocka <mpatocka@redhat.com> wrote:
> 
> > dm-kcopyd: Delayed unplug of the queues
> > 
> > This patch improves performance aby about 20% when writing to the snapshot
> > origin. We keep track of two block devices to unplug (one for read and the
> > other for write) and unplug them when exiting "do_work" thread. If there are
> > more devices used (in theory it could happen, in practice it is rare), we
> > unplug immediatelly.
> 
> Again, more specifics on how you realized this 20% performance would be
> appreciated.  I'd imagine this patch is really where the increased
> latency comes back (due to removing immediate unplug)?
> 
> But does the fact that >2 devices will unplug immediately mitigate the
> worst of the higher latency (e.g. for DM mirrors with >2 legs)?
> 
> Mike

Hi

The test setup is the same as in previous email. The previous patch 
(removing SYNC) is already applied. cfq is used as the scheduler (as in 
previous tests).

Sequential write throughput in MB/s (4k, 32k, 512k chunksize)

without this patch:
non-shared snapshots: 15.3, 18.5, 17.3
shared snapshots: 10.9, 24.0, 25.9

with this patch:
non-shared snapshots: 14.4, 22.6, 23.0
shared snapshots: 13.2, 24.7, 26.1

Mikulas

[PATCH v2] dm kcopyd: delay unplug of the queues

[Mike Snitzer]

From: Mikulas Patocka <mpatocka@redhat.com>

This patch improves performance by about 20% when writing to the
snapshot origin.

Sequential write throughput (chunksize of 4k, 32k, 512k)
unpatched: 15.2, 18.5, 17.5 MB/s
patched:   14.4, 22.6, 23.0 MB/s

We keep track of two block devices to unplug (one for read and the
other for write) and unplug them when exiting "do_work" thread.  If
there are more devices used (in theory it could happen, in practice it
is rare), we unplug immediately.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
 drivers/md/dm-kcopyd.c |   35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

v2: added missing parenthesis around sizeof, renamed prepare_unplug and
    unplug 'queue' parameter to 'rw'.

Index: linux-2.6/drivers/md/dm-kcopyd.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-kcopyd.c
+++ linux-2.6/drivers/md/dm-kcopyd.c
@@ -37,6 +37,8 @@ struct dm_kcopyd_client {
 	unsigned int nr_pages;
 	unsigned int nr_free_pages;
 
+	struct block_device *unplug[2];
+
 	struct dm_io_client *io_client;
 
 	wait_queue_head_t destroyq;
@@ -308,6 +310,23 @@ static int run_complete_job(struct kcopy
 	return 0;
 }
 
+static void unplug(struct dm_kcopyd_client *kc, int rw)
+{
+	if (kc->unplug[rw] != NULL) {
+		blk_unplug(bdev_get_queue(kc->unplug[rw]));
+		kc->unplug[rw] = NULL;
+	}
+}
+
+static void prepare_unplug(struct dm_kcopyd_client *kc, int rw,
+			   struct block_device *bdev)
+{
+	if (likely(kc->unplug[rw] == bdev))
+		return;
+	unplug(kc, rw);
+	kc->unplug[rw] = bdev;
+}
+
 static void complete_io(unsigned long error, void *context)
 {
 	struct kcopyd_job *job = (struct kcopyd_job *) context;
@@ -345,7 +364,7 @@ static int run_io_job(struct kcopyd_job 
 {
 	int r;
 	struct dm_io_request io_req = {
-		.bi_rw = job->rw | REQ_UNPLUG,
+		.bi_rw = job->rw,
 		.mem.type = DM_IO_PAGE_LIST,
 		.mem.ptr.pl = job->pages,
 		.mem.offset = job->offset,
@@ -354,10 +373,16 @@ static int run_io_job(struct kcopyd_job 
 		.client = job->kc->io_client,
 	};
 
-	if (job->rw == READ)
+	if (job->rw == READ) {
 		r = dm_io(&io_req, 1, &job->source, NULL);
-	else
+		prepare_unplug(job->kc, READ, job->source.bdev);
+	} else {
+		if (unlikely(job->num_dests > 1))
+			io_req.bi_rw |= REQ_UNPLUG;
 		r = dm_io(&io_req, job->num_dests, job->dests, NULL);
+		if (likely(!(io_req.bi_rw & REQ_UNPLUG)))
+			prepare_unplug(job->kc, WRITE, job->dests[0].bdev);
+	}
 
 	return r;
 }
@@ -439,6 +464,8 @@ static void do_work(struct work_struct *
 	process_jobs(&kc->complete_jobs, kc, run_complete_job);
 	process_jobs(&kc->pages_jobs, kc, run_pages_job);
 	process_jobs(&kc->io_jobs, kc, run_io_job);
+	unplug(kc, READ);
+	unplug(kc, WRITE);
 }
 
 /*
@@ -619,6 +646,8 @@ int dm_kcopyd_client_create(unsigned int
 	INIT_LIST_HEAD(&kc->io_jobs);
 	INIT_LIST_HEAD(&kc->pages_jobs);
 
+	memset(kc->unplug, 0, sizeof(kc->unplug));
+
 	kc->job_pool = mempool_create_slab_pool(MIN_JOBS, _job_cache);
 	if (!kc->job_pool)
 		goto bad_slab;