* [PATCH] Remove BIO_RW_SYNCIO flag from kcopyd
@ 2010-10-25 1:02 Mikulas Patocka
2010-11-19 22:17 ` Mike Snitzer
2010-11-23 23:31 ` [PATCH v2] dm kcopyd: remove SYNC from flags used in dm_io_request Mike Snitzer
0 siblings, 2 replies; 4+ messages in thread
From: Mikulas Patocka @ 2010-10-25 1:02 UTC (permalink / raw)
To: dm-devel; +Cc: Alasdair Graeme Kergon
Remove BIO_RW_SYNCIO flag from kcopyd
This improves write throughput twice when writing to the origin with snapshot
on the same device.
This helps for CFQ I/O scheduler that has separate queues for sync and async
I/O, async is optimized for throughput, sync for latency.
Reallocations are triggered by writes that are usually async, so mark it
as async as well.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/md/dm-kcopyd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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-07 16:09:13.000000000 +0200
+++ linux-2.6.36-rc7-fast/drivers/md/dm-kcopyd.c 2010-10-15 03:18:52.000000000 +0200
@@ -345,7 +345,7 @@ static int run_io_job(struct kcopyd_job
{
int r;
struct dm_io_request io_req = {
- .bi_rw = job->rw | REQ_SYNC | REQ_UNPLUG,
+ .bi_rw = job->rw | REQ_UNPLUG,
.mem.type = DM_IO_PAGE_LIST,
.mem.ptr.pl = job->pages,
.mem.offset = job->offset,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: Remove BIO_RW_SYNCIO flag from kcopyd 2010-10-25 1:02 [PATCH] Remove BIO_RW_SYNCIO flag from kcopyd Mikulas Patocka @ 2010-11-19 22:17 ` Mike Snitzer 2010-11-22 13:45 ` Mikulas Patocka 2010-11-23 23:31 ` [PATCH v2] dm kcopyd: remove SYNC from flags used in dm_io_request Mike Snitzer 1 sibling, 1 reply; 4+ messages in thread From: Mike Snitzer @ 2010-11-19 22:17 UTC (permalink / raw) To: Mikulas Patocka; +Cc: dm-devel, Alasdair G Kergon Hi Mikulas, On Sun, Oct 24 2010 at 9:02pm -0400, Mikulas Patocka <mpatocka@redhat.com> wrote: > Remove BIO_RW_SYNCIO flag from kcopyd > > This improves write throughput twice when writing to the origin with snapshot > on the same device. Can you please be more specific on the performance improvement you observed? You're saying something like: baseline=10MB/s patched=20MB/s? Did you use a larger chunk_size for the snapshot device (e.g. 512K)? I can't reproduce such significant performance improvements but my testbed may be overly constrained (I'm passing an entire disk from host into KVM guest using virtio). I can look to test on bare metal early next week (unless you have more insight that would obviate the need to do so). > This helps for CFQ I/O scheduler that has separate queues for sync and async > I/O, async is optimized for throughput, sync for latency. > Reallocations are triggered by writes that are usually async, so mark it > as async as well. This change is at odds with a previous patch you made to reduce latency of SYNC IO, see linux-2.6 commit: 7ff14a36159d947872870 "dm: unplug queues in threads" So we're now trading latency for throughput? Given the concerns about snapshot-origin throughput (on lvm-general) I think that is reasonable. But I just want to make sure we're consciously favoring throughput over latency (I'll likely update the patch header to explicitly say as much). > 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-07 16:09:13.000000000 +0200 > +++ linux-2.6.36-rc7-fast/drivers/md/dm-kcopyd.c 2010-10-15 03:18:52.000000000 +0200 > @@ -345,7 +345,7 @@ static int run_io_job(struct kcopyd_job > { > int r; > struct dm_io_request io_req = { > - .bi_rw = job->rw | REQ_SYNC | REQ_UNPLUG, > + .bi_rw = job->rw | REQ_UNPLUG, > .mem.type = DM_IO_PAGE_LIST, > .mem.ptr.pl = job->pages, > .mem.offset = job->offset, > > -- > dm-devel mailing list > dm-devel@redhat.com > https://www.redhat.com/mailman/listinfo/dm-devel ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Remove BIO_RW_SYNCIO flag from kcopyd 2010-11-19 22:17 ` Mike Snitzer @ 2010-11-22 13:45 ` Mikulas Patocka 0 siblings, 0 replies; 4+ messages in thread From: Mikulas Patocka @ 2010-11-22 13:45 UTC (permalink / raw) To: Mike Snitzer; +Cc: dm-devel, Alasdair G Kergon Hi On Fri, 19 Nov 2010, Mike Snitzer wrote: > Hi Mikulas, > > On Sun, Oct 24 2010 at 9:02pm -0400, > Mikulas Patocka <mpatocka@redhat.com> wrote: > > > Remove BIO_RW_SYNCIO flag from kcopyd > > > > This improves write throughput twice when writing to the origin with snapshot > > on the same device. > > Can you please be more specific on the performance improvement you > observed? You're saying something like: baseline=10MB/s patched=20MB/s? Yes. I rechecked it now, and it is approximatelly so: (for chunk size 4k, 32k, 512k) unpatched: 8.5, 8.6, 9.3 MB/s this patch: 15.2, 18.5, 17.5 MB/s > Did you use a larger chunk_size for the snapshot device (e.g. 512K)? It has effect for all chunk sizes. > I can't reproduce such significant performance improvements but my testbed > may be overly constrained (I'm passing an entire disk from host into KVM > guest using virtio). I can look to test on bare metal early next week > (unless you have more insight that would obviate the need to do so). My scenario is: LSI MPT controller, U320 15k SCSI disk, 64GiB file mapped with dm-loop, volume group on dm-loop, with both snapshot and origin in it. > > This helps for CFQ I/O scheduler that has separate queues for sync and async > > I/O, async is optimized for throughput, sync for latency. > > Reallocations are triggered by writes that are usually async, so mark it > > as async as well. > > This change is at odds with a previous patch you made to reduce latency > of SYNC IO, see linux-2.6 commit: > 7ff14a36159d947872870 "dm: unplug queues in threads" > > So we're now trading latency for throughput? Yes. I think throughput matters more than latency for writes. > Given the concerns about > snapshot-origin throughput (on lvm-general) I think that is reasonable. > But I just want to make sure we're consciously favoring throughput over > latency (I'll likely update the patch header to explicitly say as much). Yes. Write it there. If someone need latency for writes, we could propagate the SYNC flag from the original write bio here. But if no one needs it, it would be needless complication. Mikulas > > 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-07 16:09:13.000000000 +0200 > > +++ linux-2.6.36-rc7-fast/drivers/md/dm-kcopyd.c 2010-10-15 03:18:52.000000000 +0200 > > @@ -345,7 +345,7 @@ static int run_io_job(struct kcopyd_job > > { > > int r; > > struct dm_io_request io_req = { > > - .bi_rw = job->rw | REQ_SYNC | REQ_UNPLUG, > > + .bi_rw = job->rw | REQ_UNPLUG, > > .mem.type = DM_IO_PAGE_LIST, > > .mem.ptr.pl = job->pages, > > .mem.offset = job->offset, > > > > -- > > dm-devel mailing list > > dm-devel@redhat.com > > https://www.redhat.com/mailman/listinfo/dm-devel > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] dm kcopyd: remove SYNC from flags used in dm_io_request 2010-10-25 1:02 [PATCH] Remove BIO_RW_SYNCIO flag from kcopyd Mikulas Patocka 2010-11-19 22:17 ` Mike Snitzer @ 2010-11-23 23:31 ` Mike Snitzer 1 sibling, 0 replies; 4+ messages in thread From: Mike Snitzer @ 2010-11-23 23:31 UTC (permalink / raw) To: Mikulas Patocka; +Cc: dm-devel, Alasdair G Kergon From: Mikulas Patocka <mpatocka@redhat.com> Removing the REQ_SYNC flag improves write throughput twice when writing to the origin with a snapshot on the same device (using the CFQ I/O scheduler). Sequential write throughput (chunksize of 4k, 32k, 512k) unpatched: 8.5, 8.6, 9.3 MB/s patched: 15.2, 18.5, 17.5 MB/s Snapshot exception reallocations are triggered by writes that are usually async, so mark the associated dm_io_request as async as well. This helps when using the CFQ I/O scheduler because it has separate queues for sync and async I/O, async is optimized for throughput, sync for latency. With this change we're consciously favoring throughput over latency. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> Signed-off-by: Mike Snitzer <snitzer@redhat.com> --- drivers/md/dm-kcopyd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) v2: updated patch header 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-07 16:09:13.000000000 +0200 +++ linux-2.6.36-rc7-fast/drivers/md/dm-kcopyd.c 2010-10-15 03:18:52.000000000 +0200 @@ -345,7 +345,7 @@ static int run_io_job(struct kcopyd_job { int r; struct dm_io_request io_req = { - .bi_rw = job->rw | REQ_SYNC | REQ_UNPLUG, + .bi_rw = job->rw | REQ_UNPLUG, .mem.type = DM_IO_PAGE_LIST, .mem.ptr.pl = job->pages, .mem.offset = job->offset, ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-11-23 23:31 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-10-25 1:02 [PATCH] Remove BIO_RW_SYNCIO flag from kcopyd Mikulas Patocka 2010-11-19 22:17 ` Mike Snitzer 2010-11-22 13:45 ` Mikulas Patocka 2010-11-23 23:31 ` [PATCH v2] dm kcopyd: remove SYNC from flags used in dm_io_request Mike Snitzer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).