Re: Cache queue_congestion_on/off_threshold

Jens Axboe
2004-05-10T14:30:37Z

On Fri, May 07 2004, Chen, Kenneth W wrote:
> >>>> Jens Axboe wrote on Friday, May 07, 2004 2:39 AM
> > On Thu, May 06 2004, Chen, Kenneth W wrote:
> > > (3) can we allocate request structure up front in __make_request?
> > > For I/O that cannot be merged, the elevator code executes twice
> > > in __make_request.
> > >
> >
> > Actually, with the good working batching we might get away with killing
> > freereq completely. Have you tested that (if not, could you?)
>
> Sorry, I'm clueless on "good working batching". If you could please give
> me some pointers, I will definitely test it.

Something like this.

--- linux-2.6.6/drivers/block/ll_rw_blk.c~ 2004-05-10 16:23:45.684726955 +0200
+++ linux-2.6.6/drivers/block/ll_rw_blk.c 2004-05-10 16:29:04.333792268 +0200
@@ -2138,8 +2138,8 @@

static int __make_request(request_queue_t *q, struct bio *bio)
{
- struct request *req, *freereq = NULL;
int el_ret, rw, nr_sectors, cur_nr_sectors, barrier, ra;
+ struct request *req;
sector_t sector;

sector = bio->bi_sector;
@@ -2161,15 +2161,15 @@

ra = bio->bi_rw & (1 << BIO_RW_AHEAD);

-again:
+ if (barrier)
+ goto get_rq_nolock;
+
spin_lock_irq(q->queue_lock);

if (elv_queue_empty(q)) {
blk_plug_device(q);
goto get_rq;
}
- if (barrier)
- goto get_rq;

el_ret = elv_merge(q, &req, bio);
switch (el_ret) {
@@ -2230,21 +2230,17 @@
* a free slot.
*/
get_rq:
- if (freereq) {
- req = freereq;
- freereq = NULL;
- } else {
- spin_unlock_irq(q->queue_lock);
- if ((freereq = get_request(q, rw, GFP_ATOMIC)) == NULL) {
- /*
- * READA bit set
- */
- if (ra)
- goto end_io;
+ spin_unlock_irq(q->queue_lock);
+get_rq_nolock:
+ req = get_request(q, rw, GFP_ATOMIC);
+ if (!req) {
+ /*
+ * READA bit set
+ */
+ if (ra)
+ goto end_io;

- freereq = get_request_wait(q, rw);
- }
- goto again;
+ req = get_request_wait(q, rw);
}

req->flags |= REQ_CMD;
@@ -2276,10 +2272,9 @@
req->rq_disk = bio->bi_bdev->bd_disk;
req->start_time = jiffies;

+ spin_lock_irq(q->queue_lock);
add_request(q, req);
out:
- if (freereq)
- __blk_put_request(q, freereq);

if (blk_queue_plugged(q)) {
int nrq = q->rq.count[READ] + q->rq.count[WRITE] - q->in_flight;

--
Jens Axboe