From owner-svn-src-all@freebsd.org Mon Nov 5 18:47:32 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id BC1851108C1E; Mon, 5 Nov 2018 18:47:32 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 3087F6D544; Mon, 5 Nov 2018 18:47:32 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 7EABF1B6CC; Mon, 5 Nov 2018 18:47:30 +0000 (UTC) (envelope-from imp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id wA5IlU1d056117; Mon, 5 Nov 2018 18:47:30 GMT (envelope-from imp@FreeBSD.org) Received: (from imp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id wA5IlUdA056116; Mon, 5 Nov 2018 18:47:30 GMT (envelope-from imp@FreeBSD.org) Message-Id: <201811051847.wA5IlUdA056116@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: imp set sender to imp@FreeBSD.org using -f From: Warner Losh Date: Mon, 5 Nov 2018 18:47:30 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r340155 - head/sys/cam/scsi X-SVN-Group: head X-SVN-Commit-Author: imp X-SVN-Commit-Paths: head/sys/cam/scsi X-SVN-Commit-Revision: 340155 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 3087F6D544 X-Spamd-Result: default: False [-1.54 / 200.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.66)[-0.663,0]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[3]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; TO_DN_NONE(0.00)[]; HAS_XAW(0.00)[]; R_SPF_SOFTFAIL(0.00)[~all]; DMARC_NA(0.00)[FreeBSD.org]; RCVD_COUNT_THREE(0.00)[4]; MX_GOOD(-0.01)[cached: mx1.FreeBSD.org]; NEURAL_HAM_SHORT(-0.77)[-0.767,0]; FROM_EQ_ENVFROM(0.00)[]; R_DKIM_NA(0.00)[]; RCVD_TLS_LAST(0.00)[] X-Rspamd-Server: mx1.freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Nov 2018 18:47:33 -0000 Author: imp Date: Mon Nov 5 18:47:29 2018 New Revision: 340155 URL: https://svnweb.freebsd.org/changeset/base/340155 Log: Only assert locked for many async events. Many async events that we see are called for this specific path. When calling an async callback for a targetted device, XTP will lock that specific device's path lock (same as what cam_periph_lock does). For those AC_ events, assert we have the lock rather than trying to recusrively take it (which causes panics since it's not recursive). Add annotations about this and about the fact that AC_SCSI_AEN events are generated now only in the ata stack (which cannot have a scsi_da attachment). Leave it in place in case I've overlooked something as the code is harmless. This is fallout from my attempts to "fix" locking for softc->flags in r330796 that's not been triggered often enough to get my attention until now. Sponsored by: Netflix MFC After: 3 days Differential Revision: https://reviews.freebsd.org/D17837 Modified: head/sys/cam/scsi/scsi_da.c Modified: head/sys/cam/scsi/scsi_da.c ============================================================================== --- head/sys/cam/scsi/scsi_da.c Mon Nov 5 18:12:41 2018 (r340154) +++ head/sys/cam/scsi/scsi_da.c Mon Nov 5 18:47:29 2018 (r340155) @@ -2005,7 +2005,7 @@ daasync(void *callback_arg, u_int32_t code, periph = (struct cam_periph *)callback_arg; switch (code) { - case AC_FOUND_DEVICE: + case AC_FOUND_DEVICE: /* callback to create periph, no locking yet */ { struct ccb_getdev *cgd; cam_status status; @@ -2041,7 +2041,7 @@ daasync(void *callback_arg, u_int32_t code, "due to status 0x%x\n", status); return; } - case AC_ADVINFO_CHANGED: + case AC_ADVINFO_CHANGED: /* Doesn't touch periph */ { uintptr_t buftype; @@ -2064,8 +2064,10 @@ daasync(void *callback_arg, u_int32_t code, ccb = (union ccb *)arg; /* - * Handle all UNIT ATTENTIONs except our own, - * as they will be handled by daerror(). + * Handle all UNIT ATTENTIONs except our own, as they will be + * handled by daerror(). Since this comes from a different periph, + * that periph's lock is held, not ours, so we have to take it ours + * out to touch softc flags. */ if (xpt_path_periph(ccb->ccb_h.path) != periph && scsi_extract_sense_ccb(ccb, @@ -2093,9 +2095,13 @@ daasync(void *callback_arg, u_int32_t code, } break; } - case AC_SCSI_AEN: + case AC_SCSI_AEN: /* Called for this path: periph locked */ + /* + * Appears to be currently unused for SCSI devices, only ata SIMs + * generate this. + */ + cam_periph_assert(periph, MA_OWNED); softc = (struct da_softc *)periph->softc; - cam_periph_lock(periph); if (!cam_iosched_has_work_flags(softc->cam_iosched, DA_WORK_TUR) && (softc->flags & DA_FLAG_TUR_PENDING) == 0) { if (da_periph_acquire(periph, DA_REF_TUR) == 0) { @@ -2103,31 +2109,28 @@ daasync(void *callback_arg, u_int32_t code, daschedule(periph); } } - cam_periph_unlock(periph); /* FALLTHROUGH */ - case AC_SENT_BDR: - case AC_BUS_RESET: + case AC_SENT_BDR: /* Called for this path: periph locked */ + case AC_BUS_RESET: /* Called for this path: periph locked */ { struct ccb_hdr *ccbh; + cam_periph_assert(periph, MA_OWNED); softc = (struct da_softc *)periph->softc; /* * Don't fail on the expected unit attention * that will occur. */ - cam_periph_lock(periph); softc->flags |= DA_FLAG_RETRY_UA; LIST_FOREACH(ccbh, &softc->pending_ccbs, periph_links.le) ccbh->ccb_state |= DA_CCB_RETRY_UA; - cam_periph_unlock(periph); break; } - case AC_INQ_CHANGED: - cam_periph_lock(periph); + case AC_INQ_CHANGED: /* Called for this path: periph locked */ + cam_periph_assert(periph, MA_OWNED); softc = (struct da_softc *)periph->softc; softc->flags &= ~DA_FLAG_PROBED; dareprobe(periph); - cam_periph_unlock(periph); break; default: break;