From owner-dev-commits-src-all@freebsd.org Fri Sep 3 00:43:16 2021 Return-Path: Delivered-To: dev-commits-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 1F82D66B922; Fri, 3 Sep 2021 00:43:16 +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 4H0zYH115fz3vVj; Fri, 3 Sep 2021 00:43:14 +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 CA0DD22CB1; Fri, 3 Sep 2021 00:43:14 +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 1830hElk077329; Fri, 3 Sep 2021 00:43:14 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 1830hEZ6077328; Fri, 3 Sep 2021 00:43:14 GMT (envelope-from git) Date: Fri, 3 Sep 2021 00:43:14 GMT Message-Id: <202109030043.1830hEZ6077328@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: a996b55ab34c - stable/13 - mpr(4): Fix unmatched devq release. 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: a996b55ab34c3e2a67a9cc6bea9df1fc39af09d5 Auto-Submitted: auto-generated X-BeenThere: dev-commits-src-all@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Commit messages for all branches of the src repository List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Sep 2021 00:43:16 -0000 The branch stable/13 has been updated by mav: URL: https://cgit.FreeBSD.org/src/commit/?id=a996b55ab34c3e2a67a9cc6bea9df1fc39af09d5 commit a996b55ab34c3e2a67a9cc6bea9df1fc39af09d5 Author: Alexander Motin AuthorDate: 2021-08-20 18:37:55 +0000 Commit: Alexander Motin CommitDate: 2021-09-03 00:35:27 +0000 mpr(4): Fix unmatched devq release. 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. MFC after: 2 weeks Sponsored by: iXsystems, Inc. (cherry picked from commit 9781c28c6d63cfa8438d1aa31f512a6b217a6b2b) --- sys/dev/mpr/mpr_sas.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/sys/dev/mpr/mpr_sas.c b/sys/dev/mpr/mpr_sas.c index 7ead27cb51f9..043d24e82f44 100644 --- a/sys/dev/mpr/mpr_sas.c +++ b/sys/dev/mpr/mpr_sas.c @@ -237,7 +237,6 @@ mprsas_alloc_tm(struct mpr_softc *sc) void mprsas_free_tm(struct mpr_softc *sc, struct mpr_command *tm) { - int target_id = 0xFFFFFFFF; MPR_FUNCTRACE(sc); if (tm == NULL) @@ -248,13 +247,10 @@ mprsas_free_tm(struct mpr_softc *sc, struct mpr_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 &= ~MPRSAS_TARGET_INRESET; - target_id = tm->cm_targ->tid; - } if (tm->cm_ccb) { - mpr_dprint(sc, MPR_INFO, "Unfreezing devq for target ID %d\n", - target_id); + mpr_dprint(sc, MPR_XINFO, "Unfreezing devq for target ID %d\n", + tm->cm_targ->tid); + tm->cm_targ->flags &= ~MPRSAS_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); @@ -1923,13 +1919,11 @@ mprsas_action_scsiio(struct mprsas_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 & MPRSAS_TARGET_INRESET) { - ccb->ccb_h.status = CAM_BUSY | CAM_DEV_QFRZN; - mpr_dprint(sc, MPR_INFO, "%s: Freezing devq for target ID %d\n", - __func__, targ->tid); + ccb->ccb_h.status = CAM_REQUEUE_REQ | CAM_DEV_QFRZN; xpt_freeze_devq(ccb->ccb_h.path, 1); xpt_done(ccb); return; @@ -3425,10 +3419,9 @@ mprsas_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. */ void @@ -3445,6 +3438,10 @@ mprsas_prepare_for_tm(struct mpr_softc *sc, struct mpr_command *tm, target->tid, lun_id) != CAM_REQ_CMP) { xpt_free_ccb(ccb); } else { + mpr_dprint(sc, MPR_XINFO, + "%s: Freezing devq for target ID %d\n", + __func__, target->tid); + xpt_freeze_devq(ccb->ccb_h.path, 1); tm->cm_ccb = ccb; tm->cm_targ = target; target->flags |= MPRSAS_TARGET_INRESET;