Messages in this thread Patch in this message | | | Date | Fri, 26 Nov 2004 09:07:34 +0100 | From | Jens Axboe <> | Subject | Re: 2.6.10-rc2-bk7 - Badness in cfq_put_request at drivers/block/cfq-iosched.c:1402 |
| |
On Fri, Nov 26 2004, Jens Axboe wrote: > On Thu, Nov 25 2004, Jens Axboe wrote: > > On Wed, Nov 24 2004, Christopher S. Aker wrote: > > > > Can you try this simple check to see if it triggers anything? > > > > > > > > ===== cfq-iosched.c 1.13 vs edited ===== > > > > --- 1.13/drivers/block/cfq-iosched.c 2004-10-30 01:35:21 +02:00 > > > > +++ edited/cfq-iosched.c 2004-11-24 14:40:13 +01:00 > > > > @@ -1389,6 +1389,8 @@ > > > > struct cfq_data *cfqd = q->elevator->elevator_data; > > > > struct cfq_rq *crq = RQ_DATA(rq); > > > > > > > > + WARN_ON(!spin_is_locked(q->queue_lock)); > > > > + > > > > if (crq) { > > > > struct cfq_queue *cfqq = crq->cfq_queue; > > > > > > I'd be happy to, but I won't have a free machine for a couple of days. > > > I'll can probably give it a shot during the weekend... > > > > Nevermind, here's a patch to fix it. I was so focused on the decrement > > side of things that I forgot to check the increment, pretty silly error > > really. > > And a small fix is needed on top of that, in case of hash type changes > (writing to hash_key).
... And that one relied on another hashing change I made locally. 3rd time is the charm,.
Signed-off-by: Jens Axboe <axboe@suse.de>
===== drivers/block/cfq-iosched.c 1.13 vs edited ===== --- 1.13/drivers/block/cfq-iosched.c 2004-10-30 01:35:21 +02:00 +++ edited/drivers/block/cfq-iosched.c 2004-11-26 09:06:38 +01:00 @@ -1398,10 +1398,7 @@ if (crq->io_context) put_io_context(crq->io_context->ioc); - if (!cfqq->allocated[crq->is_write]) { - WARN_ON(1); - cfqq->allocated[crq->is_write] = 1; - } + BUG_ON(!cfqq->allocated[crq->is_write]); cfqq->allocated[crq->is_write]--; mempool_free(crq, cfqd->crq_pool); @@ -1421,7 +1418,7 @@ struct cfq_data *cfqd = q->elevator->elevator_data; struct cfq_io_context *cic; const int rw = rq_data_dir(rq); - struct cfq_queue *cfqq; + struct cfq_queue *cfqq, *saved_cfqq; struct cfq_rq *crq; unsigned long flags; @@ -1439,20 +1436,30 @@ #endif } +repeat: if (cfqq->allocated[rw] >= cfqd->max_queued) goto out_lock; + cfqq->allocated[rw]++; spin_unlock_irqrestore(q->queue_lock, flags); /* - * if hashing type has changed, the cfq_queue might change here. we - * don't bother rechecking ->allocated since it should be a rare - * event + * if hashing type has changed, the cfq_queue might change here. */ + saved_cfqq = cfqq; cic = cfq_get_io_context(&cfqq, gfp_mask); if (!cic) goto err; + /* + * repeat allocation checks on queue change + */ + if (unlikely(saved_cfqq != cfqq)) { + spin_lock_irqsave(q->queue_lock, flags); + saved_cfqq->allocated[rw]--; + goto repeat; + } + crq = mempool_alloc(cfqd->crq_pool, gfp_mask); if (crq) { RB_CLEAR(&crq->rb_node); @@ -1465,7 +1472,6 @@ crq->in_flight = crq->accounted = crq->is_sync = 0; crq->is_write = rw; rq->elevator_private = crq; - cfqq->allocated[rw]++; cfqq->alloc_limit[rw] = 0; return 0; } @@ -1473,6 +1479,7 @@ put_io_context(cic->ioc); err: spin_lock_irqsave(q->queue_lock, flags); + cfqq->allocated[rw]--; cfq_put_queue(cfqq); out_lock: spin_unlock_irqrestore(q->queue_lock, flags); -- Jens Axboe
- 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/
| |