Re: 2.6.20-rc2+: CFQ halving disk throughput.

From: Jens Axboe
Date: Tue Jan 02 2007 - 06:58:22 EST


On Tue, Jan 02 2007, Rene Herman wrote:
> Jens Axboe wrote:
>
> >On Mon, Jan 01 2007, Andrew Morton wrote:
>
> >>The patch would appear to need this fix:
> >>
> >>--- a/block/cfq-iosched.c~a
> >>+++ a/block/cfq-iosched.c
> >>@@ -592,7 +592,7 @@ static int cfq_allow_merge(request_queue
> >> if (cfqq == RQ_CFQQ(rq))
> >> return 1;
> >>
> >>- return 1;
> >>+ return 0;
> >> }
> >>
> >> static inline void
> >>_
> >>
> >>But that might not fix things...
> >
> >Yeah it is, but I don't think it'll fix it (if anything, it'll be more
> >conservative).
>
> (to possibly save others from trying -- no, doesn't fix any)

As expected. The issue is rq_is_sync(rq) takes the data direction into
account as well, while bio_sync() only checks the sync bit. This should
fix it.

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 4b4217d..26fb40f 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -574,12 +574,14 @@ static int cfq_allow_merge(request_queue
struct cfq_data *cfqd = q->elevator->elevator_data;
const int rw = bio_data_dir(bio);
struct cfq_queue *cfqq;
+ int bio_is_sync;
pid_t key;

/*
* Disallow merge, if bio and rq aren't both sync or async
*/
- if (!!bio_sync(bio) != !!rq_is_sync(rq))
+ bio_is_sync = bio_data_dir(bio) == READ || bio_sync(bio);
+ if (bio_is_sync != !!rq_is_sync(rq))
return 0;

/*
@@ -592,7 +594,7 @@ static int cfq_allow_merge(request_queue
if (cfqq == RQ_CFQQ(rq))
return 1;

- return 1;
+ return 0;
}


--
Jens Axboe

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/