From owner-p4-projects@FreeBSD.ORG Sat Oct 31 16:06:11 2009 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 8F08F106568B; Sat, 31 Oct 2009 16:06:11 +0000 (UTC) Delivered-To: perforce@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 536281065670 for ; Sat, 31 Oct 2009 16:06:11 +0000 (UTC) (envelope-from mav@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 401A58FC21 for ; Sat, 31 Oct 2009 16:06:11 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id n9VG6BRp084705 for ; Sat, 31 Oct 2009 16:06:11 GMT (envelope-from mav@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id n9VG6BKv084703 for perforce@freebsd.org; Sat, 31 Oct 2009 16:06:11 GMT (envelope-from mav@freebsd.org) Date: Sat, 31 Oct 2009 16:06:11 GMT Message-Id: <200910311606.n9VG6BKv084703@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to mav@freebsd.org using -f From: Alexander Motin To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 170004 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 31 Oct 2009 16:06:11 -0000 http://p4web.freebsd.org/chv.cgi?CH=170004 Change 170004 by mav@mav_mavtest on 2009/10/31 16:05:19 Fix reference counting bug, when device unreferenced before then invalidated. To do it, do not handle validity flag as another reference, but explicitly modify reference count each time flag is modified. Discovered by: thompsa Affected files ... .. //depot/projects/scottl-camlock/src/sys/cam/ata/ata_xpt.c#47 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#114 edit .. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt_internal.h#10 edit .. //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_xpt.c#21 edit Differences ... ==== //depot/projects/scottl-camlock/src/sys/cam/ata/ata_xpt.c#47 (text+ko) ==== @@ -735,6 +735,7 @@ 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, @@ -779,6 +780,7 @@ 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); @@ -812,6 +814,7 @@ 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, @@ -1517,8 +1520,10 @@ 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; ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#114 (text+ko) ==== @@ -217,9 +217,7 @@ 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* @@ -3528,9 +3526,9 @@ 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); } @@ -3542,11 +3540,11 @@ { 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) { @@ -4383,15 +4381,15 @@ } 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); } } @@ -4478,13 +4476,18 @@ 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 @@ -4494,16 +4497,16 @@ 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); } } ==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt_internal.h#10 (text+ko) ==== @@ -37,9 +37,7 @@ 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_done_func)(union ccb *done_ccb); typedef void (*xpt_dev_async_func)(u_int32_t async_code, @@ -174,6 +172,8 @@ 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); ==== //depot/projects/scottl-camlock/src/sys/cam/scsi/scsi_xpt.c#21 (text+ko) ==== @@ -1077,8 +1077,10 @@ 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; @@ -1337,8 +1339,12 @@ 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; @@ -1388,8 +1394,12 @@ 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; @@ -2376,8 +2386,10 @@ 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;