[LU-8553] incorrect fix in LU-7558

• Status: Closed
• Project: Lustre
• Type: Bug
• Priority: Critical
• Reporter: Alexey Lyashkov
• Assignee: Mikhail Pershin
• Resolution: Not a Bug
• Labels: ptlrpc
• Severity: 3

Comments

[Alexey Lyashkov]

grr.. LU-7558 tryes to fix two connection interpret functions race. It assume a new connection attempt is called while first connection interpret executed. But solving that race don't need an introduce a new flag, but just refine a conditional in ptlrpc_connect_import function. Currently it block a connect attempt in several conditions

        if (imp->imp_state == LUSTRE_IMP_CLOSED) {
                spin_unlock(&imp->imp_lock);
                CERROR("can't connect to a closed import\n");
                RETURN(-EINVAL);
        } else if (imp->imp_state == LUSTRE_IMP_FULL) {
                spin_unlock(&imp->imp_lock);
                CERROR("already connected\n");
                RETURN(0);
        } else if (imp->imp_state == LUSTRE_IMP_CONNECTING) {
                spin_unlock(&imp->imp_lock);
                CERROR("already connecting\n");
                RETURN(-EALREADY);
        }

so open a race to send new connect while import switched to the recovery. but changing a last conditional to the something like imp_state != LUSTRE_IMP_DISCON => return -EALREADY. solve same bug without new flag introduce.

But that patch isn't solve second problem anyway. Second problem is introduced with many ptlrpcd threads added as part of SMP improvement work. first connect interpret scheduled to ptlrpcd thread X, but it thread may blocked for some time while processed other interprets like IO processing, so we may enter to situation when client send send second connect while first in flight. It should be easy reconnect with lctl tool which send a parallel connect request, or recovery vs pinger race. First connect send while replay request failed, second request send from pinger. First thread may blocked on cancel unused locks in that case.
I think checking a imp_conn_cnt on connect interpret (in same way as server side does) will solve it problem as don't allow to execute a stale connect interpret.

[Mikhail Pershin]

Closing issue as an outdated one. Alexey, feel free to reopen if you think it still exists