From owner-svn-src-all@FreeBSD.ORG Wed Jan 9 17:02:09 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 21ED6DA1; Wed, 9 Jan 2013 17:02:09 +0000 (UTC) (envelope-from ken@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:1900:2254:2068::e6a:0]) by mx1.freebsd.org (Postfix) with ESMTP id 0ED53E5F; Wed, 9 Jan 2013 17:02:09 +0000 (UTC) Received: from svn.freebsd.org (svn.FreeBSD.org [8.8.178.70]) by svn.freebsd.org (8.14.5/8.14.5) with ESMTP id r09H28IO053603; Wed, 9 Jan 2013 17:02:08 GMT (envelope-from ken@svn.freebsd.org) Received: (from ken@localhost) by svn.freebsd.org (8.14.5/8.14.5/Submit) id r09H28TD053601; Wed, 9 Jan 2013 17:02:08 GMT (envelope-from ken@svn.freebsd.org) Message-Id: <201301091702.r09H28TD053601@svn.freebsd.org> From: "Kenneth D. Merry" Date: Wed, 9 Jan 2013 17:02:08 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r245228 - head/sys/cam/ctl X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 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: Wed, 09 Jan 2013 17:02:09 -0000 Author: ken Date: Wed Jan 9 17:02:08 2013 New Revision: 245228 URL: http://svnweb.freebsd.org/changeset/base/245228 Log: Make CTL work a little better with loading and unloading drivers. Previously CTL would leave individual LUNs enabled in the target driver, whether or not the port as a whole was enabled. It would also leave the wildcard LUN enabled indefinitely. This change means that CTL will enable and disable any active LUNs, as well as the wildcard LUN, when enabling and disabling a port. Also, fix a bug that could crop up due to an uninitialized CCB type. ctl.c: Before calling ctl_frontend_online(), run through the LUN list and enable all active LUNs. After calling ctl_frontend_offline(), run through the LUN list and disble all active LUNs. scsi_ctl.c: Before bringing a port online, allocate the wildcard peripheral for that bus. And after taking a port offline, invalidate the wildcard peripheral for that bus. Make sure that we hold the SIM lock around all calls to xpt_action() and other transport layer interfaces that require it. Use CAM_SIM_{LOCK|UNLOCK} consistently to acquire and release the SIM lock. Update a number of outdated comments. Some of these should have been fixed long ago. Actually do LUN disbables now. The newer drivers in the tree work correctly for this as far as I know. Initialize the CCB type to CTLFE_CCB_DEFAULT to avoid a panic due to uninitialized memory. Submitted by: Chuck Tuffli (partially) MFC after: 1 week Modified: head/sys/cam/ctl/ctl.c head/sys/cam/ctl/scsi_ctl.c Modified: head/sys/cam/ctl/ctl.c ============================================================================== --- head/sys/cam/ctl/ctl.c Wed Jan 9 16:51:47 2013 (r245227) +++ head/sys/cam/ctl/ctl.c Wed Jan 9 17:02:08 2013 (r245228) @@ -2257,11 +2257,31 @@ ctl_ioctl(struct cdev *dev, u_long cmd, */ mtx_unlock(&softc->ctl_lock); - if (cmd == CTL_ENABLE_PORT) + if (cmd == CTL_ENABLE_PORT) { + struct ctl_lun *lun; + + STAILQ_FOREACH(lun, &softc->lun_list, + links) { + fe->lun_enable(fe->targ_lun_arg, + lun->target, + lun->lun); + } + ctl_frontend_online(fe); - else if (cmd == CTL_DISABLE_PORT) + } else if (cmd == CTL_DISABLE_PORT) { + struct ctl_lun *lun; + ctl_frontend_offline(fe); + STAILQ_FOREACH(lun, &softc->lun_list, + links) { + fe->lun_disable( + fe->targ_lun_arg, + lun->target, + lun->lun); + } + } + mtx_lock(&softc->ctl_lock); if (cmd == CTL_SET_PORT_WWNS) Modified: head/sys/cam/ctl/scsi_ctl.c ============================================================================== --- head/sys/cam/ctl/scsi_ctl.c Wed Jan 9 16:51:47 2013 (r245227) +++ head/sys/cam/ctl/scsi_ctl.c Wed Jan 9 17:02:08 2013 (r245228) @@ -73,6 +73,7 @@ __FBSDID("$FreeBSD$"); #include typedef enum { + CTLFE_CCB_DEFAULT = 0x00, CTLFE_CCB_WAITING = 0x01 } ctlfe_ccb_types; @@ -304,10 +305,7 @@ ctlfeasync(void *callback_arg, uint32_t case AC_PATH_REGISTERED: { struct ctl_frontend *fe; struct ctlfe_softc *bus_softc; - struct ctlfe_lun_softc *lun_softc; - struct cam_path *path; struct ccb_pathinq *cpi; - cam_status status; int retval; cpi = (struct ccb_pathinq *)arg; @@ -330,7 +328,6 @@ ctlfeasync(void *callback_arg, uint32_t M_NOWAIT | M_ZERO); if (ccb == NULL) { printf("%s: unable to malloc CCB!\n", __func__); - xpt_free_path(path); return; } xpt_setup_ccb(&ccb->ccb_h, cpi->ccb_h.path, @@ -448,44 +445,31 @@ ctlfeasync(void *callback_arg, uint32_t mtx_unlock(&ctlfe_list_mtx); } - status = xpt_create_path(&path, /*periph*/ NULL, - bus_softc->path_id,CAM_TARGET_WILDCARD, - CAM_LUN_WILDCARD); - if (status != CAM_REQ_CMP) { - printf("%s: unable to create path for wildcard " - "periph\n", __func__); - break; - } - lun_softc = malloc(sizeof(*lun_softc), M_CTLFE, - M_NOWAIT | M_ZERO); - if (lun_softc == NULL) { - xpt_print(path, "%s: unable to allocate softc for " - "wildcard periph\n", __func__); - xpt_free_path(path); - break; - } - - lun_softc->parent_softc = bus_softc; - lun_softc->flags |= CTLFE_LUN_WILDCARD; - - status = cam_periph_alloc(ctlferegister, - ctlfeoninvalidate, - ctlfecleanup, - ctlfestart, - "ctl", - CAM_PERIPH_BIO, - path, - ctlfeasync, - 0, - lun_softc); + break; + } + case AC_PATH_DEREGISTERED: { + struct ctlfe_softc *softc = NULL; - xpt_free_path(path); + mtx_lock(&ctlfe_list_mtx); + STAILQ_FOREACH(softc, &ctlfe_softc_list, links) { + if (softc->path_id == xpt_path_path_id(path)) { + STAILQ_REMOVE(&ctlfe_softc_list, softc, + ctlfe_softc, links); + break; + } + } + mtx_unlock(&ctlfe_list_mtx); + if (softc != NULL) { + /* + * XXX KDM are we certain at this point that there + * are no outstanding commands for this frontend? + */ + ctl_frontend_deregister(&softc->fe); + free(softc, M_CTLFE); + } break; } - case AC_PATH_DEREGISTERED: - /* ctl_frontend_deregister() */ - break; case AC_CONTRACT: { struct ac_contract *ac; @@ -699,11 +683,14 @@ ctlfecleanup(struct cam_periph *periph) softc = (struct ctlfe_lun_softc *)periph->softc; bus_softc = softc->parent_softc; - STAILQ_REMOVE(&bus_softc->lun_softc_list, softc, ctlfe_lun_softc,links); + STAILQ_REMOVE(&bus_softc->lun_softc_list, softc, ctlfe_lun_softc, links); /* * XXX KDM is there anything else that needs to be done here? */ + + callout_stop(&softc->dma_callout); + free(softc, M_CTLFE); } @@ -717,6 +704,8 @@ ctlfestart(struct cam_periph *periph, un softc->ccbs_alloced++; + start_ccb->ccb_h.ccb_type = CTLFE_CCB_DEFAULT; + ccb_h = TAILQ_FIRST(&softc->work_queue); if (periph->immediate_priority <= periph->pinfo.priority) { panic("shouldn't get to the CCB waiting case!"); @@ -857,8 +846,6 @@ ctlfestart(struct cam_periph *periph, un /* * Datamove call, we need to setup the S/G list. - * If we pass in a S/G list, the isp(4) driver at - * least expects physical/bus addresses. */ cmd_info = (struct ctlfe_lun_cmd_info *) @@ -933,12 +920,13 @@ ctlfestart(struct cam_periph *periph, un int *ti; /* - * XXX KDM this is a temporary hack. The - * isp(4) driver can't deal with S/G lists - * with virtual pointers, so we need to - * go through and send down one virtual - * pointer at a time. + * If we have more S/G list pointers than + * will fit in the available storage in the + * cmd_info structure inside the ctl_io header, + * then we need to send down the pointers + * one element at a time. */ + sglist = (struct ctl_sg_entry *) io->scsiio.kern_data_ptr; ti = &cmd_info->cur_transfer_index; @@ -1405,13 +1393,15 @@ ctlfedone(struct cam_periph *periph, uni break; default: /* - * XXX KDM the isp(4) driver doesn't really - * seem to send errors back for data - * transfers that I can tell. There is one - * case where it'll send CAM_REQ_CMP_ERR, - * but probably not that many more cases. - * So set a generic data phase error here, - * like the SXP driver sets. + * XXX KDM we probably need to figure out a + * standard set of errors that the SIM + * drivers should return in the event of a + * data transfer failure. A data phase + * error will at least point the user to a + * data transfer error of some sort. + * Hopefully the SIM printed out some + * additional information to give the user + * a clue what happened. */ io->io_hdr.port_status = 0xbad1; ctl_set_data_phase_error(&io->scsiio); @@ -1689,20 +1679,22 @@ ctlfe_onoffline(void *arg, int online) sim = bus_softc->sim; - CAM_SIM_LOCK(sim); + mtx_assert(sim->mtx, MA_OWNED); + status = xpt_create_path(&path, /*periph*/ NULL, bus_softc->path_id, CAM_TARGET_WILDCARD, CAM_LUN_WILDCARD); if (status != CAM_REQ_CMP) { printf("%s: unable to create path!\n", __func__); - CAM_SIM_UNLOCK(sim); return; } - CAM_SIM_UNLOCK(sim); - - ccb = (union ccb *)malloc(sizeof(*ccb), M_TEMP, M_WAITOK | M_ZERO); + ccb = (union ccb *)malloc(sizeof(*ccb), M_TEMP, M_NOWAIT | M_ZERO); + if (ccb == NULL) { + printf("%s: unable to malloc CCB!\n", __func__); + xpt_free_path(path); + return; + } xpt_setup_ccb(&ccb->ccb_h, path, CAM_PRIORITY_NONE); - /* * Copan WWN format: * @@ -1720,11 +1712,9 @@ ctlfe_onoffline(void *arg, int online) ccb->ccb_h.func_code = XPT_GET_SIM_KNOB; - CAM_SIM_LOCK(sim); xpt_action(ccb); - CAM_SIM_UNLOCK(sim); if ((ccb->knob.xport_specific.valid & KNOB_VALID_ADDRESS) != 0){ #ifdef RANDOM_WWNN @@ -1822,9 +1812,6 @@ ctlfe_onoffline(void *arg, int online) else ccb->knob.xport_specific.fc.role = KNOB_ROLE_NONE; - - CAM_SIM_LOCK(sim); - xpt_action(ccb); if ((ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) { @@ -1841,8 +1828,6 @@ ctlfe_onoffline(void *arg, int online) xpt_free_path(path); - CAM_SIM_UNLOCK(sim); - free(ccb, M_TEMP); return; @@ -1851,13 +1836,111 @@ ctlfe_onoffline(void *arg, int online) static void ctlfe_online(void *arg) { + struct ctlfe_softc *bus_softc; + struct cam_path *path; + cam_status status; + struct ctlfe_lun_softc *lun_softc; + struct cam_sim *sim; + + bus_softc = (struct ctlfe_softc *)arg; + sim = bus_softc->sim; + + CAM_SIM_LOCK(sim); + + /* + * Create the wildcard LUN before bringing the port online. + */ + status = xpt_create_path(&path, /*periph*/ NULL, + bus_softc->path_id, CAM_TARGET_WILDCARD, + CAM_LUN_WILDCARD); + if (status != CAM_REQ_CMP) { + printf("%s: unable to create path for wildcard periph\n", + __func__); + CAM_SIM_UNLOCK(sim); + return; + } + + lun_softc = malloc(sizeof(*lun_softc), M_CTLFE, + M_NOWAIT | M_ZERO); + if (lun_softc == NULL) { + xpt_print(path, "%s: unable to allocate softc for " + "wildcard periph\n", __func__); + xpt_free_path(path); + CAM_SIM_UNLOCK(sim); + return; + } + + lun_softc->parent_softc = bus_softc; + lun_softc->flags |= CTLFE_LUN_WILDCARD; + + STAILQ_INSERT_TAIL(&bus_softc->lun_softc_list, lun_softc, links); + + + status = cam_periph_alloc(ctlferegister, + ctlfeoninvalidate, + ctlfecleanup, + ctlfestart, + "ctl", + CAM_PERIPH_BIO, + path, + ctlfeasync, + 0, + lun_softc); + + if ((status & CAM_STATUS_MASK) != CAM_REQ_CMP) { + const struct cam_status_entry *entry; + + entry = cam_fetch_status_entry(status); + + printf("%s: CAM error %s (%#x) returned from " + "cam_periph_alloc()\n", __func__, (entry != NULL) ? + entry->status_text : "Unknown", status); + } + + xpt_free_path(path); + ctlfe_onoffline(arg, /*online*/ 1); + + CAM_SIM_UNLOCK(sim); } static void ctlfe_offline(void *arg) { + struct ctlfe_softc *bus_softc; + struct cam_path *path; + cam_status status; + struct cam_periph *periph; + struct cam_sim *sim; + + bus_softc = (struct ctlfe_softc *)arg; + sim = bus_softc->sim; + + CAM_SIM_LOCK(sim); + ctlfe_onoffline(arg, /*online*/ 0); + + /* + * Disable the wildcard LUN for this port now that we have taken + * the port offline. + */ + status = xpt_create_path(&path, /*periph*/ NULL, + bus_softc->path_id, CAM_TARGET_WILDCARD, + CAM_LUN_WILDCARD); + if (status != CAM_REQ_CMP) { + CAM_SIM_UNLOCK(sim); + printf("%s: unable to create path for wildcard periph\n", + __func__); + return; + } + + + if ((periph = cam_periph_find(path, "ctl")) != NULL) + cam_periph_invalidate(periph); + + xpt_free_path(path); + + CAM_SIM_UNLOCK(sim); } static int @@ -1890,21 +1973,17 @@ ctlfe_lun_enable(void *arg, struct ctl_i bus_softc = (struct ctlfe_softc *)arg; sim = bus_softc->sim; - CAM_SIM_LOCK(sim); - - status = xpt_create_path(&path, /*periph*/ NULL, bus_softc->path_id, - targ_id.id, lun_id); + status = xpt_create_path_unlocked(&path, /*periph*/ NULL, + bus_softc->path_id, + targ_id.id, lun_id); /* XXX KDM need some way to return status to CTL here? */ if (status != CAM_REQ_CMP) { printf("%s: could not create path, status %#x\n", __func__, status); - CAM_SIM_UNLOCK(sim); return (1); } - CAM_SIM_UNLOCK(sim); softc = malloc(sizeof(*softc), M_CTLFE, M_WAITOK | M_ZERO); - CAM_SIM_LOCK(sim); periph = cam_periph_find(path, "ctl"); if (periph != NULL) { @@ -1937,23 +2016,20 @@ ctlfe_lun_enable(void *arg, struct ctl_i } /* - * XXX KDM we disable LUN removal here. The problem is that the isp(4) - * driver doesn't currently handle LUN removal properly. We need to keep - * enough state here at the peripheral level even after LUNs have been - * removed inside CTL. - * - * Once the isp(4) driver is fixed, this can be re-enabled. + * This will get called when the user removes a LUN to disable that LUN + * on every bus that is attached to CTL. */ static int ctlfe_lun_disable(void *arg, struct ctl_id targ_id, int lun_id) { -#ifdef NOTYET struct ctlfe_softc *softc; struct ctlfe_lun_softc *lun_softc; + struct cam_sim *sim; softc = (struct ctlfe_softc *)arg; + sim = softc->sim; - mtx_lock(softc->sim->mtx); + CAM_SIM_LOCK(sim); STAILQ_FOREACH(lun_softc, &softc->lun_softc_list, links) { struct cam_path *path; @@ -1965,7 +2041,7 @@ ctlfe_lun_disable(void *arg, struct ctl_ } } if (lun_softc == NULL) { - mtx_unlock(softc->sim->mtx); + CAM_SIM_UNLOCK(sim); printf("%s: can't find target %d lun %d\n", __func__, targ_id.id, lun_id); return (1); @@ -1973,8 +2049,7 @@ ctlfe_lun_disable(void *arg, struct ctl_ cam_periph_invalidate(lun_softc->periph); - mtx_unlock(softc->sim->mtx); -#endif + CAM_SIM_UNLOCK(sim); return (0); } @@ -2139,7 +2214,7 @@ ctlfe_datamove_done(union ctl_io *io) sim = xpt_path_sim(ccb->ccb_h.path); - mtx_lock(sim->mtx); + CAM_SIM_LOCK(sim); periph = xpt_path_periph(ccb->ccb_h.path); @@ -2186,7 +2261,7 @@ ctlfe_datamove_done(union ctl_io *io) xpt_schedule(periph, /*priority*/ 1); } - mtx_unlock(sim->mtx); + CAM_SIM_UNLOCK(sim); } static void