Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 21 Aug 2005 17:13:32 GMT
From:      Scott Long <scottl@FreeBSD.org>
To:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   PERFORCE change 82377 for review
Message-ID:  <200508211713.j7LHDWnZ056624@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=82377

Change 82377 by scottl@scottl-junior on 2005/08/21 17:13:18

	Introduce the topology lock.  It covers the reference counting of
	path components so that peripherals and sims can use path objects
	without having to lock them.

Affected files ...

.. //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#16 edit

Differences ...

==== //depot/projects/scottl-camlock/src/sys/cam/cam_xpt.c#16 (text+ko) ====

@@ -619,6 +619,7 @@
 typedef TAILQ_HEAD(cam_isrq, ccb_hdr) cam_isrq_t;
 static cam_isrq_t cam_bioq;
 static struct mtx cam_bioq_lock;
+static struct mtx cam_topo_lock;
 
 /* "Pool" of inactive ccbs managed by xpt_alloc_ccb and xpt_free_ccb */
 static SLIST_HEAD(,ccb_hdr) ccb_freeq;
@@ -1390,6 +1391,7 @@
 	STAILQ_INIT(&highpowerq);
 
 	mtx_init(&cam_bioq_lock, "CAM BIOQ lock", NULL, MTX_DEF);
+	mtx_init(&cam_topo_lock, "XPT Topology lock", NULL, MTX_DEF);
 
 	/*
 	 * The xpt layer is, itself, the equivelent of a SIM.
@@ -3301,7 +3303,6 @@
 	}
 	case XPT_DEV_MATCH:
 	{
-		int s;
 		dev_pos_type position_type;
 		struct ccb_dev_match *cdm;
 
@@ -3310,7 +3311,7 @@
 		/*
 		 * Prevent EDT changes while we traverse it.
 		 */
-		s = splcam();
+		mtx_lock(&cam_topo_lock);
 		/*
 		 * There are two ways of getting at information in the EDT.
 		 * The first way is via the primary EDT tree.  It starts
@@ -3358,7 +3359,7 @@
 			break;
 		}
 
-		splx(s);
+		mtx_unlock(&cam_topo_lock);
 
 		if (cdm->status == CAM_DEV_MATCH_ERROR)
 			start_ccb->ccb_h.status = CAM_REQ_CMP_ERR;
@@ -3421,6 +3422,7 @@
 			csa->ccb_h.path->device->refcount++;
 		}
 
+		mtx_lock(&cam_topo_lock);
 		if ((added & AC_FOUND_DEVICE) != 0) {
 			/*
 			 * Get this peripheral up to date with all
@@ -3435,6 +3437,7 @@
 			 */
 			xpt_for_all_busses(xptsetasyncbusfunc, cur_entry);
 		}
+		mtx_unlock(&cam_topo_lock);
 		splx(s);
 		start_ccb->ccb_h.status = CAM_REQ_CMP;
 		break;
@@ -3969,7 +3972,6 @@
 void
 xpt_merge_ccb(union ccb *master_ccb, union ccb *slave_ccb)
 {
-	GIANT_REQUIRED;
 
 	/*
 	 * Pull fields that are valid for peripheral drivers to set
@@ -3986,7 +3988,6 @@
 void
 xpt_setup_ccb(struct ccb_hdr *ccb_h, struct cam_path *path, u_int32_t priority)
 {
-	GIANT_REQUIRED;
 
 	CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_setup_ccb\n"));
 	ccb_h->pinfo.priority = priority;
@@ -3998,7 +3999,7 @@
 		ccb_h->target_id = CAM_TARGET_WILDCARD;
 	if (path->device) {
 		ccb_h->target_lun = path->device->lun_id;
-		ccb_h->pinfo.generation = ++path->device->ccbq.queue.generation;
+		atomic_add_int(&ccb_h->pinfo.generation, 1);
 	} else {
 		ccb_h->target_lun = CAM_TARGET_WILDCARD;
 	}
@@ -4014,8 +4015,6 @@
 	struct	   cam_path *path;
 	cam_status status;
 
-	GIANT_REQUIRED;
-
 	path = (struct cam_path *)malloc(sizeof(*path), M_CAMXPT, M_NOWAIT);
 
 	if (path == NULL) {
@@ -4032,6 +4031,33 @@
 }
 
 static cam_status
+xpt_clone_path(struct cam_path *old_path_ptr, struct cam_path **new_path_ptr)
+{
+	struct cam_path *path;
+
+	path = (struct cam_path *)malloc(sizeof(*path), M_CAMXPT, M_NOWAIT);
+
+	if (path == NULL)
+		return (CAM_RESRC_UNAVAIL);
+
+	mtx_lock(&cam_topo_lock);
+	path->periph = old_path_ptr->periph;
+	path->bus = old_path_ptr->bus;
+	path->target = old_path_ptr->target;
+	path->device = old_path_ptr->device;
+	if (path->bus != NULL)
+		path->bus->refcount++;
+	if (path->target != NULL)
+		path->target->refcount++;
+	if (path->device != NULL)
+		path->device->refcount++;
+	mtx_unlock(&cam_topo_lock);
+
+	*new_path_ptr = path;
+	return (CAM_REQ_CMP);
+}
+
+static cam_status
 xpt_compile_path(struct cam_path *new_path, struct cam_periph *perph,
 		 path_id_t path_id, target_id_t target_id, lun_id_t lun_id)
 {
@@ -4039,7 +4065,6 @@
 	struct	     cam_et *target;
 	struct	     cam_ed *device;
 	cam_status   status;
-	int	     s;
 
 	status = CAM_REQ_CMP;	/* Completed without error */
 	target = NULL;		/* Wildcarded */
@@ -4049,7 +4074,7 @@
 	 * We will potentially modify the EDT, so block interrupts
 	 * that may attempt to create cam paths.
 	 */
-	s = splcam();
+	mtx_lock(&cam_topo_lock);
 	bus = xpt_find_bus(path_id);
 	if (bus == NULL) {
 		status = CAM_PATH_INVALID;
@@ -4083,7 +4108,7 @@
 			}
 		}
 	}
-	splx(s);
+	mtx_unlock(&cam_topo_lock);
 
 	/*
 	 * Only touch the user's data if we are successful.
@@ -4095,12 +4120,14 @@
 		new_path->device = device;
 		CAM_DEBUG(new_path, CAM_DEBUG_TRACE, ("xpt_compile_path\n"));
 	} else {
+		mtx_lock(&cam_topo_lock);
 		if (device != NULL)
 			xpt_release_device(bus, target, device);
 		if (target != NULL)
 			xpt_release_target(bus, target);
 		if (bus != NULL)
 			xpt_release_bus(bus);
+		mtx_unlock(&cam_topo_lock);
 	}
 	return (status);
 }
@@ -4109,6 +4136,8 @@
 xpt_release_path(struct cam_path *path)
 {
 	CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_release_path\n"));
+
+	mtx_lock(&cam_topo_lock);
 	if (path->device != NULL) {
 		xpt_release_device(path->bus, path->target, path->device);
 		path->device = NULL;
@@ -4121,12 +4150,12 @@
 		xpt_release_bus(path->bus);
 		path->bus = NULL;
 	}
+	mtx_unlock(&cam_topo_lock);
 }
 
 void
 xpt_free_path(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_free_path\n"));
 	xpt_release_path(path);
@@ -4141,7 +4170,6 @@
 int
 xpt_path_comp(struct cam_path *path1, struct cam_path *path2)
 {
-	GIANT_REQUIRED;
 
 	int retval = 0;
 
@@ -4177,7 +4205,6 @@
 void
 xpt_print_path(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	if (path == NULL)
 		printf("(nopath): ");
@@ -4212,8 +4239,6 @@
 {
 	struct sbuf sb;
 
-	GIANT_REQUIRED;
-
 	sbuf_new(&sb, str, str_len, 0);
 
 	if (path == NULL)
@@ -4250,7 +4275,6 @@
 path_id_t
 xpt_path_path_id(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	return(path->bus->path_id);
 }
@@ -4258,7 +4282,6 @@
 target_id_t
 xpt_path_target_id(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	if (path->target != NULL)
 		return (path->target->target_id);
@@ -4269,7 +4292,6 @@
 lun_id_t
 xpt_path_lun_id(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	if (path->device != NULL)
 		return (path->device->lun_id);
@@ -4280,7 +4302,6 @@
 struct cam_sim *
 xpt_path_sim(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	return (path->bus->sim);
 }
@@ -4288,7 +4309,6 @@
 struct cam_periph*
 xpt_path_periph(struct cam_path *path)
 {
-	GIANT_REQUIRED;
 
 	return (path->periph);
 }
@@ -4419,7 +4439,9 @@
 	xpt_async(AC_PATH_DEREGISTERED, &bus_path, NULL);
 	
 	/* Release the reference count held while registered. */
+	mtx_lock(&cam_topo_lock);
 	xpt_release_bus(bus_path.bus);
+	mtx_unlock(&cam_topo_lock);
 	xpt_release_path(&bus_path);
 
 	return (CAM_REQ_CMP);
@@ -4499,8 +4521,9 @@
 }
 
 void
-xpt_async(u_int32_t async_code, struct cam_path *path, void *async_arg)
+xpt_async(u_int32_t async_code, struct cam_path *opath, void *async_arg)
 {
+	struct cam_path *path;
 	struct cam_eb *bus;
 	struct cam_et *target, *next_target;
 	struct cam_ed *device, *next_device;
@@ -4511,6 +4534,13 @@
 	CAM_DEBUG(path, CAM_DEBUG_TRACE, ("xpt_async\n"));
 
 	/*
+	 * The caller owns the path object that was passed in.  Clone
+	 * it so that we don't risk having it disappear on us.
+	 */
+	if (xpt_clone_path(opath, &path) != CAM_REQ_CMP)
+		return;
+
+	/*
 	 * Most async events come from a CAM interrupt context.  In
 	 * a few cases, the error recovery code at the peripheral layer,
 	 * which may run from our SWI or a process context, may signal
@@ -4577,6 +4607,7 @@
 		xpt_async_bcast(&xpt_periph->path->device->asyncs, async_code,
 				path, async_arg);
 	splx(s);
+	xpt_free_path(path);
 }
 
 static void
@@ -4808,6 +4839,7 @@
 					  sim->c_handle);
 				sim->flags &= ~CAM_SIM_REL_TIMEOUT_PENDING;
 			}
+			mtx_lock(&cam_topo_lock);
 			bus = xpt_find_bus(sim->path_id);
 			splx(s);
 
@@ -4815,9 +4847,12 @@
 				/*
 				 * Now that we are unfrozen run the send queue.
 				 */
+				mtx_unlock(&cam_topo_lock);
 				xpt_run_dev_sendq(bus);
+				mtx_lock(&cam_topo_lock);
 			}
 			xpt_release_bus(bus);
+			mtx_unlock(&cam_topo_lock);
 		} else
 			splx(s);
 	} else
@@ -4868,8 +4903,6 @@
 {
 	union ccb *new_ccb;
 
-	GIANT_REQUIRED;
-
 	new_ccb = malloc(sizeof(*new_ccb), M_CAMXPT, M_WAITOK);
 	return (new_ccb);
 }
@@ -4879,8 +4912,6 @@
 {
 	union ccb *new_ccb;
 
-	GIANT_REQUIRED;
-
 	new_ccb = malloc(sizeof(*new_ccb), M_CAMXPT, M_NOWAIT);
 	return (new_ccb);
 }
@@ -4929,17 +4960,14 @@
 static void
 xpt_release_bus(struct cam_eb *bus)
 {
-	int s;
 
-	s = splcam();
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	if ((--bus->refcount == 0)
 	 && (TAILQ_FIRST(&bus->et_entries) == NULL)) {
 		TAILQ_REMOVE(&xpt_busses, bus, links);
 		bus_generation++;
-		splx(s);
 		free(bus, M_CAMXPT);
-	} else
-		splx(s);
+	}
 }
 
 static struct cam_et *
@@ -4947,6 +4975,7 @@
 {
 	struct cam_et *target;
 
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	target = (struct cam_et *)malloc(sizeof(*target), M_CAMXPT, M_NOWAIT);
 	if (target != NULL) {
 		struct cam_et *cur_target;
@@ -4981,18 +5010,15 @@
 static void
 xpt_release_target(struct cam_eb *bus, struct cam_et *target)
 {
-	int s;
 
-	s = splcam();
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	if ((--target->refcount == 0)
 	 && (TAILQ_FIRST(&target->ed_entries) == NULL)) {
 		TAILQ_REMOVE(&bus->et_entries, target, links);
 		bus->generation++;
-		splx(s);
 		free(target, M_CAMXPT);
 		xpt_release_bus(bus);
-	} else
-		splx(s);
+	}
 }
 
 static struct cam_ed *
@@ -5005,6 +5031,7 @@
 	struct	   cam_devq *devq;
 	cam_status status;
 
+	mtx_assert(&cam_topo_lock, 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);
@@ -5097,9 +5124,8 @@
 xpt_release_device(struct cam_eb *bus, struct cam_et *target,
 		   struct cam_ed *device)
 {
-	int s;
 
-	s = splcam();
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	if ((--device->refcount == 0)
 	 && ((device->flags & CAM_DEV_UNCONFIGURED) != 0)) {
 		struct cam_devq *devq;
@@ -5118,13 +5144,11 @@
 		/* Release our slot in the devq */
 		devq = bus->sim->devq;
 		cam_devq_resize(devq, devq->alloc_queue.array_size - 1);
-		splx(s);
 		camq_fini(&device->drvq);
 		camq_fini(&device->ccbq.queue);
 		free(device, M_CAMXPT);
 		xpt_release_target(bus, target);
-	} else
-		splx(s);
+	}
 }
 
 static u_int32_t
@@ -5157,6 +5181,7 @@
 {
 	struct cam_eb *bus;
 
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	for (bus = TAILQ_FIRST(&xpt_busses);
 	     bus != NULL;
 	     bus = TAILQ_NEXT(bus, links)) {
@@ -5173,6 +5198,7 @@
 {
 	struct cam_et *target;
 
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	for (target = TAILQ_FIRST(&bus->et_entries);
 	     target != NULL;
 	     target = TAILQ_NEXT(target, links)) {
@@ -5189,6 +5215,7 @@
 {
 	struct cam_ed *device;
 
+	mtx_assert(&cam_topo_lock, MA_OWNED);
 	for (device = TAILQ_FIRST(&target->ed_entries);
 	     device != NULL;
 	     device = TAILQ_NEXT(device, links)) {
@@ -6861,10 +6888,12 @@
 	/*
 	 * Scan all installed busses.
 	 */
+	mtx_lock(&cam_topo_lock);
 	xpt_for_all_busses(xptconfigbuscountfunc, NULL);
 
 	if (busses_to_config == 0) {
 		/* Call manually because we don't have any busses */
+		mtx_unlock(&cam_topo_lock);
 		xpt_finishconfig(xpt_periph, NULL);
 	} else  {
 		if (busses_to_reset > 0 && scsi_delay >= 2000) {
@@ -6872,6 +6901,7 @@
 			       "devices to settle\n", scsi_delay/1000);
 		}
 		xpt_for_all_busses(xptconfigfunc, NULL);
+		mtx_unlock(&cam_topo_lock);
 	}
 }
 
@@ -6937,7 +6967,9 @@
 		 * attached.  For any devices like that, announce the
 		 * passthrough driver so the user will see something.
 		 */
+		mtx_lock(&cam_topo_lock);
 		xpt_for_all_devices(xptpassannouncefunc, NULL);
+		mtx_unlock(&cam_topo_lock);
 
 		/* Release our hook so that the boot can continue. */
 		config_intrhook_disestablish(xpt_config_hook);



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