CFQ v2 high cpu load fix(?)

CFQ v2 high cpu load fix(?)

[Ronny V. Vindenes]

[-- Attachment #1: Type: text/plain, Size: 562 bytes --]

CFQ v2 is much better in a lot of cases, but certain situations trigger
a cpu load so high it starves the rest of the system thus completely
ruining the interactive experience. While casually looking at the
problem, I stumbled upon a patch by Arjan van de Ven sent to lkml on
sept. 1 (Subject: block fixes). Part of it is already included in the
CFQ v2 patches and after applying the rest[1] I'm no longer able to
trigger the problem.

[1] Patch attached against 2.6.9-rc4-ck1 but applies to rc4-mm1 with
some minor fuzz.

-- 
Ronny V. Vindenes <s864@ii.uib.no>

[-- Attachment #2: block-fix.patch --]
[-- Type: text/x-patch, Size: 1085 bytes --]

--- patches/linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 12:25:09.798003278 +0200
+++ linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 12:25:42.959479479 +0200
@@ -100,7 +100,7 @@
 		nr = q->nr_requests;
 	q->nr_congestion_on = nr;
 
-	nr = q->nr_requests - (q->nr_requests / 8) - 1;
+	nr = q->nr_requests - (q->nr_requests / 8) - (q->nr_requests/16)- 1;
 	if (nr < 1)
 		nr = 1;
 	q->nr_congestion_off = nr;
@@ -1758,8 +1758,10 @@
 {
 	DEFINE_WAIT(wait);
 	struct request *rq;
+	struct io_context *ioc;
 
 	generic_unplug_device(q);
+	ioc = get_io_context(GFP_NOIO);
 	do {
 		struct request_list *rl = &q->rq;
 
@@ -1769,7 +1771,6 @@
 		rq = get_request(q, rw, GFP_NOIO);
 
 		if (!rq) {
-			struct io_context *ioc;
 
 			io_schedule();
 
@@ -1779,12 +1780,11 @@
 			 * up to a big batch of them for a small period time.
 			 * See ioc_batching, ioc_set_batching
 			 */
-			ioc = get_io_context(GFP_NOIO);
 			ioc_set_batching(q, ioc);
-			put_io_context(ioc);
 		}
 		finish_wait(&rl->wait[rw], &wait);
 	} while (!rq);
+	put_io_context(ioc);
 
 	return rq;
 }

Re: CFQ v2 high cpu load fix(?)

[Nick Piggin]

Ronny V. Vindenes wrote:
> CFQ v2 is much better in a lot of cases, but certain situations trigger
> a cpu load so high it starves the rest of the system thus completely
> ruining the interactive experience. While casually looking at the
> problem, I stumbled upon a patch by Arjan van de Ven sent to lkml on
> sept. 1 (Subject: block fixes). Part of it is already included in the
> CFQ v2 patches and after applying the rest[1] I'm no longer able to
> trigger the problem.
> 
> [1] Patch attached against 2.6.9-rc4-ck1 but applies to rc4-mm1 with
> some minor fuzz.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- patches/linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 12:25:09.798003278 +0200
> +++ linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 12:25:42.959479479 +0200
> @@ -100,7 +100,7 @@
>  		nr = q->nr_requests;
>  	q->nr_congestion_on = nr;
>  
> -	nr = q->nr_requests - (q->nr_requests / 8) - 1;
> +	nr = q->nr_requests - (q->nr_requests / 8) - (q->nr_requests/16)- 1;
>  	if (nr < 1)
>  		nr = 1;
>  	q->nr_congestion_off = nr;


I thought this first hunk looked like a good idea when Arjan sent the
patch. Can you check if it alone helps your problem?

The second hunk should be basically a noop.

Re: CFQ v2 high cpu load fix(?)

[Jens Axboe]

On Tue, Oct 12 2004, Nick Piggin wrote:
> Ronny V. Vindenes wrote:
> >CFQ v2 is much better in a lot of cases, but certain situations trigger
> >a cpu load so high it starves the rest of the system thus completely
> >ruining the interactive experience. While casually looking at the
> >problem, I stumbled upon a patch by Arjan van de Ven sent to lkml on
> >sept. 1 (Subject: block fixes). Part of it is already included in the
> >CFQ v2 patches and after applying the rest[1] I'm no longer able to
> >trigger the problem.
> >
> >[1] Patch attached against 2.6.9-rc4-ck1 but applies to rc4-mm1 with
> >some minor fuzz.
> >
> >
> >
> >------------------------------------------------------------------------
> >
> >--- patches/linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 
> >12:25:09.798003278 +0200
> >+++ linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 
> >12:25:42.959479479 +0200
> >@@ -100,7 +100,7 @@
> > 		nr = q->nr_requests;
> > 	q->nr_congestion_on = nr;
> > 
> >-	nr = q->nr_requests - (q->nr_requests / 8) - 1;
> >+	nr = q->nr_requests - (q->nr_requests / 8) - (q->nr_requests/16)- 1;
> > 	if (nr < 1)
> > 		nr = 1;
> > 	q->nr_congestion_off = nr;
> 
> 
> I thought this first hunk looked like a good idea when Arjan sent the
> patch. Can you check if it alone helps your problem?

Yeah agree, it's a good idea to leave a bit of air between congestion on
and off. Fully explains the cfq v2 excessive sys time for some
workloads, which is extra nice.

> The second hunk should be basically a noop.

I don't see what it is trying to achieve, I like the current code
better.

-- 
Jens Axboe

Re: CFQ v2 high cpu load fix(?)

[Nick Piggin]

Jens Axboe wrote:
> On Tue, Oct 12 2004, Nick Piggin wrote:
> 
>>Ronny V. Vindenes wrote:
>>
>>>CFQ v2 is much better in a lot of cases, but certain situations trigger
>>>a cpu load so high it starves the rest of the system thus completely
>>>ruining the interactive experience. While casually looking at the
>>>problem, I stumbled upon a patch by Arjan van de Ven sent to lkml on
>>>sept. 1 (Subject: block fixes). Part of it is already included in the
>>>CFQ v2 patches and after applying the rest[1] I'm no longer able to
>>>trigger the problem.
>>>
>>>[1] Patch attached against 2.6.9-rc4-ck1 but applies to rc4-mm1 with
>>>some minor fuzz.
>>>
>>>
>>>
>>>------------------------------------------------------------------------
>>>
>>>--- patches/linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 
>>>12:25:09.798003278 +0200
>>>+++ linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 
>>>12:25:42.959479479 +0200
>>>@@ -100,7 +100,7 @@
>>>		nr = q->nr_requests;
>>>	q->nr_congestion_on = nr;
>>>
>>>-	nr = q->nr_requests - (q->nr_requests / 8) - 1;
>>>+	nr = q->nr_requests - (q->nr_requests / 8) - (q->nr_requests/16)- 1;
>>>	if (nr < 1)
>>>		nr = 1;
>>>	q->nr_congestion_off = nr;
>>
>>
>>I thought this first hunk looked like a good idea when Arjan sent the
>>patch. Can you check if it alone helps your problem?
> 
> 
> Yeah agree, it's a good idea to leave a bit of air between congestion on
> and off. Fully explains the cfq v2 excessive sys time for some
> workloads, which is extra nice.
> 

Cool. Can you queue up a patch for when -mm opens again, or shall I?
I can't imagine it should cause any problems but a bit of testing
would be wise.

> 
>>The second hunk should be basically a noop.
> 
> 
> I don't see what it is trying to achieve, I like the current code
> better.
> 

I think Arjan may have just misread the code a little bit.

Re: CFQ v2 high cpu load fix(?)

[Jens Axboe]

On Tue, Oct 12 2004, Nick Piggin wrote:
> Jens Axboe wrote:
> >On Tue, Oct 12 2004, Nick Piggin wrote:
> >
> >>Ronny V. Vindenes wrote:
> >>
> >>>CFQ v2 is much better in a lot of cases, but certain situations trigger
> >>>a cpu load so high it starves the rest of the system thus completely
> >>>ruining the interactive experience. While casually looking at the
> >>>problem, I stumbled upon a patch by Arjan van de Ven sent to lkml on
> >>>sept. 1 (Subject: block fixes). Part of it is already included in the
> >>>CFQ v2 patches and after applying the rest[1] I'm no longer able to
> >>>trigger the problem.
> >>>
> >>>[1] Patch attached against 2.6.9-rc4-ck1 but applies to rc4-mm1 with
> >>>some minor fuzz.
> >>>
> >>>
> >>>
> >>>------------------------------------------------------------------------
> >>>
> >>>--- patches/linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 
> >>>12:25:09.798003278 +0200
> >>>+++ linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c	2004-10-12 
> >>>12:25:42.959479479 +0200
> >>>@@ -100,7 +100,7 @@
> >>>		nr = q->nr_requests;
> >>>	q->nr_congestion_on = nr;
> >>>
> >>>-	nr = q->nr_requests - (q->nr_requests / 8) - 1;
> >>>+	nr = q->nr_requests - (q->nr_requests / 8) - (q->nr_requests/16)- 1;
> >>>	if (nr < 1)
> >>>		nr = 1;
> >>>	q->nr_congestion_off = nr;
> >>
> >>
> >>I thought this first hunk looked like a good idea when Arjan sent the
> >>patch. Can you check if it alone helps your problem?
> >
> >
> >Yeah agree, it's a good idea to leave a bit of air between congestion on
> >and off. Fully explains the cfq v2 excessive sys time for some
> >workloads, which is extra nice.
> >
> 
> Cool. Can you queue up a patch for when -mm opens again, or shall I?
> I can't imagine it should cause any problems but a bit of testing
> would be wise.

Testing is always good, but maybe Andrew can take it now but just not
feed to Linus until after 2.6.9?

Jens

Re: CFQ v2 high cpu load fix(?)

[Nick Piggin]

Jens Axboe wrote:
> On Tue, Oct 12 2004, Nick Piggin wrote:

>>Cool. Can you queue up a patch for when -mm opens again, or shall I?
>>I can't imagine it should cause any problems but a bit of testing
>>would be wise.
> 
> 
> Testing is always good, but maybe Andrew can take it now but just not
> feed to Linus until after 2.6.9?
> 

If Andrew doesn't mind, then yeah that would be nice.

Re: CFQ v2 high cpu load fix(?)

[Prakash K. Cheemplavam]

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

Nick Piggin schrieb:
> Jens Axboe wrote:
> 
>> On Tue, Oct 12 2004, Nick Piggin wrote:
>>
>>> Ronny V. Vindenes wrote:
>>>
>>>>
>>>> --- patches/linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c    
>>>> 2004-10-12 12:25:09.798003278 +0200
>>>> +++ linux-2.6.9-rc4-ck1/drivers/block/ll_rw_blk.c    2004-10-12 
>>>> 12:25:42.959479479 +0200
>>>> @@ -100,7 +100,7 @@
>>>>         nr = q->nr_requests;
>>>>     q->nr_congestion_on = nr;
>>>>
>>>> -    nr = q->nr_requests - (q->nr_requests / 8) - 1;
>>>> +    nr = q->nr_requests - (q->nr_requests / 8) - 
>>>> (q->nr_requests/16)- 1;
>>>>     if (nr < 1)
>>>>         nr = 1;
>>>>     q->nr_congestion_off = nr;
>>>
>>>
>>>
>>> I thought this first hunk looked like a good idea when Arjan sent the
>>> patch. Can you check if it alone helps your problem?
>>
>>
>>
>> Yeah agree, it's a good idea to leave a bit of air between congestion on
>> and off. Fully explains the cfq v2 excessive sys time for some
>> workloads, which is extra nice.
>>
> 
> Cool. Can you queue up a patch for when -mm opens again, or shall I?
> I can't imagine it should cause any problems but a bit of testing
> would be wise.

Just as a feedback: I applied the first hunk of that patch to 
2.6.9-rc4-ck1 and it really makes a difference. At first I thought the 
staircase scheduler was responsible for io starvation sometimes, but now 
this behaviour is gone. Very well!

Prakash

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]