diff options
author | Martin Peschke <mp3@de.ibm.com> | 2007-11-15 07:57:17 -0500 |
---|---|---|
committer | James Bottomley <James.Bottomley@HansenPartnership.com> | 2007-11-16 14:03:21 -0500 |
commit | 86e8dfc5603ed76917eed0a9dd9e85a1e1a8b162 (patch) | |
tree | 3d5bf28741aacff3f60df35c0619c7d9cd352f44 /drivers/s390/scsi/zfcp_erp.c | |
parent | d0076f7754dce07c7a1d752034561acadd99eafa (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/s390/scsi/zfcp_erp.c')
-rw-r--r-- | drivers/s390/scsi/zfcp_erp.c | 14 |
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: |