From owner-svn-src-stable-9@FreeBSD.ORG Thu Apr 18 10:40:41 2013 Return-Path: Delivered-To: svn-src-stable-9@freebsd.org Received: from mx1.freebsd.org (mx1.FreeBSD.org [8.8.178.115]) by hub.freebsd.org (Postfix) with ESMTP id 9A3FEE10; Thu, 18 Apr 2013 10:40:41 +0000 (UTC) (envelope-from mav@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 8AB02B38; Thu, 18 Apr 2013 10:40:41 +0000 (UTC) Received: from svn.freebsd.org ([127.0.1.70]) by svn.freebsd.org (8.14.6/8.14.6) with ESMTP id r3IAefBM012738; Thu, 18 Apr 2013 10:40:41 GMT (envelope-from mav@svn.freebsd.org) Received: (from mav@localhost) by svn.freebsd.org (8.14.6/8.14.5/Submit) id r3IAeeJP012733; Thu, 18 Apr 2013 10:40:40 GMT (envelope-from mav@svn.freebsd.org) Message-Id: <201304181040.r3IAeeJP012733@svn.freebsd.org> From: Alexander Motin Date: Thu, 18 Apr 2013 10:40:40 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-9@freebsd.org Subject: svn commit: r249617 - in stable/9/sys/cam: . ata scsi X-SVN-Group: stable-9 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-9@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: SVN commit messages for only the 9-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Apr 2013 10:40:41 -0000 Author: mav Date: Thu Apr 18 10:40:40 2013 New Revision: 249617 URL: http://svnweb.freebsd.org/changeset/base/249617 Log: MFC r249108: - Unify device to target insertion inside xpt_alloc_device() instead of duplicating it three times. - Reformat code to reduce indentation. - Add lock assertions to every point where reference counters are modified. - When reference counters are reaching zero, add assertions that there are no children items left. - Add a bit more locking to the xptpdperiphtraverse(). Modified: stable/9/sys/cam/ata/ata_xpt.c stable/9/sys/cam/cam_periph.c stable/9/sys/cam/cam_sim.c stable/9/sys/cam/cam_xpt.c stable/9/sys/cam/scsi/scsi_xpt.c Directory Properties: stable/9/sys/ (props changed) Modified: stable/9/sys/cam/ata/ata_xpt.c ============================================================================== --- stable/9/sys/cam/ata/ata_xpt.c Thu Apr 18 10:08:27 2013 (r249616) +++ stable/9/sys/cam/ata/ata_xpt.c Thu Apr 18 10:40:40 2013 (r249617) @@ -1534,7 +1534,6 @@ ata_alloc_device(struct cam_eb *bus, str struct cam_path path; struct ata_quirk_entry *quirk; struct cam_ed *device; - struct cam_ed *cur_device; device = xpt_alloc_device(bus, target, lun_id); if (device == NULL) @@ -1559,16 +1558,6 @@ ata_alloc_device(struct cam_eb *bus, str * do. */ bus->sim->max_ccbs += device->ccbq.devq_openings; - /* Insertion sort into our target's device list */ - cur_device = TAILQ_FIRST(&target->ed_entries); - while (cur_device != NULL && cur_device->lun_id < lun_id) - cur_device = TAILQ_NEXT(cur_device, links); - if (cur_device != NULL) { - TAILQ_INSERT_BEFORE(cur_device, device, links); - } else { - TAILQ_INSERT_TAIL(&target->ed_entries, device, links); - } - target->generation++; if (lun_id != CAM_LUN_WILDCARD) { xpt_compile_path(&path, NULL, Modified: stable/9/sys/cam/cam_periph.c ============================================================================== --- stable/9/sys/cam/cam_periph.c Thu Apr 18 10:08:27 2013 (r249616) +++ stable/9/sys/cam/cam_periph.c Thu Apr 18 10:40:40 2013 (r249617) @@ -378,13 +378,10 @@ cam_periph_acquire(struct cam_periph *pe void cam_periph_release_locked_buses(struct cam_periph *periph) { - if (periph->refcount != 0) { - periph->refcount--; - } else { - panic("%s: release of %p when refcount is zero\n ", __func__, - periph); - } - if (periph->refcount == 0 + + mtx_assert(periph->sim->mtx, MA_OWNED); + KASSERT(periph->refcount >= 1, ("periph->refcount >= 1")); + if (--periph->refcount == 0 && (periph->flags & CAM_PERIPH_INVALID)) { camperiphfree(periph); } @@ -583,6 +580,7 @@ cam_periph_invalidate(struct cam_periph { CAM_DEBUG(periph->path, CAM_DEBUG_INFO, ("Periph invalidated\n")); + mtx_assert(periph->sim->mtx, MA_OWNED); /* * We only call this routine the first time a peripheral is * invalidated. @@ -605,6 +603,7 @@ camperiphfree(struct cam_periph *periph) { struct periph_driver **p_drv; + mtx_assert(periph->sim->mtx, MA_OWNED); for (p_drv = periph_drivers; *p_drv != NULL; p_drv++) { if (strcmp((*p_drv)->driver_name, periph->periph_name) == 0) break; Modified: stable/9/sys/cam/cam_sim.c ============================================================================== --- stable/9/sys/cam/cam_sim.c Thu Apr 18 10:08:27 2013 (r249616) +++ stable/9/sys/cam/cam_sim.c Thu Apr 18 10:40:40 2013 (r249617) @@ -109,6 +109,7 @@ cam_sim_free(struct cam_sim *sim, int fr union ccb *ccb; int error; + mtx_assert(sim->mtx, MA_OWNED); sim->refcount--; if (sim->refcount > 0) { error = msleep(sim, sim->mtx, PRIBIO, "simfree", 0); Modified: stable/9/sys/cam/cam_xpt.c ============================================================================== --- stable/9/sys/cam/cam_xpt.c Thu Apr 18 10:08:27 2013 (r249616) +++ stable/9/sys/cam/cam_xpt.c Thu Apr 18 10:40:40 2013 (r249617) @@ -2101,6 +2101,7 @@ xpttargettraverse(struct cam_eb *bus, st struct cam_et *target, *next_target; int retval; + mtx_assert(bus->sim->mtx, MA_OWNED); retval = 1; for (target = (start_target ? start_target : TAILQ_FIRST(&bus->et_entries)); @@ -2128,6 +2129,7 @@ xptdevicetraverse(struct cam_et *target, struct cam_ed *device, *next_device; int retval; + mtx_assert(target->bus->sim->mtx, MA_OWNED); retval = 1; for (device = (start_device ? start_device : TAILQ_FIRST(&target->ed_entries)); @@ -2166,6 +2168,7 @@ xptperiphtraverse(struct cam_ed *device, retval = 1; + mtx_assert(device->sim->mtx, MA_OWNED); xpt_lock_buses(); for (periph = (start_periph ? start_periph : SLIST_FIRST(&device->periphs)); @@ -2247,6 +2250,7 @@ xptpdperiphtraverse(struct periph_driver xpt_periphfunc_t *tr_func, void *arg) { struct cam_periph *periph, *next_periph; + struct cam_sim *sim; int retval; retval = 1; @@ -2275,7 +2279,10 @@ xptpdperiphtraverse(struct periph_driver * traversal function, so it can't go away. */ periph->refcount++; - + sim = periph->sim; + xpt_unlock_buses(); + CAM_SIM_LOCK(sim); + xpt_lock_buses(); retval = tr_func(periph, arg); /* @@ -2285,6 +2292,7 @@ xptpdperiphtraverse(struct periph_driver next_periph = TAILQ_NEXT(periph, unit_links); cam_periph_release_locked_buses(periph); + CAM_SIM_UNLOCK(sim); if (retval == 0) goto bailout_done; @@ -3566,13 +3574,13 @@ xpt_path_counts(struct cam_path *path, u else *bus_ref = 0; } - xpt_unlock_buses(); if (periph_ref) { if (path->periph) *periph_ref = path->periph->refcount; else *periph_ref = 0; } + xpt_unlock_buses(); if (target_ref) { if (path->target) *target_ref = path->target->refcount; @@ -4447,54 +4455,55 @@ xpt_release_bus(struct cam_eb *bus) xpt_lock_buses(); KASSERT(bus->refcount >= 1, ("bus->refcount >= 1")); - if ((--bus->refcount == 0) - && (TAILQ_FIRST(&bus->et_entries) == NULL)) { - TAILQ_REMOVE(&xsoftc.xpt_busses, bus, links); - xsoftc.bus_generation++; - xpt_unlock_buses(); - cam_sim_release(bus->sim); - free(bus, M_CAMXPT); - } else + if (--bus->refcount > 0) { xpt_unlock_buses(); + return; + } + KASSERT(TAILQ_EMPTY(&bus->et_entries), + ("refcount is zero, but target list is not empty")); + TAILQ_REMOVE(&xsoftc.xpt_busses, bus, links); + xsoftc.bus_generation++; + xpt_unlock_buses(); + cam_sim_release(bus->sim); + free(bus, M_CAMXPT); } static struct cam_et * xpt_alloc_target(struct cam_eb *bus, target_id_t target_id) { - struct cam_et *target; + struct cam_et *cur_target, *target; + mtx_assert(bus->sim->mtx, MA_OWNED); target = (struct cam_et *)malloc(sizeof(*target), M_CAMXPT, M_NOWAIT|M_ZERO); - if (target != NULL) { - struct cam_et *cur_target; - - TAILQ_INIT(&target->ed_entries); - target->bus = bus; - target->target_id = target_id; - target->refcount = 1; - target->generation = 0; - target->luns = NULL; - timevalclear(&target->last_reset); - /* - * Hold a reference to our parent bus so it - * will not go away before we do. - */ - xpt_lock_buses(); - bus->refcount++; - xpt_unlock_buses(); + if (target == NULL) + return (NULL); - /* Insertion sort into our bus's target list */ - cur_target = TAILQ_FIRST(&bus->et_entries); - while (cur_target != NULL && cur_target->target_id < target_id) - cur_target = TAILQ_NEXT(cur_target, links); + TAILQ_INIT(&target->ed_entries); + target->bus = bus; + target->target_id = target_id; + target->refcount = 1; + target->generation = 0; + target->luns = NULL; + timevalclear(&target->last_reset); + /* + * Hold a reference to our parent bus so it + * will not go away before we do. + */ + xpt_lock_buses(); + bus->refcount++; + xpt_unlock_buses(); - if (cur_target != NULL) { - TAILQ_INSERT_BEFORE(cur_target, target, links); - } else { - TAILQ_INSERT_TAIL(&bus->et_entries, target, links); - } - bus->generation++; + /* Insertion sort into our bus's target list */ + cur_target = TAILQ_FIRST(&bus->et_entries); + while (cur_target != NULL && cur_target->target_id < target_id) + cur_target = TAILQ_NEXT(cur_target, links); + if (cur_target != NULL) { + TAILQ_INSERT_BEFORE(cur_target, target, links); + } else { + TAILQ_INSERT_TAIL(&bus->et_entries, target, links); } + bus->generation++; return (target); } @@ -4502,24 +4511,24 @@ static void xpt_release_target(struct cam_et *target) { - if (target->refcount == 1) { - if (TAILQ_FIRST(&target->ed_entries) == NULL) { - TAILQ_REMOVE(&target->bus->et_entries, target, links); - target->bus->generation++; - xpt_release_bus(target->bus); - if (target->luns) - free(target->luns, M_CAMXPT); - free(target, M_CAMXPT); - } - } else - target->refcount--; + mtx_assert(target->bus->sim->mtx, MA_OWNED); + if (--target->refcount > 0) + return; + KASSERT(TAILQ_EMPTY(&target->ed_entries), + ("refcount is zero, but device list is not empty")); + TAILQ_REMOVE(&target->bus->et_entries, target, links); + target->bus->generation++; + xpt_release_bus(target->bus); + if (target->luns) + free(target->luns, M_CAMXPT); + free(target, M_CAMXPT); } static struct cam_ed * xpt_alloc_device_default(struct cam_eb *bus, struct cam_et *target, lun_id_t lun_id) { - struct cam_ed *device, *cur_device; + struct cam_ed *device; device = xpt_alloc_device(bus, target, lun_id); if (device == NULL) @@ -4528,73 +4537,65 @@ xpt_alloc_device_default(struct cam_eb * device->mintags = 1; device->maxtags = 1; bus->sim->max_ccbs += device->ccbq.devq_openings; - cur_device = TAILQ_FIRST(&target->ed_entries); - while (cur_device != NULL && cur_device->lun_id < lun_id) - cur_device = TAILQ_NEXT(cur_device, links); - if (cur_device != NULL) { - TAILQ_INSERT_BEFORE(cur_device, device, links); - } else { - TAILQ_INSERT_TAIL(&target->ed_entries, device, links); - } - target->generation++; - return (device); } struct cam_ed * xpt_alloc_device(struct cam_eb *bus, struct cam_et *target, lun_id_t lun_id) { - struct cam_ed *device; - struct cam_devq *devq; + struct cam_ed *cur_device, *device; + struct cam_devq *devq; cam_status status; + mtx_assert(target->bus->sim->mtx, MA_OWNED); /* Make space for us in the device queue on our bus */ devq = bus->sim->devq; status = cam_devq_resize(devq, devq->alloc_queue.array_size + 1); + if (status != CAM_REQ_CMP) + return (NULL); - if (status != CAM_REQ_CMP) { - device = NULL; - } else { - device = (struct cam_ed *)malloc(sizeof(*device), - M_CAMDEV, M_NOWAIT|M_ZERO); - } - - if (device != NULL) { - cam_init_pinfo(&device->alloc_ccb_entry.pinfo); - device->alloc_ccb_entry.device = device; - cam_init_pinfo(&device->send_ccb_entry.pinfo); - device->send_ccb_entry.device = device; - device->target = target; - device->lun_id = lun_id; - device->sim = bus->sim; - /* Initialize our queues */ - if (camq_init(&device->drvq, 0) != 0) { - free(device, M_CAMDEV); - return (NULL); - } - if (cam_ccbq_init(&device->ccbq, - bus->sim->max_dev_openings) != 0) { - camq_fini(&device->drvq); - free(device, M_CAMDEV); - return (NULL); - } - SLIST_INIT(&device->asyncs); - SLIST_INIT(&device->periphs); - device->generation = 0; - device->owner = NULL; - device->flags = CAM_DEV_UNCONFIGURED; - device->tag_delay_count = 0; - device->tag_saved_openings = 0; - device->refcount = 1; - callout_init_mtx(&device->callout, bus->sim->mtx, 0); - - /* - * Hold a reference to our parent target so it - * will not go away before we do. - */ - target->refcount++; + device = (struct cam_ed *)malloc(sizeof(*device), + M_CAMDEV, M_NOWAIT|M_ZERO); + if (device == NULL) + return (NULL); + cam_init_pinfo(&device->alloc_ccb_entry.pinfo); + device->alloc_ccb_entry.device = device; + cam_init_pinfo(&device->send_ccb_entry.pinfo); + device->send_ccb_entry.device = device; + device->target = target; + device->lun_id = lun_id; + device->sim = bus->sim; + /* Initialize our queues */ + if (camq_init(&device->drvq, 0) != 0) { + free(device, M_CAMDEV); + return (NULL); + } + if (cam_ccbq_init(&device->ccbq, + bus->sim->max_dev_openings) != 0) { + camq_fini(&device->drvq); + free(device, M_CAMDEV); + return (NULL); } + SLIST_INIT(&device->asyncs); + SLIST_INIT(&device->periphs); + device->generation = 0; + device->owner = NULL; + device->flags = CAM_DEV_UNCONFIGURED; + device->tag_delay_count = 0; + device->tag_saved_openings = 0; + device->refcount = 1; + callout_init_mtx(&device->callout, bus->sim->mtx, 0); + + cur_device = TAILQ_FIRST(&target->ed_entries); + while (cur_device != NULL && cur_device->lun_id < lun_id) + cur_device = TAILQ_NEXT(cur_device, links); + if (cur_device != NULL) + TAILQ_INSERT_BEFORE(cur_device, device, links); + else + TAILQ_INSERT_TAIL(&target->ed_entries, device, links); + target->refcount++; + target->generation++; return (device); } @@ -4602,46 +4603,49 @@ void xpt_acquire_device(struct cam_ed *device) { + mtx_assert(device->sim->mtx, MA_OWNED); device->refcount++; } void xpt_release_device(struct cam_ed *device) { + struct cam_devq *devq; - if (device->refcount == 1) { - struct cam_devq *devq; + mtx_assert(device->sim->mtx, MA_OWNED); + if (--device->refcount > 0) + return; - if (device->alloc_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX - || device->send_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX) - panic("Removing device while still queued for ccbs"); - - if ((device->flags & CAM_DEV_REL_TIMEOUT_PENDING) != 0) - callout_stop(&device->callout); - - TAILQ_REMOVE(&device->target->ed_entries, device,links); - device->target->generation++; - device->target->bus->sim->max_ccbs -= device->ccbq.devq_openings; - /* Release our slot in the devq */ - devq = device->target->bus->sim->devq; - cam_devq_resize(devq, devq->alloc_queue.array_size - 1); - camq_fini(&device->drvq); - cam_ccbq_fini(&device->ccbq); - /* - * Free allocated memory. free(9) does nothing if the - * supplied pointer is NULL, so it is safe to call without - * checking. - */ - free(device->supported_vpds, M_CAMXPT); - free(device->device_id, M_CAMXPT); - free(device->physpath, M_CAMXPT); - free(device->rcap_buf, M_CAMXPT); - free(device->serial_num, M_CAMXPT); + KASSERT(SLIST_EMPTY(&device->periphs), + ("refcount is zero, but periphs list is not empty")); + if (device->alloc_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX + || device->send_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX) + panic("Removing device while still queued for ccbs"); + + if ((device->flags & CAM_DEV_REL_TIMEOUT_PENDING) != 0) + callout_stop(&device->callout); + + TAILQ_REMOVE(&device->target->ed_entries, device,links); + device->target->generation++; + device->target->bus->sim->max_ccbs -= device->ccbq.devq_openings; + /* Release our slot in the devq */ + devq = device->target->bus->sim->devq; + cam_devq_resize(devq, devq->alloc_queue.array_size - 1); + camq_fini(&device->drvq); + cam_ccbq_fini(&device->ccbq); + /* + * Free allocated memory. free(9) does nothing if the + * supplied pointer is NULL, so it is safe to call without + * checking. + */ + free(device->supported_vpds, M_CAMXPT); + free(device->device_id, M_CAMXPT); + free(device->physpath, M_CAMXPT); + free(device->rcap_buf, M_CAMXPT); + free(device->serial_num, M_CAMXPT); - xpt_release_target(device->target); - free(device, M_CAMDEV); - } else - device->refcount--; + xpt_release_target(device->target); + free(device, M_CAMDEV); } u_int32_t @@ -4689,6 +4693,7 @@ xpt_find_target(struct cam_eb *bus, targ { struct cam_et *target; + mtx_assert(bus->sim->mtx, MA_OWNED); for (target = TAILQ_FIRST(&bus->et_entries); target != NULL; target = TAILQ_NEXT(target, links)) { @@ -4705,6 +4710,7 @@ xpt_find_device(struct cam_et *target, l { struct cam_ed *device; + mtx_assert(target->bus->sim->mtx, MA_OWNED); for (device = TAILQ_FIRST(&target->ed_entries); device != NULL; device = TAILQ_NEXT(device, links)) { Modified: stable/9/sys/cam/scsi/scsi_xpt.c ============================================================================== --- stable/9/sys/cam/scsi/scsi_xpt.c Thu Apr 18 10:08:27 2013 (r249616) +++ stable/9/sys/cam/scsi/scsi_xpt.c Thu Apr 18 10:40:40 2013 (r249617) @@ -2306,7 +2306,6 @@ scsi_alloc_device(struct cam_eb *bus, st struct cam_path path; struct scsi_quirk_entry *quirk; struct cam_ed *device; - struct cam_ed *cur_device; device = xpt_alloc_device(bus, target, lun_id); if (device == NULL) @@ -2335,16 +2334,6 @@ scsi_alloc_device(struct cam_eb *bus, st * do. */ bus->sim->max_ccbs += device->ccbq.devq_openings; - /* Insertion sort into our target's device list */ - cur_device = TAILQ_FIRST(&target->ed_entries); - while (cur_device != NULL && cur_device->lun_id < lun_id) - cur_device = TAILQ_NEXT(cur_device, links); - if (cur_device != NULL) { - TAILQ_INSERT_BEFORE(cur_device, device, links); - } else { - TAILQ_INSERT_TAIL(&target->ed_entries, device, links); - } - target->generation++; if (lun_id != CAM_LUN_WILDCARD) { xpt_compile_path(&path, NULL,