Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 1 Nov 2009 11:31:06 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r198748 - in head/sys/cam: . ata scsi
Message-ID:  <200911011131.nA1BV6lO022276@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Sun Nov  1 11:31:06 2009
New Revision: 198748
URL: http://svn.freebsd.org/changeset/base/198748

Log:
  MFp4:
  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

Modified:
  head/sys/cam/ata/ata_xpt.c
  head/sys/cam/cam_xpt.c
  head/sys/cam/cam_xpt_internal.h
  head/sys/cam/scsi/scsi_xpt.c

Modified: head/sys/cam/ata/ata_xpt.c
==============================================================================
--- head/sys/cam/ata/ata_xpt.c	Sun Nov  1 10:57:07 2009	(r198747)
+++ head/sys/cam/ata/ata_xpt.c	Sun Nov  1 11:31:06 2009	(r198748)
@@ -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: head/sys/cam/cam_xpt.c
==============================================================================
--- head/sys/cam/cam_xpt.c	Sun Nov  1 10:57:07 2009	(r198747)
+++ head/sys/cam/cam_xpt.c	Sun Nov  1 11:31:06 2009	(r198748)
@@ -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) {
@@ -4375,15 +4373,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 +4468,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 +4489,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: head/sys/cam/cam_xpt_internal.h
==============================================================================
--- head/sys/cam/cam_xpt_internal.h	Sun Nov  1 10:57:07 2009	(r198747)
+++ head/sys/cam/cam_xpt_internal.h	Sun Nov  1 11:31:06 2009	(r198748)
@@ -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: head/sys/cam/scsi/scsi_xpt.c
==============================================================================
--- head/sys/cam/scsi/scsi_xpt.c	Sun Nov  1 10:57:07 2009	(r198747)
+++ head/sys/cam/scsi/scsi_xpt.c	Sun Nov  1 11:31:06 2009	(r198748)
@@ -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;
 



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200911011131.nA1BV6lO022276>