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;

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

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

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel