aboutsummaryrefslogtreecommitdiffstats
path: root/drivers
diff options
context:
space:
mode:
authorMartin Peschke <mp3@de.ibm.com>2007-11-15 07:57:17 -0500
committerJames Bottomley <James.Bottomley@HansenPartnership.com>2007-11-16 14:03:21 -0500
commit86e8dfc5603ed76917eed0a9dd9e85a1e1a8b162 (patch)
tree3d5bf28741aacff3f60df35c0619c7d9cd352f44 /drivers
parentd0076f7754dce07c7a1d752034561acadd99eafa (diff)
[SCSI] zfcp: fix cleanup of dismissed error recovery actions
Calling zfcp_erp_strategy_check_action() after zfcp_erp_action_to_running() in zfcp_erp_strategy() might cause an unbalanced up() for erp_ready_sem, which makes the zfcp recovery fail somewhere along the way: erp thread processing erp_action: | | someone waking up erp thread for erp_action | | | | someone else dismissing erp_action: | | | V V V write_lock_irqsave(&adapter->erp_lock, flags); ... if (zfcp_erp_action_exists(erp_action) == ZFCP_ERP_ACTION_RUNNING) { zfcp_erp_action_to_ready(erp_action); up(&adapter->erp_ready_sem); /* first up() for erp_action */ } write_unlock_irqrestore(&adapter->erp_lock, flags); write_lock_irqsave(&adapter->erp_lock, flags); ... zfcp_erp_action_to_running(erp_action); write_unlock_restore(&adapter->erp_lock, flags); /* processing erp_action */ write_lock_irqsave(&adapter->erp_lock, flags); ... erp_action->status |= ZFCP_STATUS_ERP_DISMISSED; if (zfcp_erp_action_exists(erp_action) == ZFCP_ERP_ACTION_RUNNING) { zfcp_erp_action_to_ready(erp_action); up(&adapter->erp_ready_sem); /* second, unbalanced up() for erp_action */ } ... write_unlock_restore(&adapter->erp_lock, flags); write_lock_irqsave(&adapter->erp_lock, flags); if (erp_action->status & ZFCP_STATUS_ERP_DISMISSED) { zfcp_erp_action_dequeue(erp_action); retval = ZFCP_ERP_DISMISSED; } ... write_unlock_restore(&adapter->erp_lock, flags); down(&adapter->erp_ready_sem); /* this down() is meant to balance the first up() */ The erp thread must not dismiss an erp_action after moving that action to erp_running_head. Instead it should just go through the down() operation, which balances the first up(), and run through zfcp_erp_strategy one more time for the second up(), which eventually cleans up erp_action. Which is similar to the normal processing of an event for erp_action doing something asynchronously (e.g. waiting for the completion of an fsf_req). This only works if we make sure that a dismissed erp_action is passed to zfcp_erp_strategy() prior to the other action, which caused actions to be dismissed. Therefore the patch implements this rule: running actions go to the head of the ready list; new actions go to the tail of the ready list; the erp thread picks actions to be processed from the ready list's head. Signed-off-by: Martin Peschke <mp3@de.ibm.com> Acked-by: Swen Schillig <swen@vnet.ibm.com> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
Diffstat (limited to 'drivers')
-rw-r--r--drivers/s390/scsi/zfcp_erp.c14
1 files changed, 6 insertions, 8 deletions
diff --git a/drivers/s390/scsi/zfcp_erp.c b/drivers/s390/scsi/zfcp_erp.c
index ad5b481ddafa..07fa824d179f 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -1065,7 +1065,7 @@ zfcp_erp_thread(void *data)
1065 &adapter->status)) { 1065 &adapter->status)) {
1066 1066
1067 write_lock_irqsave(&adapter->erp_lock, flags); 1067 write_lock_irqsave(&adapter->erp_lock, flags);
1068 next = adapter->erp_ready_head.prev; 1068 next = adapter->erp_ready_head.next;
1069 write_unlock_irqrestore(&adapter->erp_lock, flags); 1069 write_unlock_irqrestore(&adapter->erp_lock, flags);
1070 1070
1071 if (next != &adapter->erp_ready_head) { 1071 if (next != &adapter->erp_ready_head) {
@@ -1155,15 +1155,13 @@ zfcp_erp_strategy(struct zfcp_erp_action *erp_action)
1155 1155
1156 /* 1156 /*
1157 * check for dismissed status again to avoid follow-up actions, 1157 * check for dismissed status again to avoid follow-up actions,
1158 * failing of targets and so on for dismissed actions 1158 * failing of targets and so on for dismissed actions,
1159 * we go through down() here because there has been an up()
1159 */ 1160 */
1160 retval = zfcp_erp_strategy_check_action(erp_action, retval); 1161 if (erp_action->status & ZFCP_STATUS_ERP_DISMISSED)
1162 retval = ZFCP_ERP_CONTINUES;
1161 1163
1162 switch (retval) { 1164 switch (retval) {
1163 case ZFCP_ERP_DISMISSED:
1164 /* leave since this action has ridden to its ancestors */
1165 debug_text_event(adapter->erp_dbf, 6, "a_st_dis2");
1166 goto unlock;
1167 case ZFCP_ERP_NOMEM: 1165 case ZFCP_ERP_NOMEM:
1168 /* no memory to continue immediately, let it sleep */ 1166 /* no memory to continue immediately, let it sleep */
1169 if (!(erp_action->status & ZFCP_STATUS_ERP_LOWMEM)) { 1167 if (!(erp_action->status & ZFCP_STATUS_ERP_LOWMEM)) {
@@ -3091,7 +3089,7 @@ zfcp_erp_action_enqueue(int action,
3091 ++adapter->erp_total_count; 3089 ++adapter->erp_total_count;
3092 3090
3093 /* finally put it into 'ready' queue and kick erp thread */ 3091 /* finally put it into 'ready' queue and kick erp thread */
3094 list_add(&erp_action->list, &adapter->erp_ready_head); 3092 list_add_tail(&erp_action->list, &adapter->erp_ready_head);
3095 up(&adapter->erp_ready_sem); 3093 up(&adapter->erp_ready_sem);
3096 retval = 0; 3094 retval = 0;
3097 out: 3095 out: