mtd/drivers/mtd/nand nand_base.c,1.143,1.144

[gleixner at infradead.org]

Update of /home/cvs/mtd/drivers/mtd/nand
In directory phoenix.infradead.org:/tmp/cvs-serv32473/drivers/mtd/nand

Modified Files:
	nand_base.c 
Log Message:
[MTD] NAND: Reorganize chip locking

The code was wrong in several aspects. The locking order was
inconsistent, the device aquire code did not reset a variable
after a wakeup and the wakeup handling was not working for
applications where multiple chips are sharing a single
hardware controller.
When a hardware controller is available the locking is now
reduced to the hardware controller lock and the waitqueue is
moved to the hardware controller structure in order to avoid
a wake_up_all().

The problem was pointed out by Ben Dooks, who also found the
missing variable reset as main cause for his deadlock problem.



Index: nand_base.c
===================================================================
RCS file: /home/cvs/mtd/drivers/mtd/nand/nand_base.c,v
retrieving revision 1.143
retrieving revision 1.144
diff -u -r1.143 -r1.144
--- nand_base.c	19 May 2005 16:10:22 -0000	1.143
+++ nand_base.c	31 May 2005 19:39:16 -0000	1.144
@@ -167,17 +167,21 @@
 
 	/* De-select the NAND device */
 	this->select_chip(mtd, -1);
-	/* Do we have a hardware controller ? */
+
 	if (this->controller) {
+		/* Release the controller and the chip */
 		spin_lock(&this->controller->lock);
 		this->controller->active = NULL;
+		this->state = FL_READY;
+		wake_up(&this->controller->wq);
 		spin_unlock(&this->controller->lock);
+	} else {
+		/* Release the chip */
+		spin_lock(&this->chip_lock);
+		this->state = FL_READY;
+		wake_up(&this->wq);
+		spin_unlock(&this->chip_lock);
 	}
-	/* Release the chip */
-	spin_lock (&this->chip_lock);
-	this->state = FL_READY;
-	wake_up (&this->wq);
-	spin_unlock (&this->chip_lock);
 }
 
 /**
@@ -753,37 +757,34 @@
  */
 static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state)
 {
-	struct nand_chip *active = this;
-
+	struct nand_chip *active;
+	spinlock_t *lock;
+	wait_queue_head_t *wq;
 	DECLARE_WAITQUEUE (wait, current);
 
-	/* 
-	 * Grab the lock and see if the device is available 
-	*/
+	lock = (this->controller) ? &this->controller->lock : &this->lock;
+	wq = (this->controller) ? &this->controller->wq : &this->wq;
 retry:
+	active = this;
+	spin_lock(lock);
+
 	/* Hardware controller shared among independend devices */
 	if (this->controller) {
-		spin_lock (&this->controller->lock);
 		if (this->controller->active)
 			active = this->controller->active;
 		else
 			this->controller->active = this;
-		spin_unlock (&this->controller->lock);
 	}
-	
-	if (active == this) {
-		spin_lock (&this->chip_lock);
-		if (this->state == FL_READY) {
-			this->state = new_state;
-			spin_unlock (&this->chip_lock);
-			return;
-		}
-	}	
-	set_current_state (TASK_UNINTERRUPTIBLE);
-	add_wait_queue (&active->wq, &wait);
-	spin_unlock (&active->chip_lock);
-	schedule ();
-	remove_wait_queue (&active->wq, &wait);
+	if (active == this && this->state == FL_READY) {
+		this->state = new_state;
+		spin_unlock(lock);
+		return;
+	}
+	set_current_state(TASK_UNINTERRUPTIBLE);
+	add_wait_queue(wq, &wait);
+	spin_unlock(lock);
+	schedule();
+	remove_wait_queue(wq, &wait);
 	goto retry;
 }