lkml.org 
[lkml]   [2004]   [Nov]   [27]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
DateFri, 26 Nov 2004 09:07:34 +0100
FromJens Axboe <>
SubjectRe: 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/

\
 
 \ /
  Last update: 2005-03-22 14:08    [from the cache]
©2003-2008