Commit:     86e8dfc5603ed76917eed0a9dd9e85a1e1a8b162
Parent:     d0076f7754dce07c7a1d752034561acadd99eafa
Author:     Martin Peschke <[EMAIL PROTECTED]>
AuthorDate: Thu Nov 15 13:57:17 2007 +0100
Committer:  James Bottomley <[EMAIL PROTECTED]>
CommitDate: Fri Nov 16 13:03:21 2007 -0600

    [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) {
                up(&adapter->erp_ready_sem);    /* first up() for erp_action */
        write_unlock_irqrestore(&adapter->erp_lock, flags);
    write_lock_irqsave(&adapter->erp_lock, flags);
    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) {
                                /* 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) {
        retval = ZFCP_ERP_DISMISSED;
    write_unlock_restore(&adapter->erp_lock, flags);
    /* 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 <[EMAIL PROTECTED]>
    Acked-by: Swen Schillig <[EMAIL PROTECTED]>
    Signed-off-by: James Bottomley <[EMAIL PROTECTED]>
 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 ad5b481..07fa824 100644
--- a/drivers/s390/scsi/zfcp_erp.c
+++ b/drivers/s390/scsi/zfcp_erp.c
@@ -1065,7 +1065,7 @@ zfcp_erp_thread(void *data)
                                 &adapter->status)) {
                write_lock_irqsave(&adapter->erp_lock, flags);
-               next = adapter->erp_ready_head.prev;
+               next = adapter->;
                write_unlock_irqrestore(&adapter->erp_lock, flags);
                if (next != &adapter->erp_ready_head) {
@@ -1155,15 +1155,13 @@ zfcp_erp_strategy(struct zfcp_erp_action *erp_action)
         * check for dismissed status again to avoid follow-up actions,
-        * failing of targets and so on for dismissed actions
+        * failing of targets and so on for dismissed actions,
+        * we go through down() here because there has been an up()
-       retval = zfcp_erp_strategy_check_action(erp_action, retval);
+       if (erp_action->status & ZFCP_STATUS_ERP_DISMISSED)
+               retval = ZFCP_ERP_CONTINUES;
        switch (retval) {
-       case ZFCP_ERP_DISMISSED:
-               /* leave since this action has ridden to its ancestors */
-               debug_text_event(adapter->erp_dbf, 6, "a_st_dis2");
-               goto unlock;
        case ZFCP_ERP_NOMEM:
                /* no memory to continue immediately, let it sleep */
                if (!(erp_action->status & ZFCP_STATUS_ERP_LOWMEM)) {
@@ -3091,7 +3089,7 @@ zfcp_erp_action_enqueue(int action,
        /* finally put it into 'ready' queue and kick erp thread */
-       list_add(&erp_action->list, &adapter->erp_ready_head);
+       list_add_tail(&erp_action->list, &adapter->erp_ready_head);
        retval = 0;
To unsubscribe from this list: send the line "unsubscribe git-commits-head" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at

Reply via email to