Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

[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.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

authored by

Thomas Gleixner and committed by
Thomas Gleixner
0dfc6246 7ca6448d

+33 -29
+29 -28
drivers/mtd/nand/nand_base.c
··· 59 59 * The AG-AND chips have nice features for speed improvement, 60 60 * which are not supported yet. Read / program 4 pages in one go. 61 61 * 62 - * $Id: nand_base.c,v 1.143 2005/05/19 16:10:22 gleixner Exp $ 62 + * $Id: nand_base.c,v 1.145 2005/05/31 20:32:53 gleixner Exp $ 63 63 * 64 64 * This program is free software; you can redistribute it and/or modify 65 65 * it under the terms of the GNU General Public License version 2 as ··· 167 167 168 168 /* De-select the NAND device */ 169 169 this->select_chip(mtd, -1); 170 - /* Do we have a hardware controller ? */ 170 + 171 171 if (this->controller) { 172 + /* Release the controller and the chip */ 172 173 spin_lock(&this->controller->lock); 173 174 this->controller->active = NULL; 175 + this->state = FL_READY; 176 + wake_up(&this->controller->wq); 174 177 spin_unlock(&this->controller->lock); 178 + } else { 179 + /* Release the chip */ 180 + spin_lock(&this->chip_lock); 181 + this->state = FL_READY; 182 + wake_up(&this->wq); 183 + spin_unlock(&this->chip_lock); 175 184 } 176 - /* Release the chip */ 177 - spin_lock (&this->chip_lock); 178 - this->state = FL_READY; 179 - wake_up (&this->wq); 180 - spin_unlock (&this->chip_lock); 181 185 } 182 186 183 187 /** ··· 757 753 */ 758 754 static void nand_get_device (struct nand_chip *this, struct mtd_info *mtd, int new_state) 759 755 { 760 - struct nand_chip *active = this; 761 - 756 + struct nand_chip *active; 757 + spinlock_t *lock; 758 + wait_queue_head_t *wq; 762 759 DECLARE_WAITQUEUE (wait, current); 763 760 764 - /* 765 - * Grab the lock and see if the device is available 766 - */ 761 + lock = (this->controller) ? &this->controller->lock : &this->chip_lock; 762 + wq = (this->controller) ? &this->controller->wq : &this->wq; 767 763 retry: 764 + active = this; 765 + spin_lock(lock); 766 + 768 767 /* Hardware controller shared among independend devices */ 769 768 if (this->controller) { 770 - spin_lock (&this->controller->lock); 771 769 if (this->controller->active) 772 770 active = this->controller->active; 773 771 else 774 772 this->controller->active = this; 775 - spin_unlock (&this->controller->lock); 776 773 } 777 - 778 - if (active == this) { 779 - spin_lock (&this->chip_lock); 780 - if (this->state == FL_READY) { 781 - this->state = new_state; 782 - spin_unlock (&this->chip_lock); 783 - return; 784 - } 785 - } 786 - set_current_state (TASK_UNINTERRUPTIBLE); 787 - add_wait_queue (&active->wq, &wait); 788 - spin_unlock (&active->chip_lock); 789 - schedule (); 790 - remove_wait_queue (&active->wq, &wait); 774 + if (active == this && this->state == FL_READY) { 775 + this->state = new_state; 776 + spin_unlock(lock); 777 + return; 778 + } 779 + set_current_state(TASK_UNINTERRUPTIBLE); 780 + add_wait_queue(wq, &wait); 781 + spin_unlock(lock); 782 + schedule(); 783 + remove_wait_queue(wq, &wait); 791 784 goto retry; 792 785 } 793 786
+4 -1
include/linux/mtd/nand.h
··· 5 5 * Steven J. Hill <sjhill@realitydiluted.com> 6 6 * Thomas Gleixner <tglx@linutronix.de> 7 7 * 8 - * $Id: nand.h,v 1.71 2005/02/09 12:12:59 gleixner Exp $ 8 + * $Id: nand.h,v 1.73 2005/05/31 19:39:17 gleixner Exp $ 9 9 * 10 10 * This program is free software; you can redistribute it and/or modify 11 11 * it under the terms of the GNU General Public License version 2 as ··· 253 253 * struct nand_hw_control - Control structure for hardware controller (e.g ECC generator) shared among independend devices 254 254 * @lock: protection lock 255 255 * @active: the mtd device which holds the controller currently 256 + * @wq: wait queue to sleep on if a NAND operation is in progress 257 + * used instead of the per chip wait queue when a hw controller is available 256 258 */ 257 259 struct nand_hw_control { 258 260 spinlock_t lock; 259 261 struct nand_chip *active; 262 + wait_queue_head_t wq; 260 263 }; 261 264 262 265 /**