From nobody Thu Jan 06 19:04:38 2022 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4E6991943BE6; Thu, 6 Jan 2022 19:04:39 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4JVG4Q5FbKz4kfD; Thu, 6 Jan 2022 19:04:38 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 6994B254A7; Thu, 6 Jan 2022 19:04:38 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 206J4cQB054178; Thu, 6 Jan 2022 19:04:38 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 206J4csr054177; Thu, 6 Jan 2022 19:04:38 GMT (envelope-from git) Date: Thu, 6 Jan 2022 19:04:38 GMT Message-Id: <202201061904.206J4csr054177@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Alexander Motin Subject: git: 726d8e185206 - stable/13 - mps(4): Fix unmatched devq release. List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mav X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 726d8e185206b25c9c894c66f28b405c5b204dec Auto-Submitted: auto-generated ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1641495878; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=+3XHRG5t7aR06AYXOb53pOAKZTZpwA3a8sUEVVlAEgs=; b=JmrWlXaHatzvIFbF+qpAOCvt44yt2YJxf7RP+B2mSbg3b4zQagtmixg8jz8FlqsrRecH+h cFeizlvn3+RSNsj7gHPEsOsMao7s65ENixwBE5mUBHaZK4Wv8kF8/+Vwtf2nuy9kUg9hA8 Cc7z4T8YDIrHJQRmO+yCTVaQ1kxgOJ4I9zRXlHKOgucv7BYOwl053Ykpb8LgLTlYiUw7Ky xAFe63jOCPYeNdWuAlBpEHMfydoJ+a2+4SWRWxDwRznArdL08vHnQkSaJISotpkr225xaI kOKDYdgPXCgkl+uZJQKrnBdzKteJ4WiIp/ndsVKpmqhpQFw9ut0n1Ggb5TFszg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1641495878; a=rsa-sha256; cv=none; b=gge7ajkzSVXLqIyR+fJZhQSDZGQgAeJQloCHJrEyBY/r0tMKvB9ZDxnE6G/H9MXdE6NK2C Og61ODVcC1sfkNpZOVwRDhN5TZ1OSB6taRl0thkkO0jnOtHY0zMp89PP34D5tXgydc/Mo8 WY0F5WzjGdfLQcdjmP1wY8Y+nTHOKICNv6o4YbDpWf7v63z3RDbTr72NOl3tOWNr+PEKxD yYrkre7u5Nps96Y9kK/ZBZswUeUnd4U65tENOQ39Xtb3Na/jGhnxarACd7GGFA6Lm6WthM XHP7OsOXC6HKRgcqOCWjlu8Trer4dFiz08URRjJRf5dKe1Cxg50W02e68kG3rg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=726d8e185206b25c9c894c66f28b405c5b204dec commit 726d8e185206b25c9c894c66f28b405c5b204dec Author: Warner Losh AuthorDate: 2021-12-02 20:53:44 +0000 Commit: Alexander Motin CommitDate: 2022-01-06 19:03:38 +0000 mps(4): Fix unmatched devq release. Port 9781c28c6d63 and a8837c77efd0 to the mps driver. Before this change devq was frozen only if some command was sent to the target after reset started, but release was called always. This change freezes the devq immediately, leaving mprsas_action_scsiio() check only to cover race condition due to different lock devq use. This should also avoid unnecessary requeue of the commands, creating additional log noise and confusing some broken apps. It also avoids a 'busy' requeue of I/Os failing when we're doing recovery that takes longer than the normal busy timeout. These I/Os failing can lead to filesystems being unmounted in the force unmount case for I/O errors. Sponsored by: Netflix Reviewed by: mav Differential Revision: https://reviews.freebsd.org/D33228 (cherry picked from commit a10253cffea84c0c980a36ba6776b00ed96c3e3b) --- sys/dev/mps/mps_sas.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/sys/dev/mps/mps_sas.c b/sys/dev/mps/mps_sas.c index 2d580edca9eb..d21cf1e2204e 100644 --- a/sys/dev/mps/mps_sas.c +++ b/sys/dev/mps/mps_sas.c @@ -235,8 +235,6 @@ mpssas_alloc_tm(struct mps_softc *sc) void mpssas_free_tm(struct mps_softc *sc, struct mps_command *tm) { - int target_id = 0xFFFFFFFF; - if (tm == NULL) return; @@ -245,13 +243,11 @@ mpssas_free_tm(struct mps_softc *sc, struct mps_command *tm) * free the resources used for freezing the devq. Must clear the * INRESET flag as well or scsi I/O will not work. */ - if (tm->cm_targ != NULL) { - tm->cm_targ->flags &= ~MPSSAS_TARGET_INRESET; - target_id = tm->cm_targ->tid; - } if (tm->cm_ccb) { - mps_dprint(sc, MPS_INFO, "Unfreezing devq for target ID %d\n", - target_id); + mps_dprint(sc, MPS_XINFO | MPS_RECOVERY, + "Unfreezing devq for target ID %d\n", + tm->cm_targ->tid); + tm->cm_targ->flags &= ~MPSSAS_TARGET_INRESET; xpt_release_devq(tm->cm_ccb->ccb_h.path, 1, TRUE); xpt_free_path(tm->cm_ccb->ccb_h.path); xpt_free_ccb(tm->cm_ccb); @@ -1699,11 +1695,11 @@ mpssas_action_scsiio(struct mpssas_softc *sassc, union ccb *ccb) } /* - * If target has a reset in progress, freeze the devq and return. The - * devq will be released when the TM reset is finished. + * If target has a reset in progress, the devq should be frozen. + * Geting here we likely hit a race, so just requeue. */ if (targ->flags & MPSSAS_TARGET_INRESET) { - ccb->ccb_h.status = CAM_BUSY | CAM_DEV_QFRZN; + ccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_DEV_QFRZN; mps_dprint(sc, MPS_INFO, "%s: Freezing devq for target ID %d\n", __func__, targ->tid); xpt_freeze_devq(ccb->ccb_h.path, 1); @@ -3243,11 +3239,15 @@ mpssas_async(void *callback_arg, uint32_t code, struct cam_path *path, } /* - * Set the INRESET flag for this target so that no I/O will be sent to - * the target until the reset has completed. If an I/O request does - * happen, the devq will be frozen. The CCB holds the path which is - * used to release the devq. The devq is released and the CCB is freed + * Freeze the devq and set the INRESET flag so that no I/O will be sent to + * the target until the reset has completed. The CCB holds the path which + * is used to release the devq. The devq is released and the CCB is freed * when the TM completes. + * We only need to do this when we're entering reset, not at each time we + * need to send an abort (which will happen if multiple commands timeout + * while we're sending the abort). We do not release the queue for each + * command we complete (just at the end when we free the tm), so freezing + * it each time doesn't make sense. */ void mpssas_prepare_for_tm(struct mps_softc *sc, struct mps_command *tm, @@ -3265,7 +3265,13 @@ mpssas_prepare_for_tm(struct mps_softc *sc, struct mps_command *tm, } else { tm->cm_ccb = ccb; tm->cm_targ = target; - target->flags |= MPSSAS_TARGET_INRESET; + if ((target->flags & MPSSAS_TARGET_INRESET) == 0) { + mps_dprint(sc, MPS_XINFO | MPS_RECOVERY, + "%s: Freezing devq for target ID %d\n", + __func__, target->tid); + xpt_freeze_devq(ccb->ccb_h.path, 1); + target->flags |= MPSSAS_TARGET_INRESET; + } } } }