From owner-svn-src-stable-8@FreeBSD.ORG Tue Nov 17 20:49:26 2009 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A314E1065697; Tue, 17 Nov 2009 20:49:26 +0000 (UTC) (envelope-from mav@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 914638FC20; Tue, 17 Nov 2009 20:49:26 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id nAHKnQcB037658; Tue, 17 Nov 2009 20:49:26 GMT (envelope-from mav@svn.freebsd.org) Received: (from mav@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id nAHKnQwu037653; Tue, 17 Nov 2009 20:49:26 GMT (envelope-from mav@svn.freebsd.org) Message-Id: <200911172049.nAHKnQwu037653@svn.freebsd.org> From: Alexander Motin Date: Tue, 17 Nov 2009 20:49:26 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r199435 - in stable/8/sys/cam: . ata scsi X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 17 Nov 2009 20:49:26 -0000 Author: mav Date: Tue Nov 17 20:49:26 2009 New Revision: 199435 URL: http://svn.freebsd.org/changeset/base/199435 Log: MFC r198748, r198782: Fix reference counting bug, when device unreferenced before invalidated. To do it, do not handle validity flag as another reference, but explicitly modify reference count each time flag is modified. The async callback could free the device. If it is a broadcast async, it doesn't hold device reference, so take our own reference. Modified: stable/8/sys/cam/ata/ata_xpt.c stable/8/sys/cam/cam_xpt.c stable/8/sys/cam/cam_xpt_internal.h stable/8/sys/cam/scsi/scsi_xpt.c Directory Properties: stable/8/sys/ (props changed) stable/8/sys/amd64/include/xen/ (props changed) stable/8/sys/cddl/contrib/opensolaris/ (props changed) stable/8/sys/contrib/dev/acpica/ (props changed) stable/8/sys/contrib/pf/ (props changed) stable/8/sys/dev/xen/xenpci/ (props changed) Modified: stable/8/sys/cam/ata/ata_xpt.c ============================================================================== --- stable/8/sys/cam/ata/ata_xpt.c Tue Nov 17 20:49:09 2009 (r199434) +++ stable/8/sys/cam/ata/ata_xpt.c Tue Nov 17 20:49:26 2009 (r199435) @@ -733,6 +733,7 @@ noerror: case PROBE_SET_MULTI: if (periph->path->device->flags & CAM_DEV_UNCONFIGURED) { path->device->flags &= ~CAM_DEV_UNCONFIGURED; + xpt_acquire_device(path->device); done_ccb->ccb_h.func_code = XPT_GDEV_TYPE; xpt_action(done_ccb); xpt_async(AC_FOUND_DEVICE, done_ccb->ccb_h.path, @@ -777,6 +778,7 @@ noerror: ata_device_transport(path); if (periph->path->device->flags & CAM_DEV_UNCONFIGURED) { path->device->flags &= ~CAM_DEV_UNCONFIGURED; + xpt_acquire_device(path->device); done_ccb->ccb_h.func_code = XPT_GDEV_TYPE; xpt_action(done_ccb); xpt_async(AC_FOUND_DEVICE, done_ccb->ccb_h.path, done_ccb); @@ -810,6 +812,7 @@ noerror: path->device->flags |= CAM_DEV_IDENTIFY_DATA_VALID; if (periph->path->device->flags & CAM_DEV_UNCONFIGURED) { path->device->flags &= ~CAM_DEV_UNCONFIGURED; + xpt_acquire_device(path->device); done_ccb->ccb_h.func_code = XPT_GDEV_TYPE; xpt_action(done_ccb); xpt_async(AC_FOUND_DEVICE, done_ccb->ccb_h.path, @@ -1485,8 +1488,10 @@ ata_dev_async(u_int32_t async_code, stru CAM_EXPECT_INQ_CHANGE, NULL); } xpt_release_path(&newpath); - } else if (async_code == AC_LOST_DEVICE) { + } else if (async_code == AC_LOST_DEVICE && + (device->flags & CAM_DEV_UNCONFIGURED) == 0) { device->flags |= CAM_DEV_UNCONFIGURED; + xpt_release_device(device); } else if (async_code == AC_TRANSFER_NEG) { struct ccb_trans_settings *settings; Modified: stable/8/sys/cam/cam_xpt.c ============================================================================== --- stable/8/sys/cam/cam_xpt.c Tue Nov 17 20:49:09 2009 (r199434) +++ stable/8/sys/cam/cam_xpt.c Tue Nov 17 20:49:26 2009 (r199435) @@ -217,9 +217,7 @@ static void xpt_release_devq_device(str int run_queue); static struct cam_et* xpt_alloc_target(struct cam_eb *bus, target_id_t target_id); -static void xpt_release_target(struct cam_eb *bus, struct cam_et *target); -static void xpt_release_device(struct cam_eb *bus, struct cam_et *target, - struct cam_ed *device); +static void xpt_release_target(struct cam_et *target); static struct cam_eb* xpt_find_bus(path_id_t path_id); static struct cam_et* @@ -3521,9 +3519,9 @@ xpt_compile_path(struct cam_path *new_pa CAM_DEBUG(new_path, CAM_DEBUG_TRACE, ("xpt_compile_path\n")); } else { if (device != NULL) - xpt_release_device(bus, target, device); + xpt_release_device(device); if (target != NULL) - xpt_release_target(bus, target); + xpt_release_target(target); if (bus != NULL) xpt_release_bus(bus); } @@ -3535,11 +3533,11 @@ xpt_release_path(struct cam_path *path) { CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_release_path\n")); if (path->device != NULL) { - xpt_release_device(path->bus, path->target, path->device); + xpt_release_device(path->device); path->device = NULL; } if (path->target != NULL) { - xpt_release_target(path->bus, path->target); + xpt_release_target(path->target); path->target = NULL; } if (path->bus != NULL) { @@ -4024,13 +4022,19 @@ xpt_async(u_int32_t async_code, struct c && path->device->lun_id != CAM_LUN_WILDCARD && device->lun_id != CAM_LUN_WILDCARD) continue; - + /* + * The async callback could free the device. + * If it is a broadcast async, it doesn't hold + * device reference, so take our own reference. + */ + xpt_acquire_device(device); (*(bus->xport->async))(async_code, bus, target, device, async_arg); xpt_async_bcast(&device->asyncs, async_code, path, async_arg); + xpt_release_device(device); } } @@ -4375,15 +4379,15 @@ xpt_alloc_target(struct cam_eb *bus, tar } static void -xpt_release_target(struct cam_eb *bus, struct cam_et *target) +xpt_release_target(struct cam_et *target) { if ((--target->refcount == 0) && (TAILQ_FIRST(&target->ed_entries) == NULL)) { - TAILQ_REMOVE(&bus->et_entries, target, links); - bus->generation++; + TAILQ_REMOVE(&target->bus->et_entries, target, links); + target->bus->generation++; + xpt_release_bus(target->bus); free(target, M_CAMXPT); - xpt_release_bus(bus); } } @@ -4470,13 +4474,18 @@ xpt_alloc_device(struct cam_eb *bus, str return (device); } -static void -xpt_release_device(struct cam_eb *bus, struct cam_et *target, - struct cam_ed *device) +void +xpt_acquire_device(struct cam_ed *device) +{ + + device->refcount++; +} + +void +xpt_release_device(struct cam_ed *device) { - if ((--device->refcount == 0) - && ((device->flags & CAM_DEV_UNCONFIGURED) != 0)) { + if (--device->refcount == 0) { struct cam_devq *devq; if (device->alloc_ccb_entry.pinfo.index != CAM_UNQUEUED_INDEX @@ -4486,16 +4495,16 @@ xpt_release_device(struct cam_eb *bus, s if ((device->flags & CAM_DEV_REL_TIMEOUT_PENDING) != 0) callout_stop(&device->callout); - TAILQ_REMOVE(&target->ed_entries, device,links); - target->generation++; - bus->sim->max_ccbs -= device->ccbq.devq_openings; + 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 = bus->sim->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); + xpt_release_target(device->target); free(device, M_CAMXPT); - xpt_release_target(bus, target); } } Modified: stable/8/sys/cam/cam_xpt_internal.h ============================================================================== --- stable/8/sys/cam/cam_xpt_internal.h Tue Nov 17 20:49:09 2009 (r199434) +++ stable/8/sys/cam/cam_xpt_internal.h Tue Nov 17 20:49:26 2009 (r199435) @@ -37,9 +37,7 @@ struct cam_ed; typedef struct cam_ed * (*xpt_alloc_device_func)(struct cam_eb *bus, struct cam_et *target, lun_id_t lun_id); -typedef void (*xpt_release_device_func)(struct cam_eb *bus, - struct cam_et *target, - struct cam_ed *device); +typedef void (*xpt_release_device_func)(struct cam_ed *device); typedef void (*xpt_action_func)(union ccb *start_ccb); typedef void (*xpt_dev_async_func)(u_int32_t async_code, struct cam_eb *bus, @@ -172,6 +170,8 @@ struct xpt_xport * ata_get_xport(void); struct cam_ed * xpt_alloc_device(struct cam_eb *bus, struct cam_et *target, lun_id_t lun_id); +void xpt_acquire_device(struct cam_ed *device); +void xpt_release_device(struct cam_ed *device); void xpt_run_dev_sendq(struct cam_eb *bus); int xpt_schedule_dev(struct camq *queue, cam_pinfo *dev_pinfo, u_int32_t new_priority); Modified: stable/8/sys/cam/scsi/scsi_xpt.c ============================================================================== --- stable/8/sys/cam/scsi/scsi_xpt.c Tue Nov 17 20:49:09 2009 (r199434) +++ stable/8/sys/cam/scsi/scsi_xpt.c Tue Nov 17 20:49:26 2009 (r199435) @@ -1076,8 +1076,10 @@ probedone(struct cam_periph *periph, uni else PROBE_SET_ACTION(softc, PROBE_SERIAL_NUM_0); - path->device->flags &= ~CAM_DEV_UNCONFIGURED; - + if (path->device->flags & CAM_DEV_UNCONFIGURED) { + path->device->flags &= ~CAM_DEV_UNCONFIGURED; + xpt_acquire_device(path->device); + } xpt_release_ccb(done_ccb); xpt_schedule(periph, priority); return; @@ -1336,8 +1338,12 @@ probedone(struct cam_periph *periph, uni CAM_DEBUG(periph->path, CAM_DEBUG_INFO, ("Leave Domain Validation\n")); } + if (path->device->flags & CAM_DEV_UNCONFIGURED) { + path->device->flags &= ~CAM_DEV_UNCONFIGURED; + xpt_acquire_device(path->device); + } path->device->flags &= - ~(CAM_DEV_UNCONFIGURED|CAM_DEV_IN_DV|CAM_DEV_DV_HIT_BOTTOM); + ~(CAM_DEV_IN_DV|CAM_DEV_DV_HIT_BOTTOM); if ((softc->flags & PROBE_NO_ANNOUNCE) == 0) { /* Inform the XPT that a new device has been found */ done_ccb->ccb_h.func_code = XPT_GDEV_TYPE; @@ -1387,8 +1393,12 @@ probedone(struct cam_periph *periph, uni CAM_DEBUG(periph->path, CAM_DEBUG_INFO, ("Leave Domain Validation Successfully\n")); } + if (path->device->flags & CAM_DEV_UNCONFIGURED) { + path->device->flags &= ~CAM_DEV_UNCONFIGURED; + xpt_acquire_device(path->device); + } path->device->flags &= - ~(CAM_DEV_UNCONFIGURED|CAM_DEV_IN_DV|CAM_DEV_DV_HIT_BOTTOM); + ~(CAM_DEV_IN_DV|CAM_DEV_DV_HIT_BOTTOM); if ((softc->flags & PROBE_NO_ANNOUNCE) == 0) { /* Inform the XPT that a new device has been found */ done_ccb->ccb_h.func_code = XPT_GDEV_TYPE; @@ -2375,8 +2385,10 @@ scsi_dev_async(u_int32_t async_code, str CAM_EXPECT_INQ_CHANGE, NULL); } xpt_release_path(&newpath); - } else if (async_code == AC_LOST_DEVICE) { + } else if (async_code == AC_LOST_DEVICE && + (device->flags & CAM_DEV_UNCONFIGURED) == 0) { device->flags |= CAM_DEV_UNCONFIGURED; + xpt_release_device(device); } else if (async_code == AC_TRANSFER_NEG) { struct ccb_trans_settings *settings;