[PATCH 0/2] cfq: fixes to bring cfq in line with deadline performance for mid- to high-end storage

[PATCH 0/2] cfq: fixes to bring cfq in line with deadline performance for mid- to high-end storage

[Jeff Moyer]

Hi,

In testing iozone using the flag that enforces an fsync before close, we found
that performance for cfq on ext3 and ext4 file systems was very poor for file
sizes 8MB and below (as compared with a 2.6.18 kernel's cfq, or with a recent
kernel with deadline).  The storage involved is middle of the road SAN storage
connected via a single fiber pair.  Investigation showed that the idling logic
of cfq was causing the process issuing the I/O to stall.  iozone, in this case,
was really dependant on the journal commits done by the jbd thread, but cfq
was instead idling waiting for more I/O from iozone.  Setting slice_idle to
0 for cfq will regain this performance.

However, if you introduce a single sequential reader into the mix, even
with slice_idle set to 0, cfq is unable to perform as well as deadline:

          deadline  cfq, defaults    cfq, slice_idle=0
fs_mark:  294.3     36.1             48.0
fio bsr:  153 MB/s  152 MB/s         147 MB/s

I used fs_mark to simulate iozone (it's less verbose) and fio to start
a single Buffered Sequential Reader (bsr).  The fs_mark numbers are in
files/second.  As you can see, cfq doesn't even compete here.

With either of the two patches applied, we get back a subset of the
performance.  With both applied, we are in line with deadline.  I've also
tested these patches against a single SATA disk and observed no performance
degradation for default tuning.

[PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero
[PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero

[Jeff Moyer]

Hi,

In testing a competing fsync-ing process and a sequential reader on
mid-grade storage, I found that cfq was incapable of achieving the I/O
rates of deadline, no matter how it was tuned.  Investigation, and insight
from Vivek (mostly the latter), led to identifying that we were still
idling for the last queue in the service tree.

Modifying cfq_should_idle to not idle when slice_idle is set to zero got
us much closer to the performance of deadline for this workload.  I have
one follow-on patch that gets us on-par with deadline, but I think this
patch stands alone.

Comments, as always, are appreciated.

Cheers,
Jeff

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 block/cfq-iosched.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 5ff4f48..572a050 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -1842,6 +1842,10 @@ static bool cfq_should_idle(struct cfq_data *cfqd, struct cfq_queue *cfqq)
 	if (prio == IDLE_WORKLOAD)
 		return false;
 
+	/* Don't idle if slice idling is disabled by the user */
+	if (cfqd->cfq_slice_idle == 0)
+		return false;
+
 	/* We do for queues that were marked with idle window flag. */
 	if (cfq_cfqq_idle_window(cfqq) &&
 	   !(blk_queue_nonrot(cfqd->queue) && cfqd->hw_tag))
-- 
1.6.5.2

[PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jeff Moyer]

Hi,

In testing a workload that has a single fsync-ing process and another
process that does a sequential buffered read, I was unable to tune CFQ
to reach the throughput of deadline.  This patch, along with the previous
one, brought CFQ in line with deadline when setting slice_idle to 0.

I'm not sure what the original reason for not allowing sync and async
I/O to be dispatched together was.  If there is a workload I should be
testing that shows the inherent problems of this, please point me at it
and I will resume testing.  Until and unless that workload is identified,
please consider applying this patch.

Cheers,
Jeff

Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
---
 block/cfq-iosched.c |   12 ------------
 1 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 572a050..dab836e 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -2296,18 +2296,6 @@ static bool cfq_may_dispatch(struct cfq_data *cfqd, struct cfq_queue *cfqq)
 {
 	unsigned int max_dispatch;
 
-	/*
-	 * Drain async requests before we start sync IO
-	 */
-	if (cfq_should_idle(cfqd, cfqq) && cfqd->rq_in_flight[BLK_RW_ASYNC])
-		return false;
-
-	/*
-	 * If this is an async queue and we have sync IO in flight, let it wait
-	 */
-	if (cfqd->rq_in_flight[BLK_RW_SYNC] && !cfq_cfqq_sync(cfqq))
-		return false;
-
 	max_dispatch = max_t(unsigned int, cfqd->cfq_quantum / 2, 1);
 	if (cfq_class_idle(cfqq))
 		max_dispatch = 1;
-- 
1.6.5.2

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jens Axboe]

On 21/06/10 21.49, Jeff Moyer wrote:
> Hi,
> 
> In testing a workload that has a single fsync-ing process and another
> process that does a sequential buffered read, I was unable to tune CFQ
> to reach the throughput of deadline.  This patch, along with the previous
> one, brought CFQ in line with deadline when setting slice_idle to 0.
> 
> I'm not sure what the original reason for not allowing sync and async
> I/O to be dispatched together was.  If there is a workload I should be
> testing that shows the inherent problems of this, please point me at it
> and I will resume testing.  Until and unless that workload is identified,
> please consider applying this patch.

The problematic case is/was a normal SATA drive with a buffered
writer and an occasional reader. I'll have to double check my
mail tomorrow, but iirc the issue was that the occasional reader
would suffer great latencies since service times for that single
IO would be delayed at the drive side. It could perhaps just be
a bug in how we handle the slice idling on the read side when the
IO gets delayed initially.

So if my memory is correct, google for the fsync madness and
interactiveness thread that we had some months ago and which
caused a lot of tweaking. The commit adding this is
5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
in July last year. So it was around that time that the mails went
around.


-- 
Jens Axboe

Re: [PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero

[Jens Axboe]

On 21/06/10 21.49, Jeff Moyer wrote:
> Hi,
> 
> In testing a competing fsync-ing process and a sequential reader on
> mid-grade storage, I found that cfq was incapable of achieving the I/O
> rates of deadline, no matter how it was tuned.  Investigation, and insight
> from Vivek (mostly the latter), led to identifying that we were still
> idling for the last queue in the service tree.
> 
> Modifying cfq_should_idle to not idle when slice_idle is set to zero got
> us much closer to the performance of deadline for this workload.  I have
> one follow-on patch that gets us on-par with deadline, but I think this
> patch stands alone.
> 
> Comments, as always, are appreciated.

This looks good.

-- 
Jens Axboe

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jeff Moyer]

Jens Axboe <axboe@kernel.dk> writes:

> On 21/06/10 21.49, Jeff Moyer wrote:
>> Hi,
>> 
>> In testing a workload that has a single fsync-ing process and another
>> process that does a sequential buffered read, I was unable to tune CFQ
>> to reach the throughput of deadline.  This patch, along with the previous
>> one, brought CFQ in line with deadline when setting slice_idle to 0.
>> 
>> I'm not sure what the original reason for not allowing sync and async
>> I/O to be dispatched together was.  If there is a workload I should be
>> testing that shows the inherent problems of this, please point me at it
>> and I will resume testing.  Until and unless that workload is identified,
>> please consider applying this patch.
>
> The problematic case is/was a normal SATA drive with a buffered
> writer and an occasional reader. I'll have to double check my
> mail tomorrow, but iirc the issue was that the occasional reader
> would suffer great latencies since service times for that single
> IO would be delayed at the drive side. It could perhaps just be
> a bug in how we handle the slice idling on the read side when the
> IO gets delayed initially.
>
> So if my memory is correct, google for the fsync madness and
> interactiveness thread that we had some months ago and which
> caused a lot of tweaking. The commit adding this is
> 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
> in July last year. So it was around that time that the mails went
> around.

OK.  Thanks a ton for the pointers!  I really appreciate it!

-Jeff

Re: [PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero

[Vivek Goyal]

On Mon, Jun 21, 2010 at 03:49:48PM -0400, Jeff Moyer wrote:
> Hi,
> 
> In testing a competing fsync-ing process and a sequential reader on
> mid-grade storage, I found that cfq was incapable of achieving the I/O
> rates of deadline, no matter how it was tuned.  Investigation, and insight
> from Vivek (mostly the latter), led to identifying that we were still
> idling for the last queue in the service tree.
> 
> Modifying cfq_should_idle to not idle when slice_idle is set to zero got
> us much closer to the performance of deadline for this workload.  I have
> one follow-on patch that gets us on-par with deadline, but I think this
> patch stands alone.
> 
> Comments, as always, are appreciated.
> 

Looks good to me Jeff. This does take away one functionality and that is
ability to get group service differentation even with slice_idle=0.
cfq_should_idle() was making sure that we don't select the next queue
from next group.

But I think a better way to fix that would be to introduce a new tunable
say group_idle. That way if we are running on some powerful storage box
and don't want the overhead of idling, we can do slice_idle=0 and not idle
among the cfqq's with-in group but still do idle a bit on group and get
the group service differentiation.

I have a small patch for implementing group_idle. I will post that once
you patch is in Jens's tree.

Thanks
Vivek

> Cheers,
> Jeff
> 
> Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> ---
>  block/cfq-iosched.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
> index 5ff4f48..572a050 100644
> --- a/block/cfq-iosched.c
> +++ b/block/cfq-iosched.c
> @@ -1842,6 +1842,10 @@ static bool cfq_should_idle(struct cfq_data *cfqd, struct cfq_queue *cfqq)
>  	if (prio == IDLE_WORKLOAD)
>  		return false;
>  
> +	/* Don't idle if slice idling is disabled by the user */
> +	if (cfqd->cfq_slice_idle == 0)
> +		return false;
> +
>  	/* We do for queues that were marked with idle window flag. */
>  	if (cfq_cfqq_idle_window(cfqq) &&
>  	   !(blk_queue_nonrot(cfqd->queue) && cfqd->hw_tag))
> -- 
> 1.6.5.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Vivek Goyal]

On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
> On 21/06/10 21.49, Jeff Moyer wrote:
> > Hi,
> > 
> > In testing a workload that has a single fsync-ing process and another
> > process that does a sequential buffered read, I was unable to tune CFQ
> > to reach the throughput of deadline.  This patch, along with the previous
> > one, brought CFQ in line with deadline when setting slice_idle to 0.
> > 
> > I'm not sure what the original reason for not allowing sync and async
> > I/O to be dispatched together was.  If there is a workload I should be
> > testing that shows the inherent problems of this, please point me at it
> > and I will resume testing.  Until and unless that workload is identified,
> > please consider applying this patch.
> 
> The problematic case is/was a normal SATA drive with a buffered
> writer and an occasional reader. I'll have to double check my
> mail tomorrow, but iirc the issue was that the occasional reader
> would suffer great latencies since service times for that single
> IO would be delayed at the drive side. It could perhaps just be
> a bug in how we handle the slice idling on the read side when the
> IO gets delayed initially.
> 
> So if my memory is correct, google for the fsync madness and
> interactiveness thread that we had some months ago and which
> caused a lot of tweaking. The commit adding this is
> 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
> in July last year. So it was around that time that the mails went
> around.

Hi Jens,

I suspect we might have introduced this patch because mike galbraith
had issues which application interactiveness (reading data back from swap)
in the prence of heavy writeout on SATA disk.

After this patch we did two enhancements.

- You introduced the logic of building write queue depth gradually.
- Corrado introduced the logic of idling on the random reader service
  tree.

In the past random reader were not protected from WRITES as there was no
idling on random readers. But with corrado's changes of idling on
sync-noidle service tree, I think this problem might have been solved to
a great extent.

Getting rid of this exclusivity of either SYNC/ASYNC requests in request
queue might help us with throughput on storage arrys without loosing
protection for random reader on SATA. 

I will do some testing with and without patch and see if above is true
or not.

Thanks
Vivek

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Vivek Goyal]

On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
> > On 21/06/10 21.49, Jeff Moyer wrote:
> > > Hi,
> > > 
> > > In testing a workload that has a single fsync-ing process and another
> > > process that does a sequential buffered read, I was unable to tune CFQ
> > > to reach the throughput of deadline.  This patch, along with the previous
> > > one, brought CFQ in line with deadline when setting slice_idle to 0.
> > > 
> > > I'm not sure what the original reason for not allowing sync and async
> > > I/O to be dispatched together was.  If there is a workload I should be
> > > testing that shows the inherent problems of this, please point me at it
> > > and I will resume testing.  Until and unless that workload is identified,
> > > please consider applying this patch.
> > 
> > The problematic case is/was a normal SATA drive with a buffered
> > writer and an occasional reader. I'll have to double check my
> > mail tomorrow, but iirc the issue was that the occasional reader
> > would suffer great latencies since service times for that single
> > IO would be delayed at the drive side. It could perhaps just be
> > a bug in how we handle the slice idling on the read side when the
> > IO gets delayed initially.
> > 
> > So if my memory is correct, google for the fsync madness and
> > interactiveness thread that we had some months ago and which
> > caused a lot of tweaking. The commit adding this is
> > 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
> > in July last year. So it was around that time that the mails went
> > around.
> 
> Hi Jens,
> 
> I suspect we might have introduced this patch because mike galbraith
> had issues which application interactiveness (reading data back from swap)
> in the prence of heavy writeout on SATA disk.
> 
> After this patch we did two enhancements.
> 
> - You introduced the logic of building write queue depth gradually.
> - Corrado introduced the logic of idling on the random reader service
>   tree.
> 
> In the past random reader were not protected from WRITES as there was no
> idling on random readers. But with corrado's changes of idling on
> sync-noidle service tree, I think this problem might have been solved to
> a great extent.
> 
> Getting rid of this exclusivity of either SYNC/ASYNC requests in request
> queue might help us with throughput on storage arrys without loosing
> protection for random reader on SATA. 
> 
> I will do some testing with and without patch and see if above is true
> or not.

Some primilinary testing results with and without patch. I started a
buffered writer and started firefox and monitored how much time firefox
took.

dd if=/dev/zero of=zerofile bs=4K count=1024M

2.6.35-rc3 vanilla
==================
real    0m22.546s
user    0m0.566s
sys     0m0.107s


real    0m21.410s
user    0m0.527s
sys     0m0.095s


real    0m27.594s
user    0m1.256s
sys     0m0.483s

2.6.35-rc3 + jeff's patches
===========================
real    0m20.372s
user    0m0.635s
sys     0m0.128s

real    0m22.281s
user    0m0.509s
sys     0m0.093s

real    0m23.211s
user    0m0.674s
sys     0m0.140s

So looks like firefox launching times have not changed much in the presence
of heavy buffered writting going on root disk. I will do more testing tomorrow.

Thanks
Vivek

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jeff Moyer]

Vivek Goyal <vgoyal@redhat.com> writes:

> On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
>> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
>> > On 21/06/10 21.49, Jeff Moyer wrote:
>> > > Hi,
>> > > 
>> > > In testing a workload that has a single fsync-ing process and another
>> > > process that does a sequential buffered read, I was unable to tune CFQ
>> > > to reach the throughput of deadline.  This patch, along with the previous
>> > > one, brought CFQ in line with deadline when setting slice_idle to 0.
>> > > 
>> > > I'm not sure what the original reason for not allowing sync and async
>> > > I/O to be dispatched together was.  If there is a workload I should be
>> > > testing that shows the inherent problems of this, please point me at it
>> > > and I will resume testing.  Until and unless that workload is identified,
>> > > please consider applying this patch.
>> > 
>> > The problematic case is/was a normal SATA drive with a buffered
>> > writer and an occasional reader. I'll have to double check my
>> > mail tomorrow, but iirc the issue was that the occasional reader
>> > would suffer great latencies since service times for that single
>> > IO would be delayed at the drive side. It could perhaps just be
>> > a bug in how we handle the slice idling on the read side when the
>> > IO gets delayed initially.
>> > 
>> > So if my memory is correct, google for the fsync madness and
>> > interactiveness thread that we had some months ago and which
>> > caused a lot of tweaking. The commit adding this is
>> > 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
>> > in July last year. So it was around that time that the mails went
>> > around.
>> 
>> Hi Jens,
>> 
>> I suspect we might have introduced this patch because mike galbraith
>> had issues which application interactiveness (reading data back from swap)
>> in the prence of heavy writeout on SATA disk.
>> 
>> After this patch we did two enhancements.
>> 
>> - You introduced the logic of building write queue depth gradually.
>> - Corrado introduced the logic of idling on the random reader service
>>   tree.
>> 
>> In the past random reader were not protected from WRITES as there was no
>> idling on random readers. But with corrado's changes of idling on
>> sync-noidle service tree, I think this problem might have been solved to
>> a great extent.
>> 
>> Getting rid of this exclusivity of either SYNC/ASYNC requests in request
>> queue might help us with throughput on storage arrys without loosing
>> protection for random reader on SATA. 
>> 
>> I will do some testing with and without patch and see if above is true
>> or not.
>
> Some primilinary testing results with and without patch. I started a
> buffered writer and started firefox and monitored how much time firefox
> took.
>
> dd if=/dev/zero of=zerofile bs=4K count=1024M
>
> 2.6.35-rc3 vanilla
> ==================
> real    0m22.546s
> user    0m0.566s
> sys     0m0.107s
>
>
> real    0m21.410s
> user    0m0.527s
> sys     0m0.095s
>
>
> real    0m27.594s
> user    0m1.256s
> sys     0m0.483s
>
> 2.6.35-rc3 + jeff's patches
> ===========================
> real    0m20.372s
> user    0m0.635s
> sys     0m0.128s
>
> real    0m22.281s
> user    0m0.509s
> sys     0m0.093s
>
> real    0m23.211s
> user    0m0.674s
> sys     0m0.140s
>
> So looks like firefox launching times have not changed much in the presence
> of heavy buffered writting going on root disk. I will do more testing tomorrow.

Was the buffered writer actually hitting disk?  How much memory is on
your system?

Cheers,
Jeff

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Vivek Goyal]

On Tue, Jun 22, 2010 at 08:45:54AM -0400, Jeff Moyer wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
> > On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
> >> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
> >> > On 21/06/10 21.49, Jeff Moyer wrote:
> >> > > Hi,
> >> > > 
> >> > > In testing a workload that has a single fsync-ing process and another
> >> > > process that does a sequential buffered read, I was unable to tune CFQ
> >> > > to reach the throughput of deadline.  This patch, along with the previous
> >> > > one, brought CFQ in line with deadline when setting slice_idle to 0.
> >> > > 
> >> > > I'm not sure what the original reason for not allowing sync and async
> >> > > I/O to be dispatched together was.  If there is a workload I should be
> >> > > testing that shows the inherent problems of this, please point me at it
> >> > > and I will resume testing.  Until and unless that workload is identified,
> >> > > please consider applying this patch.
> >> > 
> >> > The problematic case is/was a normal SATA drive with a buffered
> >> > writer and an occasional reader. I'll have to double check my
> >> > mail tomorrow, but iirc the issue was that the occasional reader
> >> > would suffer great latencies since service times for that single
> >> > IO would be delayed at the drive side. It could perhaps just be
> >> > a bug in how we handle the slice idling on the read side when the
> >> > IO gets delayed initially.
> >> > 
> >> > So if my memory is correct, google for the fsync madness and
> >> > interactiveness thread that we had some months ago and which
> >> > caused a lot of tweaking. The commit adding this is
> >> > 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
> >> > in July last year. So it was around that time that the mails went
> >> > around.
> >> 
> >> Hi Jens,
> >> 
> >> I suspect we might have introduced this patch because mike galbraith
> >> had issues which application interactiveness (reading data back from swap)
> >> in the prence of heavy writeout on SATA disk.
> >> 
> >> After this patch we did two enhancements.
> >> 
> >> - You introduced the logic of building write queue depth gradually.
> >> - Corrado introduced the logic of idling on the random reader service
> >>   tree.
> >> 
> >> In the past random reader were not protected from WRITES as there was no
> >> idling on random readers. But with corrado's changes of idling on
> >> sync-noidle service tree, I think this problem might have been solved to
> >> a great extent.
> >> 
> >> Getting rid of this exclusivity of either SYNC/ASYNC requests in request
> >> queue might help us with throughput on storage arrys without loosing
> >> protection for random reader on SATA. 
> >> 
> >> I will do some testing with and without patch and see if above is true
> >> or not.
> >
> > Some primilinary testing results with and without patch. I started a
> > buffered writer and started firefox and monitored how much time firefox
> > took.
> >
> > dd if=/dev/zero of=zerofile bs=4K count=1024M
> >
> > 2.6.35-rc3 vanilla
> > ==================
> > real    0m22.546s
> > user    0m0.566s
> > sys     0m0.107s
> >
> >
> > real    0m21.410s
> > user    0m0.527s
> > sys     0m0.095s
> >
> >
> > real    0m27.594s
> > user    0m1.256s
> > sys     0m0.483s
> >
> > 2.6.35-rc3 + jeff's patches
> > ===========================
> > real    0m20.372s
> > user    0m0.635s
> > sys     0m0.128s
> >
> > real    0m22.281s
> > user    0m0.509s
> > sys     0m0.093s
> >
> > real    0m23.211s
> > user    0m0.674s
> > sys     0m0.140s
> >
> > So looks like firefox launching times have not changed much in the presence
> > of heavy buffered writting going on root disk. I will do more testing tomorrow.
> 
> Was the buffered writer actually hitting disk?  How much memory is on
> your system?

I have 4G of memory in the system. I used to wait for 10-15 seconds after
writer has started and then launch firefox to make sure writes are actually
hitting the disk.

Are you seeing different results in your testing?

Thanks
Vivek

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jens Axboe]

On 2010-06-22 15:18, Vivek Goyal wrote:
> On Tue, Jun 22, 2010 at 08:45:54AM -0400, Jeff Moyer wrote:
>> Vivek Goyal <vgoyal@redhat.com> writes:
>>
>>> On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
>>>> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
>>>>> On 21/06/10 21.49, Jeff Moyer wrote:
>>>>>> Hi,
>>>>>>
>>>>>> In testing a workload that has a single fsync-ing process and another
>>>>>> process that does a sequential buffered read, I was unable to tune CFQ
>>>>>> to reach the throughput of deadline.  This patch, along with the previous
>>>>>> one, brought CFQ in line with deadline when setting slice_idle to 0.
>>>>>>
>>>>>> I'm not sure what the original reason for not allowing sync and async
>>>>>> I/O to be dispatched together was.  If there is a workload I should be
>>>>>> testing that shows the inherent problems of this, please point me at it
>>>>>> and I will resume testing.  Until and unless that workload is identified,
>>>>>> please consider applying this patch.
>>>>>
>>>>> The problematic case is/was a normal SATA drive with a buffered
>>>>> writer and an occasional reader. I'll have to double check my
>>>>> mail tomorrow, but iirc the issue was that the occasional reader
>>>>> would suffer great latencies since service times for that single
>>>>> IO would be delayed at the drive side. It could perhaps just be
>>>>> a bug in how we handle the slice idling on the read side when the
>>>>> IO gets delayed initially.
>>>>>
>>>>> So if my memory is correct, google for the fsync madness and
>>>>> interactiveness thread that we had some months ago and which
>>>>> caused a lot of tweaking. The commit adding this is
>>>>> 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
>>>>> in July last year. So it was around that time that the mails went
>>>>> around.
>>>>
>>>> Hi Jens,
>>>>
>>>> I suspect we might have introduced this patch because mike galbraith
>>>> had issues which application interactiveness (reading data back from swap)
>>>> in the prence of heavy writeout on SATA disk.
>>>>
>>>> After this patch we did two enhancements.
>>>>
>>>> - You introduced the logic of building write queue depth gradually.
>>>> - Corrado introduced the logic of idling on the random reader service
>>>>   tree.
>>>>
>>>> In the past random reader were not protected from WRITES as there was no
>>>> idling on random readers. But with corrado's changes of idling on
>>>> sync-noidle service tree, I think this problem might have been solved to
>>>> a great extent.
>>>>
>>>> Getting rid of this exclusivity of either SYNC/ASYNC requests in request
>>>> queue might help us with throughput on storage arrys without loosing
>>>> protection for random reader on SATA. 
>>>>
>>>> I will do some testing with and without patch and see if above is true
>>>> or not.
>>>
>>> Some primilinary testing results with and without patch. I started a
>>> buffered writer and started firefox and monitored how much time firefox
>>> took.
>>>
>>> dd if=/dev/zero of=zerofile bs=4K count=1024M
>>>
>>> 2.6.35-rc3 vanilla
>>> ==================
>>> real    0m22.546s
>>> user    0m0.566s
>>> sys     0m0.107s
>>>
>>>
>>> real    0m21.410s
>>> user    0m0.527s
>>> sys     0m0.095s
>>>
>>>
>>> real    0m27.594s
>>> user    0m1.256s
>>> sys     0m0.483s
>>>
>>> 2.6.35-rc3 + jeff's patches
>>> ===========================
>>> real    0m20.372s
>>> user    0m0.635s
>>> sys     0m0.128s
>>>
>>> real    0m22.281s
>>> user    0m0.509s
>>> sys     0m0.093s
>>>
>>> real    0m23.211s
>>> user    0m0.674s
>>> sys     0m0.140s
>>>
>>> So looks like firefox launching times have not changed much in the presence
>>> of heavy buffered writting going on root disk. I will do more testing tomorrow.
>>
>> Was the buffered writer actually hitting disk?  How much memory is on
>> your system?
> 
> I have 4G of memory in the system. I used to wait for 10-15 seconds after
> writer has started and then launch firefox to make sure writes are actually
> hitting the disk.
> 
> Are you seeing different results in your testing?

Just to be sure, this is a regular SATA drive that has NCQ enabled and
running? Apart from that comment, the test sounds good - dirty lots of
memory and ensure that it's writing, then start the reader. Should be
worst case for the reader. Sadly, both the before and after timings
are pretty horrible :-/

-- 
Jens Axboe

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Vivek Goyal]

On Tue, Jun 22, 2010 at 03:21:18PM +0200, Jens Axboe wrote:
> On 2010-06-22 15:18, Vivek Goyal wrote:
> > On Tue, Jun 22, 2010 at 08:45:54AM -0400, Jeff Moyer wrote:
> >> Vivek Goyal <vgoyal@redhat.com> writes:
> >>
> >>> On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
> >>>> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
> >>>>> On 21/06/10 21.49, Jeff Moyer wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> In testing a workload that has a single fsync-ing process and another
> >>>>>> process that does a sequential buffered read, I was unable to tune CFQ
> >>>>>> to reach the throughput of deadline.  This patch, along with the previous
> >>>>>> one, brought CFQ in line with deadline when setting slice_idle to 0.
> >>>>>>
> >>>>>> I'm not sure what the original reason for not allowing sync and async
> >>>>>> I/O to be dispatched together was.  If there is a workload I should be
> >>>>>> testing that shows the inherent problems of this, please point me at it
> >>>>>> and I will resume testing.  Until and unless that workload is identified,
> >>>>>> please consider applying this patch.
> >>>>>
> >>>>> The problematic case is/was a normal SATA drive with a buffered
> >>>>> writer and an occasional reader. I'll have to double check my
> >>>>> mail tomorrow, but iirc the issue was that the occasional reader
> >>>>> would suffer great latencies since service times for that single
> >>>>> IO would be delayed at the drive side. It could perhaps just be
> >>>>> a bug in how we handle the slice idling on the read side when the
> >>>>> IO gets delayed initially.
> >>>>>
> >>>>> So if my memory is correct, google for the fsync madness and
> >>>>> interactiveness thread that we had some months ago and which
> >>>>> caused a lot of tweaking. The commit adding this is
> >>>>> 5ad531db6e0f3c3c985666e83d3c1c4d53acccf9 and was added back
> >>>>> in July last year. So it was around that time that the mails went
> >>>>> around.
> >>>>
> >>>> Hi Jens,
> >>>>
> >>>> I suspect we might have introduced this patch because mike galbraith
> >>>> had issues which application interactiveness (reading data back from swap)
> >>>> in the prence of heavy writeout on SATA disk.
> >>>>
> >>>> After this patch we did two enhancements.
> >>>>
> >>>> - You introduced the logic of building write queue depth gradually.
> >>>> - Corrado introduced the logic of idling on the random reader service
> >>>>   tree.
> >>>>
> >>>> In the past random reader were not protected from WRITES as there was no
> >>>> idling on random readers. But with corrado's changes of idling on
> >>>> sync-noidle service tree, I think this problem might have been solved to
> >>>> a great extent.
> >>>>
> >>>> Getting rid of this exclusivity of either SYNC/ASYNC requests in request
> >>>> queue might help us with throughput on storage arrys without loosing
> >>>> protection for random reader on SATA. 
> >>>>
> >>>> I will do some testing with and without patch and see if above is true
> >>>> or not.
> >>>
> >>> Some primilinary testing results with and without patch. I started a
> >>> buffered writer and started firefox and monitored how much time firefox
> >>> took.
> >>>
> >>> dd if=/dev/zero of=zerofile bs=4K count=1024M
> >>>
> >>> 2.6.35-rc3 vanilla
> >>> ==================
> >>> real    0m22.546s
> >>> user    0m0.566s
> >>> sys     0m0.107s
> >>>
> >>>
> >>> real    0m21.410s
> >>> user    0m0.527s
> >>> sys     0m0.095s
> >>>
> >>>
> >>> real    0m27.594s
> >>> user    0m1.256s
> >>> sys     0m0.483s
> >>>
> >>> 2.6.35-rc3 + jeff's patches
> >>> ===========================
> >>> real    0m20.372s
> >>> user    0m0.635s
> >>> sys     0m0.128s
> >>>
> >>> real    0m22.281s
> >>> user    0m0.509s
> >>> sys     0m0.093s
> >>>
> >>> real    0m23.211s
> >>> user    0m0.674s
> >>> sys     0m0.140s
> >>>
> >>> So looks like firefox launching times have not changed much in the presence
> >>> of heavy buffered writting going on root disk. I will do more testing tomorrow.
> >>
> >> Was the buffered writer actually hitting disk?  How much memory is on
> >> your system?
> > 
> > I have 4G of memory in the system. I used to wait for 10-15 seconds after
> > writer has started and then launch firefox to make sure writes are actually
> > hitting the disk.
> > 
> > Are you seeing different results in your testing?
> 
> Just to be sure, this is a regular SATA drive that has NCQ enabled and
> running? Apart from that comment, the test sounds good - dirty lots of
> memory and ensure that it's writing, then start the reader. Should be
> worst case for the reader. Sadly, both the before and after timings
> are pretty horrible :-/

This is Western Digital SATA disk. It has NCQ enabled. I see 31 in
/sys/block/<dev>/device/queue_depth. Is there another way to verify that.

I also did a blktrace and I see that many a times we are driving deeper
queue depths. So it confirms that this drive has NCQ enabled.

8,64   0        0     2.530368587     0  m   N cfq2024A / activate rq, drv=16
8,64   0        0     2.531402018     0  m   N cfq2024A / activate rq, drv=16
8,64   0        0     2.534765229     0  m   N cfq2024A / activate rq, drv=16

Setting the queue_depth=1 helps a bit and firefox launches in around 17-18
seconds.

Without any competing workload, firefox launches in around 6 seconds.

I guess the best way to deal with SATA disk is to set queue_depth=1 in 
/sys.

Thanks
Vivek

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jeff Moyer]

Vivek Goyal <vgoyal@redhat.com> writes:

> On Tue, Jun 22, 2010 at 08:45:54AM -0400, Jeff Moyer wrote:
>> Vivek Goyal <vgoyal@redhat.com> writes:
>> 
>> > On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
>> > So looks like firefox launching times have not changed much in the presence
>> > of heavy buffered writting going on root disk. I will do more testing tomorrow.
>> 
>> Was the buffered writer actually hitting disk?  How much memory is on
>> your system?
>
> I have 4G of memory in the system. I used to wait for 10-15 seconds after
> writer has started and then launch firefox to make sure writes are actually
> hitting the disk.
>
> Are you seeing different results in your testing?

No, I hadn't got to testing this yet.  I was just making sure the test
procedure was sane (and it is).  Thanks for doing the testing!

-Jeff

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jeff Moyer]

Vivek Goyal <vgoyal@redhat.com> writes:

> On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
>> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
>> > On 21/06/10 21.49, Jeff Moyer wrote:
>> > > Hi,
>> > > 
>> > > In testing a workload that has a single fsync-ing process and another
>> > > process that does a sequential buffered read, I was unable to tune CFQ
>> > > to reach the throughput of deadline.  This patch, along with the previous
>> > > one, brought CFQ in line with deadline when setting slice_idle to 0.
>> > > 
>> > > I'm not sure what the original reason for not allowing sync and async
>> > > I/O to be dispatched together was.  If there is a workload I should be
>> > > testing that shows the inherent problems of this, please point me at it
>> > > and I will resume testing.  Until and unless that workload is identified,
>> > > please consider applying this patch.
>> > 
>> > The problematic case is/was a normal SATA drive with a buffered
>> > writer and an occasional reader. I'll have to double check my
>> > mail tomorrow, but iirc the issue was that the occasional reader
>> > would suffer great latencies since service times for that single
>> > IO would be delayed at the drive side. It could perhaps just be
>> > a bug in how we handle the slice idling on the read side when the
>> > IO gets delayed initially.
>> > 

[...]

> Some primilinary testing results with and without patch. I started a
> buffered writer and started firefox and monitored how much time firefox
> took.
>
> dd if=/dev/zero of=zerofile bs=4K count=1024M
>
> 2.6.35-rc3 vanilla
> ==================
> real    0m22.546s
> user    0m0.566s
> sys     0m0.107s
>
>
> real    0m21.410s
> user    0m0.527s
> sys     0m0.095s
>
>
> real    0m27.594s
> user    0m1.256s
> sys     0m0.483s
>
> 2.6.35-rc3 + jeff's patches
> ===========================
> real    0m20.372s
> user    0m0.635s
> sys     0m0.128s
>
> real    0m22.281s
> user    0m0.509s
> sys     0m0.093s
>
> real    0m23.211s
> user    0m0.674s
> sys     0m0.140s
>
> So looks like firefox launching times have not changed much in the presence
> of heavy buffered writting going on root disk. I will do more testing tomorrow.

Jens,

What are your thoughts on this?  Can we merge it?

Cheers,
Jeff

Re: [PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero

[Jeff Moyer]

Jens Axboe <axboe@kernel.dk> writes:

> On 21/06/10 21.49, Jeff Moyer wrote:
>> Hi,
>> 
>> In testing a competing fsync-ing process and a sequential reader on
>> mid-grade storage, I found that cfq was incapable of achieving the I/O
>> rates of deadline, no matter how it was tuned.  Investigation, and insight
>> from Vivek (mostly the latter), led to identifying that we were still
>> idling for the last queue in the service tree.
>> 
>> Modifying cfq_should_idle to not idle when slice_idle is set to zero got
>> us much closer to the performance of deadline for this workload.  I have
>> one follow-on patch that gets us on-par with deadline, but I think this
>> patch stands alone.
>> 
>> Comments, as always, are appreciated.
>
> This looks good.

So.... applied to which branch?

Thanks!
Jeff

Re: [PATCH 2/2] cfq: allow dispatching of both sync and async I/O together

[Jens Axboe]

On 28/06/10 20.40, Jeff Moyer wrote:
> Vivek Goyal <vgoyal@redhat.com> writes:
> 
>> On Mon, Jun 21, 2010 at 07:22:08PM -0400, Vivek Goyal wrote:
>>> On Mon, Jun 21, 2010 at 09:59:48PM +0200, Jens Axboe wrote:
>>>> On 21/06/10 21.49, Jeff Moyer wrote:
>>>>> Hi,
>>>>>
>>>>> In testing a workload that has a single fsync-ing process and another
>>>>> process that does a sequential buffered read, I was unable to tune CFQ
>>>>> to reach the throughput of deadline.  This patch, along with the previous
>>>>> one, brought CFQ in line with deadline when setting slice_idle to 0.
>>>>>
>>>>> I'm not sure what the original reason for not allowing sync and async
>>>>> I/O to be dispatched together was.  If there is a workload I should be
>>>>> testing that shows the inherent problems of this, please point me at it
>>>>> and I will resume testing.  Until and unless that workload is identified,
>>>>> please consider applying this patch.
>>>>
>>>> The problematic case is/was a normal SATA drive with a buffered
>>>> writer and an occasional reader. I'll have to double check my
>>>> mail tomorrow, but iirc the issue was that the occasional reader
>>>> would suffer great latencies since service times for that single
>>>> IO would be delayed at the drive side. It could perhaps just be
>>>> a bug in how we handle the slice idling on the read side when the
>>>> IO gets delayed initially.
>>>>
> 
> [...]
> 
>> Some primilinary testing results with and without patch. I started a
>> buffered writer and started firefox and monitored how much time firefox
>> took.
>>
>> dd if=/dev/zero of=zerofile bs=4K count=1024M
>>
>> 2.6.35-rc3 vanilla
>> ==================
>> real    0m22.546s
>> user    0m0.566s
>> sys     0m0.107s
>>
>>
>> real    0m21.410s
>> user    0m0.527s
>> sys     0m0.095s
>>
>>
>> real    0m27.594s
>> user    0m1.256s
>> sys     0m0.483s
>>
>> 2.6.35-rc3 + jeff's patches
>> ===========================
>> real    0m20.372s
>> user    0m0.635s
>> sys     0m0.128s
>>
>> real    0m22.281s
>> user    0m0.509s
>> sys     0m0.093s
>>
>> real    0m23.211s
>> user    0m0.674s
>> sys     0m0.140s
>>
>> So looks like firefox launching times have not changed much in the presence
>> of heavy buffered writting going on root disk. I will do more testing tomorrow.
> 
> Jens,
> 
> What are your thoughts on this?  Can we merge it?

I'll add it to the .36 testing mix. I will re-run my older tests on the
end result, I really don't want to regress on the latency side. The above
numbers look OK.

-- 
Jens Axboe

Re: [PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero

[Jens Axboe]

On 28/06/10 20.41, Jeff Moyer wrote:
> Jens Axboe <axboe@kernel.dk> writes:
> 
>> On 21/06/10 21.49, Jeff Moyer wrote:
>>> Hi,
>>>
>>> In testing a competing fsync-ing process and a sequential reader on
>>> mid-grade storage, I found that cfq was incapable of achieving the I/O
>>> rates of deadline, no matter how it was tuned.  Investigation, and insight
>>> from Vivek (mostly the latter), led to identifying that we were still
>>> idling for the last queue in the service tree.
>>>
>>> Modifying cfq_should_idle to not idle when slice_idle is set to zero got
>>> us much closer to the performance of deadline for this workload.  I have
>>> one follow-on patch that gets us on-par with deadline, but I think this
>>> patch stands alone.
>>>
>>> Comments, as always, are appreciated.
>>
>> This looks good.
> 
> So.... applied to which branch?

Not applied yet, unless I explicitly say it's applied, then it's not
necessarily in any public git tree yet. But this will go into .35.

-- 
Jens Axboe

Re: [PATCH 1/2] cfq: always return false from should_idle if slice_idle is set to zero

[Jeff Moyer]

Jens Axboe <axboe@kernel.dk> writes:

> On 28/06/10 20.41, Jeff Moyer wrote:
>> Jens Axboe <axboe@kernel.dk> writes:
>> 
>>> On 21/06/10 21.49, Jeff Moyer wrote:
>>>> Hi,
>>>>
>>>> In testing a competing fsync-ing process and a sequential reader on
>>>> mid-grade storage, I found that cfq was incapable of achieving the I/O
>>>> rates of deadline, no matter how it was tuned.  Investigation, and insight
>>>> from Vivek (mostly the latter), led to identifying that we were still
>>>> idling for the last queue in the service tree.
>>>>
>>>> Modifying cfq_should_idle to not idle when slice_idle is set to zero got
>>>> us much closer to the performance of deadline for this workload.  I have
>>>> one follow-on patch that gets us on-par with deadline, but I think this
>>>> patch stands alone.
>>>>
>>>> Comments, as always, are appreciated.
>>>
>>> This looks good.
>> 
>> So.... applied to which branch?
>
> Not applied yet, unless I explicitly say it's applied, then it's not
> necessarily in any public git tree yet. But this will go into .35.

OK, thanks for the clarification.

Cheers,
Jeff