mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Adam Kropelin <akropel1@rochester.rr.com>
To: Jens Axboe <axboe@suse.de>, Charles.White@compaq.com
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] Fix cpqarray on 2.5
Date: Sun, 15 Sep 2002 09:53:05 -0400	[thread overview]
Message-ID: <20020915135305.GA22713@www.kroptech.com> (raw)

Jens & Charles,

The attached patch fixes some critical bugs in cpqarray in 2.5. One of the fixes
essentially backs out the block queue stop/start behavior that was added
recently. This code as it stands is buggy and locks up under even light SMP
workloads. Certainly we want the performance benefits of proper block queue
plugging, but the driver needs some work before it will fit nicely.

Some of these fixes do theoretically hurt performance, but when you consider
that the driver is unusable under SMP as-is, I think it is right to get
correctness first.

Specifically, this patch does the following:

* Adds locking to proc queue-walking code for debugging use. Note that the proc
registration is still broken and I've left it that way since this stuff should
probably migrate to driverfs anyway.

* Moves interrupt enabling so queue lock is initialized before interrupts are
enabled. Otherwise if we get a quick interrupt we oops the machine.

* Removes unconditional IRQ enabling in do_ida_request(). The block layer takes
the spinlock with irq_save so if we're going to play this trick then we need to
irq_restore. For now, just eliminate the unlocked region.

* Remove block queue stop/start logic since it can leave the queue stopped with
no outstanding completions to start it again. Plugging logic can come back but
it should go hand-in-hand with a cleanup of the driver's request handling
algorithm. If nobody screams about this patch I'll go ahead and start making
those improvments.

Patch is against 2.5.34 but should apply to any vaguely recent 2.5.

--Adam

--- /mnt/linux-2.5.34/drivers/block/cpqarray.c	Mon Sep  9 14:28:31 2002
+++ linux-2.5.34-mm1/drivers/block/cpqarray.c	Sat Sep 14 14:03:35 2002
@@ -202,6 +202,7 @@
 	drv_info_t *drv;
 #ifdef CPQ_PROC_PRINT_QUEUES
 	cmdlist_t *c;
+	unsigned long flags;
 #endif
 
 	ctlr = h->ctlr;
@@ -238,6 +239,7 @@
 	}
 
 #ifdef CPQ_PROC_PRINT_QUEUES
+	spin_lock_irqsave(IDA_LOCK(h->ctlr), flags); 
 	size = sprintf(buffer+len, "\nCurrent Queues:\n");
 	pos += size; len += size;
 
@@ -260,6 +262,7 @@
 	}
 
 	size = sprintf(buffer+len, "\n"); pos += size; len += size;
+	spin_unlock_irqrestore(IDA_LOCK(h->ctlr), flags); 
 #endif
 	size = sprintf(buffer+len, "nr_allocs = %d\nnr_frees = %d\n",
 			h->nr_allocs, h->nr_frees);
@@ -414,8 +417,6 @@
 		getgeometry(i);
 		start_fwbk(i); 
 
-		hba[i]->access.set_intr_mask(hba[i], FIFO_NOT_EMPTY);
-
 		ida_procinit(i);
 
 		q = BLK_DEFAULT_QUEUE(MAJOR_NR + i);
@@ -436,6 +437,9 @@
 		hba[i]->timer.function = ida_timer;
 		add_timer(&hba[i]->timer);
 
+		/* Enable IRQ now that spinlock and rate limit timer are set up */
+		hba[i]->access.set_intr_mask(hba[i], FIFO_NOT_EMPTY);
+
 		for(j=0; j<NWD; j++) {
 			struct gendisk *disk = ida_gendisk + i*NWD + j;
 			drv_info_t *drv = &hba[i]->drv[j];
@@ -832,8 +836,6 @@
 
 	blkdev_dequeue_request(creq);
 
-	spin_unlock_irq(q->queue_lock);
-
 	c->ctlr = h->ctlr;
 	c->hdr.unit = minor(creq->rq_dev) >> NWD_SHIFT;
 	c->hdr.size = sizeof(rblk_t) >> 2;
@@ -865,8 +867,6 @@
 	c->req.hdr.cmd = (rq_data_dir(creq) == READ) ? IDA_READ : IDA_WRITE;
 	c->type = CMD_RWREQ;
 
-	spin_lock_irq(q->queue_lock);
-
 	/* Put the request on the tail of the request queue */
 	addQ(&h->reqQ, c);
 	h->Qdepth++;
@@ -876,7 +876,6 @@
 	goto queue_next;
 
 startio:
-	__blk_stop_queue(q);
 	start_io(h);
 }
 
@@ -1027,8 +1026,8 @@
 	/*
 	 * See if we can queue up some more IO
 	 */
-	spin_unlock_irqrestore(IDA_LOCK(h->ctlr), flags);
-	blk_start_queue(BLK_DEFAULT_QUEUE(MAJOR_NR + h->ctlr));
+	do_ida_request(BLK_DEFAULT_QUEUE(MAJOR_NR+h->ctlr));
+	spin_unlock_irqrestore(IDA_LOCK(h->ctlr), flags); 
 }
 
 /*


                 reply	other threads:[~2002-09-15 13:48 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20020915135305.GA22713@www.kroptech.com \
    --to=akropel1@rochester.rr.com \
    --cc=Charles.White@compaq.com \
    --cc=axboe@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®