From nobody Thu Nov 06 18:56:20 2025 X-Original-To: dev-commits-src-main@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 4d2Wch27xLz6FDxC; Thu, 06 Nov 2025 18:56:20 +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 "R12" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4d2Wch1fV1z3rKK; Thu, 06 Nov 2025 18:56:20 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1762455380; 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=byQgaFtoEocaE4LsQcYmWL8mH2/7Ha+wj7WKHXGmJvc=; b=YSRdjgrqdUvig6DrmyYWvpk7EWPIj4YVcZzrtYpjJzs+lkuaL+Hh8B7prrhsaos3roUXUH p+vv/x/GHtVOGZOtpDOwRJD4qKAWHW75l97VXi0DubmBjg8l3MK+sZgDmb0sSSoMl4iVKp bL0L9PGlvorVWsjsAzsHH5ev6WgWxd3/1cXuWep5jrIZP9u9QN7JlSDqr14ogTYte/A0jE s6X/5UrEPIkSeMkNWAGbznCFf7/BcpnjYh+tWhsS/Yh9RvZms0qq5sNGy5eqdDKO2r8rpY n7PscuqIqR+GXRaPdo5AbQBlhkqSlwy9pStKH5Cil5E2ZonDRk+lUYLr2anh6Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1762455380; 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=byQgaFtoEocaE4LsQcYmWL8mH2/7Ha+wj7WKHXGmJvc=; b=CFlKaoHrzxwc3mmCoopoSzEW/u8LHdgwGo9VfXJBUkntHlmUJpxuG8lXAigsxIkm/aK/AJ L28ZQrmDRBMm1t9szMBI2UJmlaUXDUTUthuASRiJ3JXH/4ABVO8U41uJLgrA+h8rHBkm37 q/8tRheXDDtDMR1UEQmCHOekyDfMjb1urbKTe3ubhfybUm+A9usYDGpS7HnXQm0v6SgKIu XnbDru857XGkt+cSCFMf0yx6swYjn1SZQ69zVgvPM8kgaMa3zGU1YBv/R80kE6XotoqEVU ikaAtdUq0JkPQTRkzgtgNfmo52FJ4mq15tZe5tlCxvbJCSNVAKohcQWWEUlAsQ== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1762455380; a=rsa-sha256; cv=none; b=SAD4/bi5Gs4Y5odWxh1FVRT0rVofb4RfwS8wepvuWN/I5XrquB8CfeDd++k06QRz/0gF6f CXyovuP0cnxXt7FLTpAqEXd5fJwFkzVyN2mhvKIsMubxf93DX5yI/wFAXhr6cQJdjTcrUl W6EUA1ACe4AFVJ8h2iH5w+eg48aKA0gQFxh5F3NFxuVpJ0m9yFuU5edkhVo4oe+G5ygWOI nzy3tHG8XVbGg50m0jDUWB0A27QadEkPGXAVW/OKLKBR02MlEQlC8u5o7CbPuOfzNK590l X4otAfoK0Z1eT4PHt4wmABuW2AHumODgz8dtE88oSwu00ULqGiIPlnAgDP1Efw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4d2Wch172sz9PX; Thu, 06 Nov 2025 18:56:20 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 5A6IuKAT024992; Thu, 6 Nov 2025 18:56:20 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 5A6IuKVL024989; Thu, 6 Nov 2025 18:56:20 GMT (envelope-from git) Date: Thu, 6 Nov 2025 18:56:20 GMT Message-Id: <202511061856.5A6IuKVL024989@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Warner Losh Subject: git: b558ebb082ff - main - cam: Add a number of asserts to catch bad transactions List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-main@freebsd.org Sender: owner-dev-commits-src-main@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: b558ebb082ffa13f3c37aa8f7d7974b64e93fa93 Auto-Submitted: auto-generated The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=b558ebb082ffa13f3c37aa8f7d7974b64e93fa93 commit b558ebb082ffa13f3c37aa8f7d7974b64e93fa93 Author: Warner Losh AuthorDate: 2025-11-06 18:52:43 +0000 Commit: Warner Losh CommitDate: 2025-11-06 18:53:40 +0000 cam: Add a number of asserts to catch bad transactions Ensure that we're in the right state / priority for each of the states in the driver. These asserts assured that a prior patch that I committed to fix a priority leak worked when a drive departed (and bounced back too!). These have been running in our production since I committed the change and haven't trigged. Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D53259 --- sys/cam/ata/ata_da.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++ sys/cam/scsi/scsi_da.c | 21 +++++++++++++++ 2 files changed, 93 insertions(+) diff --git a/sys/cam/ata/ata_da.c b/sys/cam/ata/ata_da.c index 08747cd59131..9434756b87f9 100644 --- a/sys/cam/ata/ata_da.c +++ b/sys/cam/ata/ata_da.c @@ -2328,15 +2328,38 @@ adastart(struct cam_periph *periph, union ccb *start_ccb) { struct ada_softc *softc = (struct ada_softc *)periph->softc; struct ccb_ataio *ataio = &start_ccb->ataio; + uint32_t priority = start_ccb->ccb_h.pinfo.priority; CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("adastart\n")); + /* + * When we're running the state machine, we should only accept DEV CCBs. + * When we're doing normal I/O we should only accept NORMAL CCBs. + * + * While in the state machine, we carefully single step the queue, but + * there's no protection for 'extra' calls to xpt_schedule() at the + * wrong priority. Guard against that so that we filter any CCBs that + * are offered at the wrong priority. This avoids generating requests + * that are at normal priority. +` */ + if ((softc->state != ADA_STATE_NORMAL && priority != CAM_PRIORITY_DEV) || + (softc->state == ADA_STATE_NORMAL && priority != CAM_PRIORITY_NORMAL)) { + xpt_print(periph->path, "Bad priority for state %d prio %d\n", + softc->state, priority); + xpt_release_ccb(start_ccb); + return; + } + switch (softc->state) { case ADA_STATE_NORMAL: { struct bio *bp; uint8_t tag_code; + KASSERT(priority == CAM_PRIORITY_NORMAL, + ("Expected priority %d, found %d in state normal", + CAM_PRIORITY_NORMAL, priority)); + bp = cam_iosched_next_bio(softc->cam_iosched); if (bp == NULL) { xpt_release_ccb(start_ccb); @@ -2555,6 +2578,11 @@ out: case ADA_STATE_RAHEAD: case ADA_STATE_WCACHE: { + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in state %s", + CAM_PRIORITY_DEV, priority, + softc->state == ADA_STATE_RAHEAD ? "rahead" : "wcache")); + cam_fill_ataio(ataio, 1, adadone, @@ -2581,6 +2609,10 @@ out: { struct ata_gp_log_dir *log_dir; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in state logdir", + CAM_PRIORITY_DEV, priority)); + if ((softc->flags & ADA_FLAG_CAN_LOG) == 0) { adaprobedone(periph, start_ccb); break; @@ -2615,6 +2647,10 @@ out: { struct ata_identify_log_pages *id_dir; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in state iddir", + CAM_PRIORITY_DEV, priority)); + id_dir = malloc(sizeof(*id_dir), M_ATADA, M_NOWAIT | M_ZERO); if (id_dir == NULL) { xpt_print(periph->path, "Couldn't malloc id_dir " @@ -2643,6 +2679,10 @@ out: { struct ata_identify_log_sup_cap *sup_cap; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in state sup_cap", + CAM_PRIORITY_DEV, priority)); + sup_cap = malloc(sizeof(*sup_cap), M_ATADA, M_NOWAIT|M_ZERO); if (sup_cap == NULL) { xpt_print(periph->path, "Couldn't malloc sup_cap " @@ -2671,6 +2711,10 @@ out: { struct ata_zoned_info_log *ata_zone; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in state zone", + CAM_PRIORITY_DEV, priority)); + ata_zone = malloc(sizeof(*ata_zone), M_ATADA, M_NOWAIT|M_ZERO); if (ata_zone == NULL) { xpt_print(periph->path, "Couldn't malloc ata_zone " @@ -2896,6 +2940,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) struct bio *bp; int error; + KASSERT(priority == CAM_PRIORITY_NORMAL, + ("Expected priority %d, found %d for normal I/O", + CAM_PRIORITY_NORMAL, priority)); + cam_periph_lock(periph); bp = (struct bio *)done_ccb->ccb_h.ccb_bp; if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { @@ -3000,6 +3048,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) } case ADA_CCB_RAHEAD: { + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in ccb state rahead", + CAM_PRIORITY_DEV, priority)); + if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { if (adaerror(done_ccb, 0, 0) == ERESTART) { /* Drop freeze taken due to CAM_DEV_QFREEZE */ @@ -3023,6 +3075,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) } case ADA_CCB_WCACHE: { + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in ccb state wcache", + CAM_PRIORITY_DEV, priority)); + if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { if (adaerror(done_ccb, 0, 0) == ERESTART) { /* Drop freeze taken due to CAM_DEV_QFREEZE */ @@ -3054,6 +3110,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) { int error; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in ccb state logdir", + CAM_PRIORITY_DEV, priority)); + if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) { error = 0; softc->valid_logdir_len = 0; @@ -3123,6 +3183,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) case ADA_CCB_IDDIR: { int error; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in ccb state iddir", + CAM_PRIORITY_DEV, priority)); + if ((ataio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) { off_t entries_offset, max_entries; error = 0; @@ -3208,6 +3272,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) case ADA_CCB_SUP_CAP: { int error; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in ccb state sup_cap", + CAM_PRIORITY_DEV, priority)); + if ((ataio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) { uint32_t valid_len; size_t needed_size; @@ -3312,6 +3380,10 @@ adadone(struct cam_periph *periph, union ccb *done_ccb) case ADA_CCB_ZONE: { int error; + KASSERT(priority == CAM_PRIORITY_DEV, + ("Expected priority %d, found %d in ccb state zone", + CAM_PRIORITY_DEV, priority)); + if ((ataio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) { struct ata_zoned_info_log *zi_log; uint32_t valid_len; diff --git a/sys/cam/scsi/scsi_da.c b/sys/cam/scsi/scsi_da.c index c0c0be12856b..773a786d08f7 100644 --- a/sys/cam/scsi/scsi_da.c +++ b/sys/cam/scsi/scsi_da.c @@ -3369,12 +3369,33 @@ static void dastart(struct cam_periph *periph, union ccb *start_ccb) { struct da_softc *softc; + uint32_t priority = start_ccb->ccb_h.pinfo.priority; cam_periph_assert(periph, MA_OWNED); softc = (struct da_softc *)periph->softc; CAM_DEBUG(periph->path, CAM_DEBUG_TRACE, ("dastart\n")); + /* + * When we're running the state machine, we should only accept DEV CCBs. + * When we're doing normal I/O we should only accept NORMAL CCBs. + * + * While in the state machine, we carefully single step the queue, but + * there's no protection for 'extra' calls to xpt_schedule() at the + * wrong priority. Guard against that so that we filter any CCBs that + * are offered at the wrong priority. This avoids generating requests + * that are at normal priority. In addition, though we can't easily + * enforce it, one must not transition to the NORMAL state via the + * skipstate mechanism. +` */ + if ((softc->state != DA_STATE_NORMAL && priority != CAM_PRIORITY_DEV) || + (softc->state == DA_STATE_NORMAL && priority != CAM_PRIORITY_NORMAL)) { + xpt_print(periph->path, "Bad priority for state %d prio %d\n", + softc->state, priority); + xpt_release_ccb(start_ccb); + return; + } + skipstate: switch (softc->state) { case DA_STATE_NORMAL: