diff options
author | Martin Peschke <mpeschke@linux.vnet.ibm.com> | 2013-08-22 11:45:36 -0400 |
---|---|---|
committer | James Bottomley <JBottomley@Parallels.com> | 2013-08-22 11:53:30 -0400 |
commit | d79ff142624e1be080ad8d09101f7004d79c36e1 (patch) | |
tree | 490bd19345fa2dfee17cae0d9fe580e08d6c5d74 /include/linux | |
parent | 35dc248383bbab0a7203fca4d722875bc81ef091 (diff) |
[SCSI] zfcp: fix lock imbalance by reworking request queue locking
This patch adds wait_event_interruptible_lock_irq_timeout(), which is a
straight-forward descendant of wait_event_interruptible_timeout() and
wait_event_interruptible_lock_irq().
The zfcp driver used to call wait_event_interruptible_timeout()
in combination with some intricate and error-prone locking. Using
wait_event_interruptible_lock_irq_timeout() as a replacement
nicely cleans up that locking.
This rework removes a situation that resulted in a locking imbalance
in zfcp_qdio_sbal_get():
BUG: workqueue leaked lock or atomic: events/1/0xffffff00/10
last function: zfcp_fc_wka_port_offline+0x0/0xa0 [zfcp]
It was introduced by commit c2af7545aaff3495d9bf9a7608c52f0af86fb194
"[SCSI] zfcp: Do not wait for SBALs on stopped queue", which had a new
code path related to ZFCP_STATUS_ADAPTER_QDIOUP that took an early exit
without a required lock being held. The problem occured when a
special, non-SCSI I/O request was being submitted in process context,
when the adapter's queues had been torn down. In this case the bug
surfaced when the Fibre Channel port connection for a well-known address
was closed during a concurrent adapter shut-down procedure, which is a
rare constellation.
This patch also fixes these warnings from the sparse tool (make C=1):
drivers/s390/scsi/zfcp_qdio.c:224:12: warning: context imbalance in
'zfcp_qdio_sbal_check' - wrong count at exit
drivers/s390/scsi/zfcp_qdio.c:244:5: warning: context imbalance in
'zfcp_qdio_sbal_get' - unexpected unlock
Last but not least, we get rid of that crappy lock-unlock-lock
sequence at the beginning of the critical section.
It is okay to call zfcp_erp_adapter_reopen() with req_q_lock held.
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Reported-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Peschke <mpeschke@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org #2.6.35+
Signed-off-by: Steffen Maier <maier@linux.vnet.ibm.com>
Signed-off-by: James Bottomley <JBottomley@Parallels.com>
Diffstat (limited to 'include/linux')
-rw-r--r-- | include/linux/wait.h | 57 |
1 files changed, 57 insertions, 0 deletions
diff --git a/include/linux/wait.h b/include/linux/wait.h index f487a4750b7f..a67fc1635592 100644 --- a/include/linux/wait.h +++ b/include/linux/wait.h | |||
@@ -811,6 +811,63 @@ do { \ | |||
811 | __ret; \ | 811 | __ret; \ |
812 | }) | 812 | }) |
813 | 813 | ||
814 | #define __wait_event_interruptible_lock_irq_timeout(wq, condition, \ | ||
815 | lock, ret) \ | ||
816 | do { \ | ||
817 | DEFINE_WAIT(__wait); \ | ||
818 | \ | ||
819 | for (;;) { \ | ||
820 | prepare_to_wait(&wq, &__wait, TASK_INTERRUPTIBLE); \ | ||
821 | if (condition) \ | ||
822 | break; \ | ||
823 | if (signal_pending(current)) { \ | ||
824 | ret = -ERESTARTSYS; \ | ||
825 | break; \ | ||
826 | } \ | ||
827 | spin_unlock_irq(&lock); \ | ||
828 | ret = schedule_timeout(ret); \ | ||
829 | spin_lock_irq(&lock); \ | ||
830 | if (!ret) \ | ||
831 | break; \ | ||
832 | } \ | ||
833 | finish_wait(&wq, &__wait); \ | ||
834 | } while (0) | ||
835 | |||
836 | /** | ||
837 | * wait_event_interruptible_lock_irq_timeout - sleep until a condition gets true or a timeout elapses. | ||
838 | * The condition is checked under the lock. This is expected | ||
839 | * to be called with the lock taken. | ||
840 | * @wq: the waitqueue to wait on | ||
841 | * @condition: a C expression for the event to wait for | ||
842 | * @lock: a locked spinlock_t, which will be released before schedule() | ||
843 | * and reacquired afterwards. | ||
844 | * @timeout: timeout, in jiffies | ||
845 | * | ||
846 | * The process is put to sleep (TASK_INTERRUPTIBLE) until the | ||
847 | * @condition evaluates to true or signal is received. The @condition is | ||
848 | * checked each time the waitqueue @wq is woken up. | ||
849 | * | ||
850 | * wake_up() has to be called after changing any variable that could | ||
851 | * change the result of the wait condition. | ||
852 | * | ||
853 | * This is supposed to be called while holding the lock. The lock is | ||
854 | * dropped before going to sleep and is reacquired afterwards. | ||
855 | * | ||
856 | * The function returns 0 if the @timeout elapsed, -ERESTARTSYS if it | ||
857 | * was interrupted by a signal, and the remaining jiffies otherwise | ||
858 | * if the condition evaluated to true before the timeout elapsed. | ||
859 | */ | ||
860 | #define wait_event_interruptible_lock_irq_timeout(wq, condition, lock, \ | ||
861 | timeout) \ | ||
862 | ({ \ | ||
863 | int __ret = timeout; \ | ||
864 | \ | ||
865 | if (!(condition)) \ | ||
866 | __wait_event_interruptible_lock_irq_timeout( \ | ||
867 | wq, condition, lock, __ret); \ | ||
868 | __ret; \ | ||
869 | }) | ||
870 | |||
814 | 871 | ||
815 | /* | 872 | /* |
816 | * These are the old interfaces to sleep waiting for an event. | 873 | * These are the old interfaces to sleep waiting for an event. |