* [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) @ 2009-10-26 21:43 Corrado Zoccolo 2009-10-28 8:27 ` Jens Axboe [not found] ` <x49zl7c268s.fsf@segfault.boston.devel.redhat.com> 0 siblings, 2 replies; 8+ messages in thread From: Corrado Zoccolo @ 2009-10-26 21:43 UTC (permalink / raw) To: Linux-Kernel, Jens Axboe, Jeff Moyer [rebased on top of Jeff's latest changes for 2.6.33. Various code style improvements over v1 & v2] This patch series is intended to improve I/O latency, addressing an often neglected, important subset of workloads: the ones for which cfq currently prefers not to do any idling. Those are the ones that would benefit most from having low latency, in fact they are any of: * processes with large think times (e.g. interactive ones like file managers) * seeky (e.g. programs faulting in their code at startup) * or marked as no-idle from upper levels. The patch series addresses this by: * reducing queues' timeslice when many queues have pending I/O * separating queues with different priorities and different characteristics in different service trees, each with an allocated time slice * enable idling when switching between service trees, even for queues that would not have idling enabled otherwise. This provides various benefits: * service tree insertion code is simplified, since it doesn't need to cope with priorities any more. * high priority no_idle queues are no longer penalized when competing with lower priority, idling queues * seeky and no_idle queues have their fair share of disk time, without penalizing NCQ drives' performances, since they can all dispatch together, filling up the available NCQ slots. On a non-NCQ capable drive, a workload of 4 random readers competing with sequential writer, the maximum latency experienced by readers decreased from > 500ms to about 160ms. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) 2009-10-26 21:43 [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) Corrado Zoccolo @ 2009-10-28 8:27 ` Jens Axboe [not found] ` <x49zl7c268s.fsf@segfault.boston.devel.redhat.com> 1 sibling, 0 replies; 8+ messages in thread From: Jens Axboe @ 2009-10-28 8:27 UTC (permalink / raw) To: Corrado Zoccolo; +Cc: Linux-Kernel, Jeff Moyer On Mon, Oct 26 2009, Corrado Zoccolo wrote: > [rebased on top of Jeff's latest changes for 2.6.33. Various code style improvements over v1 & v2] > > This patch series is intended to improve I/O latency, addressing an often > neglected, important subset of workloads: the ones for which cfq currently > prefers not to do any idling. > > Those are the ones that would benefit most from having low latency, in fact > they are any of: > * processes with large think times (e.g. interactive ones like file managers) > * seeky (e.g. programs faulting in their code at startup) > * or marked as no-idle from upper levels. > > The patch series addresses this by: > * reducing queues' timeslice when many queues have pending I/O > * separating queues with different priorities and different characteristics in > different service trees, each with an allocated time slice > * enable idling when switching between service trees, even for queues that > would not have idling enabled otherwise. > > This provides various benefits: > * service tree insertion code is simplified, since it doesn't need to cope with > priorities any more. > * high priority no_idle queues are no longer penalized when competing with > lower priority, idling queues > * seeky and no_idle queues have their fair share of disk time, without > penalizing NCQ drives' performances, since they can all dispatch together, > filling up the available NCQ slots. > > On a non-NCQ capable drive, a workload of 4 random readers competing with > sequential writer, the maximum latency experienced by readers decreased from > > 500ms to about 160ms. Thanks Corrado, this is indeed good stuff. Only style issue left was the one in cfq_get_avg_queues(), I just corrected that manually. I have committed this in a test branch based off for-2.6.33 and will do some testing with it, then merge it into for-2.6.33 if it looks good. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <x49zl7c268s.fsf@segfault.boston.devel.redhat.com>]
[parent not found: <4e5e476b0910271124r2cf9f9c0l83fdc59b50619202@mail.gmail.com>]
[parent not found: <x493a4wsn5c.fsf@segfault.boston.devel.redhat.com>]
[parent not found: <x49fx8wbqd0.fsf@segfault.boston.devel.redhat.com>]
[parent not found: <4e5e476b0911030042q5963718aj5875c542e6f6cc40@mail.gmail.com>]
[parent not found: <x49ocnju35d.fsf@segfault.boston.devel.redhat.com>]
[parent not found: <4e5e476b0911030719m425c208cg311f44a91fad8342@mail.gmail.com>]
* Fwd: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) [not found] ` <4e5e476b0911030719m425c208cg311f44a91fad8342@mail.gmail.com> @ 2009-11-03 18:35 ` Corrado Zoccolo 2009-11-03 20:18 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Corrado Zoccolo @ 2009-11-03 18:35 UTC (permalink / raw) To: Jens Axboe, Linux-Kernel, Jeff Moyer Hi Jens, Jeff did some testing of this patchset on his NCQ-enabled SSD (the 30GB OCZ Vertex). The test suite contained various multiple competing workloads scenarios, and was run on for-2.6.33 and cfq-2.6.33 branches. Max latencies were reduced in most cases, and we had also improvements on bandwidth side in some scenarios, especially for multiple random readers, either alone or competing with writes. 2 random readers aggregate bw increased from 48356 to 74205 and 4 random readers vs 1 seq writer: * aggregate reader bw increased from 35242 to 56400 * writer bandwidth increased from 33269 to 55127 * maximum latency on read decreased from 535 to 324 * maximum latency on writes decreased from 22243 to 1153 It's a win on all measures. The effect increasing the number of readers to 32 (latency_test_2.fio) is even more visible (max read latency reduced from 3305 to 268, aggregated read BW increased from 32894 to 164571). The only case where I see an increased max latency is for 2 random readers vs 1 seq reader: for-2.6.33: randomread.0: read_bw = 15,418K randomread.1: read_bw = 15,399K seqread: read_bw = 409K 0: read_bw = 31226 0: read_lat_max = 11.589 0: read_lat_avg = 3.22366666666667 cfq-2.6.33: randomread.0: read_bw = 10,065K randomread.1: read_bw = 10,067K seqread: read_bw = 101M 0: read_bw = 121132 0: read_lat_max = 303 0: read_lat_avg = 0.282333333333333 but here the increased latency is paid back by a large increase in sequential read BW (the max latency is, btw, experienced by the seq reader, so I think it is a fair behaviour). Jeff observed that the for-2.6.33 numbers were worse than his baseline runs, probably due to changed hw_tag detection. My patchset is much less sensible to hw_tag on SSDs (since there are much less situations in which it would idle), so my numbers are unaffected. Corrado ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) 2009-11-03 18:35 ` Fwd: " Corrado Zoccolo @ 2009-11-03 20:18 ` Jens Axboe 2009-11-03 20:26 ` Jeff Moyer 0 siblings, 1 reply; 8+ messages in thread From: Jens Axboe @ 2009-11-03 20:18 UTC (permalink / raw) To: Corrado Zoccolo; +Cc: Linux-Kernel, Jeff Moyer On Tue, Nov 03 2009, Corrado Zoccolo wrote: > Hi Jens, > Jeff did some testing of this patchset on his NCQ-enabled SSD (the > 30GB OCZ Vertex). > The test suite contained various multiple competing workloads > scenarios, and was run on for-2.6.33 and cfq-2.6.33 branches. > > Max latencies were reduced in most cases, and we had also improvements > on bandwidth side in some scenarios, especially > for multiple random readers, either alone or competing with writes. > 2 random readers aggregate bw increased from 48356 to 74205 > and 4 random readers vs 1 seq writer: > * aggregate reader bw increased from 35242 to 56400 > * writer bandwidth increased from 33269 to 55127 > * maximum latency on read decreased from 535 to 324 > * maximum latency on writes decreased from 22243 to 1153 > It's a win on all measures. > The effect increasing the number of readers to 32 (latency_test_2.fio) > is even more visible (max read latency reduced from 3305 to 268, > aggregated read BW increased from 32894 to 164571). > > The only case where I see an increased max latency is for 2 random > readers vs 1 seq reader: > > for-2.6.33: > randomread.0: read_bw = 15,418K > randomread.1: read_bw = 15,399K > seqread: read_bw = 409K > 0: read_bw = 31226 > 0: read_lat_max = 11.589 > 0: read_lat_avg = 3.22366666666667 > > cfq-2.6.33: > randomread.0: read_bw = 10,065K > randomread.1: read_bw = 10,067K > seqread: read_bw = 101M > 0: read_bw = 121132 > 0: read_lat_max = 303 > 0: read_lat_avg = 0.282333333333333 > > but here the increased latency is paid back by a large increase in > sequential read BW (the max latency is, btw, experienced by the seq > reader, so I think it is a fair behaviour). > > Jeff observed that the for-2.6.33 numbers were worse than his baseline > runs, probably due to changed hw_tag detection. > My patchset is much less sensible to hw_tag on SSDs (since there are > much less situations in which it would idle), so my numbers are > unaffected. Thanks a lot for your testing. My testing on cfq-2.6.33 looks good too, so I pulled it into for-2.6.33 today. Since for-linus contains conflicting changes, can you and Jeff please double check that everything is still in order? The interesting bit here is the merge with for-2.6.33 and the coop limit from Shaohua Li. I did the straight forward merge, but we likely just need to drop that logic since the coop concept is radically different given that we merge and break queues in for-2.6.33. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) 2009-11-03 20:18 ` Jens Axboe @ 2009-11-03 20:26 ` Jeff Moyer 2009-11-03 20:28 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Jeff Moyer @ 2009-11-03 20:26 UTC (permalink / raw) To: Jens Axboe; +Cc: Corrado Zoccolo, Linux-Kernel Jens Axboe <jens.axboe@oracle.com> writes: > Since for-linus contains conflicting changes, can you and Jeff please > double check that everything is still in order? The interesting bit here > is the merge with for-2.6.33 and the coop limit from Shaohua Li. I did > the straight forward merge, but we likely just need to drop that logic > since the coop concept is radically different given that we merge and > break queues in for-2.6.33. Yeah, since I changed the meaning of the cfqq_coop flag, a lot of those tests are just plain wrong. Let me play with it and I'll send you an incremental patch in a bit. Cheers, Jeff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) 2009-11-03 20:26 ` Jeff Moyer @ 2009-11-03 20:28 ` Jens Axboe 2009-11-03 22:00 ` Jeff Moyer 0 siblings, 1 reply; 8+ messages in thread From: Jens Axboe @ 2009-11-03 20:28 UTC (permalink / raw) To: Jeff Moyer; +Cc: Corrado Zoccolo, Linux-Kernel On Tue, Nov 03 2009, Jeff Moyer wrote: > Jens Axboe <jens.axboe@oracle.com> writes: > > > Since for-linus contains conflicting changes, can you and Jeff please > > double check that everything is still in order? The interesting bit here > > is the merge with for-2.6.33 and the coop limit from Shaohua Li. I did > > the straight forward merge, but we likely just need to drop that logic > > since the coop concept is radically different given that we merge and > > break queues in for-2.6.33. > > Yeah, since I changed the meaning of the cfqq_coop flag, a lot of those > tests are just plain wrong. Let me play with it and I'll send you an > incremental patch in a bit. Thanks, here's what I have. It's basically a revert of the commit in question. diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c index b700f41..4ab240c 100644 --- a/block/cfq-iosched.c +++ b/block/cfq-iosched.c @@ -253,7 +253,6 @@ enum cfqq_state_flags { CFQ_CFQQ_FLAG_slice_new, /* no requests dispatched in slice */ CFQ_CFQQ_FLAG_sync, /* synchronous queue */ CFQ_CFQQ_FLAG_coop, /* cfqq is shared */ - CFQ_CFQQ_FLAG_coop_preempt, /* coop preempt */ }; #define CFQ_CFQQ_FNS(name) \ @@ -280,7 +279,6 @@ CFQ_CFQQ_FNS(prio_changed); CFQ_CFQQ_FNS(slice_new); CFQ_CFQQ_FNS(sync); CFQ_CFQQ_FNS(coop); -CFQ_CFQQ_FNS(coop_preempt); #undef CFQ_CFQQ_FNS #define cfq_log_cfqq(cfqd, cfqq, fmt, args...) \ @@ -1070,16 +1068,9 @@ static struct cfq_queue *cfq_get_next_queue(struct cfq_data *cfqd) static struct cfq_queue *cfq_set_active_queue(struct cfq_data *cfqd, struct cfq_queue *cfqq) { - if (!cfqq) { + if (!cfqq) cfqq = cfq_get_next_queue(cfqd); - if (cfqq && !cfq_cfqq_coop_preempt(cfqq)) - cfq_clear_cfqq_coop(cfqq); - } - - if (cfqq) - cfq_clear_cfqq_coop_preempt(cfqq); - __cfq_set_active_queue(cfqd, cfqq); return cfqq; } @@ -2433,16 +2424,8 @@ cfq_should_preempt(struct cfq_data *cfqd, struct cfq_queue *new_cfqq, * if this request is as-good as one we would expect from the * current cfqq, let it preempt */ - if (cfq_rq_close(cfqd, cfqq, rq) && (!cfq_cfqq_coop(new_cfqq) || - cfqd->busy_queues == 1)) { - /* - * Mark new queue coop_preempt, so its coop flag will not be - * cleared when new queue gets scheduled at the very first time - */ - cfq_mark_cfqq_coop_preempt(new_cfqq); - cfq_mark_cfqq_coop(new_cfqq); + if (cfq_rq_close(cfqd, cfqq, rq)) return true; - } return false; } -- Jens Axboe ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Fwd: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) 2009-11-03 20:28 ` Jens Axboe @ 2009-11-03 22:00 ` Jeff Moyer 2009-11-04 7:51 ` Jens Axboe 0 siblings, 1 reply; 8+ messages in thread From: Jeff Moyer @ 2009-11-03 22:00 UTC (permalink / raw) To: Jens Axboe; +Cc: Corrado Zoccolo, Linux-Kernel Jens Axboe <jens.axboe@oracle.com> writes: > On Tue, Nov 03 2009, Jeff Moyer wrote: >> Jens Axboe <jens.axboe@oracle.com> writes: >> >> > Since for-linus contains conflicting changes, can you and Jeff please >> > double check that everything is still in order? The interesting bit here >> > is the merge with for-2.6.33 and the coop limit from Shaohua Li. I did >> > the straight forward merge, but we likely just need to drop that logic >> > since the coop concept is radically different given that we merge and >> > break queues in for-2.6.33. >> >> Yeah, since I changed the meaning of the cfqq_coop flag, a lot of those >> tests are just plain wrong. Let me play with it and I'll send you an >> incremental patch in a bit. > > Thanks, here's what I have. It's basically a revert of the commit in > question. Your patch looks like a straight-forward revert. I still think we need some guards in place, though. For now, I think we can go with what you have, and I'll come up with some other mechanism to deal with this case. Cheers, Jeff ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Fwd: [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) 2009-11-03 22:00 ` Jeff Moyer @ 2009-11-04 7:51 ` Jens Axboe 0 siblings, 0 replies; 8+ messages in thread From: Jens Axboe @ 2009-11-04 7:51 UTC (permalink / raw) To: Jeff Moyer; +Cc: Corrado Zoccolo, Linux-Kernel On Tue, Nov 03 2009, Jeff Moyer wrote: > Jens Axboe <jens.axboe@oracle.com> writes: > > > On Tue, Nov 03 2009, Jeff Moyer wrote: > >> Jens Axboe <jens.axboe@oracle.com> writes: > >> > >> > Since for-linus contains conflicting changes, can you and Jeff please > >> > double check that everything is still in order? The interesting bit here > >> > is the merge with for-2.6.33 and the coop limit from Shaohua Li. I did > >> > the straight forward merge, but we likely just need to drop that logic > >> > since the coop concept is radically different given that we merge and > >> > break queues in for-2.6.33. > >> > >> Yeah, since I changed the meaning of the cfqq_coop flag, a lot of those > >> tests are just plain wrong. Let me play with it and I'll send you an > >> incremental patch in a bit. > > > > Thanks, here's what I have. It's basically a revert of the commit in > > question. > > Your patch looks like a straight-forward revert. I still think we need > some guards in place, though. For now, I think we can go with what you > have, and I'll come up with some other mechanism to deal with this case. Thanks Jeff, I'll merge it up and we can get things straightened out in due time. -- Jens Axboe ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-11-04 7:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-10-26 21:43 [PATCH 0/5] cfq-iosched: improve latency for no-idle queues (v3) Corrado Zoccolo 2009-10-28 8:27 ` Jens Axboe [not found] ` <x49zl7c268s.fsf@segfault.boston.devel.redhat.com> [not found] ` <4e5e476b0910271124r2cf9f9c0l83fdc59b50619202@mail.gmail.com> [not found] ` <x493a4wsn5c.fsf@segfault.boston.devel.redhat.com> [not found] ` <x49fx8wbqd0.fsf@segfault.boston.devel.redhat.com> [not found] ` <4e5e476b0911030042q5963718aj5875c542e6f6cc40@mail.gmail.com> [not found] ` <x49ocnju35d.fsf@segfault.boston.devel.redhat.com> [not found] ` <4e5e476b0911030719m425c208cg311f44a91fad8342@mail.gmail.com> 2009-11-03 18:35 ` Fwd: " Corrado Zoccolo 2009-11-03 20:18 ` Jens Axboe 2009-11-03 20:26 ` Jeff Moyer 2009-11-03 20:28 ` Jens Axboe 2009-11-03 22:00 ` Jeff Moyer 2009-11-04 7:51 ` Jens Axboe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.