Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 2 Dec 2009 10:10:37 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org
Subject:   svn commit: r200021 - in stable/8/sys/cam: . scsi
Message-ID:  <200912021010.nB2AAbCb094399@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Wed Dec  2 10:10:37 2009
New Revision: 200021
URL: http://svn.freebsd.org/changeset/base/200021

Log:
  MFC r199279, r199280, r199281:
  - Fix several device freeze counting bugs.
  - Remove code that years ago was closing race between request submission
  to SIM and device/SIM freeze. That race become impossible after moving from
  spl to mutex locking, while this workaround causes some unexpected effects.

Modified:
  stable/8/sys/cam/cam_periph.c
  stable/8/sys/cam/cam_queue.c
  stable/8/sys/cam/cam_queue.h
  stable/8/sys/cam/cam_xpt.c
  stable/8/sys/cam/scsi/scsi_cd.c
  stable/8/sys/cam/scsi/scsi_ch.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/cam_periph.c
==============================================================================
--- stable/8/sys/cam/cam_periph.c	Wed Dec  2 08:52:06 2009	(r200020)
+++ stable/8/sys/cam/cam_periph.c	Wed Dec  2 10:10:37 2009	(r200021)
@@ -981,16 +981,21 @@ camperiphdone(struct cam_periph *periph,
 {
 	union ccb      *saved_ccb;
 	cam_status	status;
-	int		frozen;
+	int		frozen = 0;
 	int		sense;
 	struct scsi_start_stop_unit *scsi_cmd;
 	u_int32_t	relsim_flags, timeout;
-	u_int32_t	qfrozen_cnt;
-	int		xpt_done_ccb;
+	int		xpt_done_ccb = FALSE;
 
-	xpt_done_ccb = FALSE;
 	status = done_ccb->ccb_h.status;
-	frozen = (status & CAM_DEV_QFRZN) != 0;
+	if (status & CAM_DEV_QFRZN) {
+		frozen = 1;
+		/*
+		 * Clear freeze flag now for case of retry,
+		 * freeze will be dropped later.
+		 */
+		done_ccb->ccb_h.status &= ~CAM_DEV_QFRZN;
+	}
 	sense  = (status & CAM_AUTOSNS_VALID) != 0;
 	status &= CAM_STATUS_MASK;
 
@@ -998,17 +1003,6 @@ camperiphdone(struct cam_periph *periph,
 	relsim_flags = 0;
 	saved_ccb = (union ccb *)done_ccb->ccb_h.saved_ccb_ptr;
 
-	/* 
-	 * Unfreeze the queue once if it is already frozen..
-	 */
-	if (frozen != 0) {
-		qfrozen_cnt = cam_release_devq(done_ccb->ccb_h.path,
-					      /*relsim_flags*/0,
-					      /*openings*/0,
-					      /*timeout*/0,
-					      /*getcount_only*/0);
-	}
-
 	switch (status) {
 	case CAM_REQ_CMP:
 	{
@@ -1185,14 +1179,33 @@ camperiphdone(struct cam_periph *periph,
 	 */
 	if (done_ccb->ccb_h.retry_count > 0)
 		done_ccb->ccb_h.retry_count--;
-
-	qfrozen_cnt = cam_release_devq(done_ccb->ccb_h.path,
-				      /*relsim_flags*/relsim_flags,
-				      /*openings*/0,
-				      /*timeout*/timeout,
-				      /*getcount_only*/0);
-	if (xpt_done_ccb == TRUE)
+	/*
+	 * Drop freeze taken due to CAM_DEV_QFREEZE flag set on recovery
+	 * request.
+	 */
+	cam_release_devq(done_ccb->ccb_h.path,
+			 /*relsim_flags*/relsim_flags,
+			 /*openings*/0,
+			 /*timeout*/timeout,
+			 /*getcount_only*/0);
+	if (xpt_done_ccb == TRUE) {
+		/*
+		 * Copy frozen flag from recovery request if it is set there
+		 * for some reason.
+		 */
+		if (frozen != 0)
+			done_ccb->ccb_h.status |= CAM_DEV_QFRZN;
 		(*done_ccb->ccb_h.cbfcnp)(periph, done_ccb);
+	} else {
+		/* Drop freeze taken, if this recovery request got error. */
+		if (frozen != 0) {
+			cam_release_devq(done_ccb->ccb_h.path,
+				 /*relsim_flags*/0,
+				 /*openings*/0,
+				 /*timeout*/0,
+				 /*getcount_only*/0);
+		}
+	}
 }
 
 /*
@@ -1452,6 +1465,11 @@ camperiphscsisenseerror(union ccb *ccb, 
 				action_string = "No recovery CCB supplied";
 				goto sense_error_done;
 			}
+			/*
+			 * Clear freeze flag for original request here, as
+			 * this freeze will be dropped as part of ERESTART.
+			 */
+			ccb->ccb_h.status &= ~CAM_DEV_QFRZN;
 			bcopy(ccb, save_ccb, sizeof(*save_ccb));
 			print_ccb = save_ccb;
 			periph->flags |= CAM_PERIPH_RECOVERY_INPROG;

Modified: stable/8/sys/cam/cam_queue.c
==============================================================================
--- stable/8/sys/cam/cam_queue.c	Wed Dec  2 08:52:06 2009	(r200020)
+++ stable/8/sys/cam/cam_queue.c	Wed Dec  2 10:10:37 2009	(r200021)
@@ -334,7 +334,6 @@ cam_ccbq_init(struct cam_ccbq *ccbq, int
 	}
 	ccbq->devq_openings = openings;
 	ccbq->dev_openings = openings;	
-	TAILQ_INIT(&ccbq->active_ccbs);
 	return (0);
 }
 

Modified: stable/8/sys/cam/cam_queue.h
==============================================================================
--- stable/8/sys/cam/cam_queue.h	Wed Dec  2 08:52:06 2009	(r200020)
+++ stable/8/sys/cam/cam_queue.h	Wed Dec  2 10:10:37 2009	(r200021)
@@ -60,7 +60,6 @@ struct cam_ccbq {
 	int	dev_openings;	
 	int	dev_active;
 	int	held;
-	struct	ccb_hdr_tailq active_ccbs;
 };
 
 struct cam_ed;
@@ -209,9 +208,6 @@ static __inline void
 cam_ccbq_send_ccb(struct cam_ccbq *ccbq, union ccb *send_ccb)
 {
 
-	TAILQ_INSERT_TAIL(&ccbq->active_ccbs,
-			  &(send_ccb->ccb_h),
-			  xpt_links.tqe);
 	send_ccb->ccb_h.pinfo.index = CAM_ACTIVE_INDEX;
 	ccbq->dev_active++;
 	ccbq->dev_openings--;		
@@ -220,8 +216,7 @@ cam_ccbq_send_ccb(struct cam_ccbq *ccbq,
 static __inline void
 cam_ccbq_ccb_done(struct cam_ccbq *ccbq, union ccb *done_ccb)
 {
-	TAILQ_REMOVE(&ccbq->active_ccbs, &done_ccb->ccb_h,
-		     xpt_links.tqe);
+
 	ccbq->dev_active--;
 	ccbq->dev_openings++;	
 	ccbq->held++;

Modified: stable/8/sys/cam/cam_xpt.c
==============================================================================
--- stable/8/sys/cam/cam_xpt.c	Wed Dec  2 08:52:06 2009	(r200020)
+++ stable/8/sys/cam/cam_xpt.c	Wed Dec  2 10:10:37 2009	(r200021)
@@ -3273,16 +3273,13 @@ xpt_run_dev_sendq(struct cam_eb *bus)
 
 	devq->send_queue.qfrozen_cnt++;
 	while ((devq->send_queue.entries > 0)
-	    && (devq->send_openings > 0)) {
+	    && (devq->send_openings > 0)
+	    && (devq->send_queue.qfrozen_cnt <= 1)) {
 		struct	cam_ed_qinfo *qinfo;
 		struct	cam_ed *device;
 		union ccb *work_ccb;
 		struct	cam_sim *sim;
 
-	    	if (devq->send_queue.qfrozen_cnt > 1) {
-			break;
-		}
-
 		qinfo = (struct cam_ed_qinfo *)camq_remove(&devq->send_queue,
 							   CAMQ_HEAD);
 		device = qinfo->device;
@@ -3330,9 +3327,7 @@ xpt_run_dev_sendq(struct cam_eb *bus)
 			}
 			mtx_unlock(&xsoftc.xpt_lock);
 		}
-		devq->active_dev = device;
 		cam_ccbq_remove_ccb(&device->ccbq, work_ccb);
-
 		cam_ccbq_send_ccb(&device->ccbq, work_ccb);
 
 		devq->send_openings--;
@@ -3370,8 +3365,6 @@ xpt_run_dev_sendq(struct cam_eb *bus)
 		 */
 		sim = work_ccb->ccb_h.path->bus->sim;
 		(*(sim->sim_action))(sim, work_ccb);
-
-		devq->active_dev = NULL;
 	}
 	devq->send_queue.qfrozen_cnt--;
 }
@@ -4102,45 +4095,18 @@ xpt_dev_async_default(u_int32_t async_co
 u_int32_t
 xpt_freeze_devq(struct cam_path *path, u_int count)
 {
-	struct ccb_hdr *ccbh;
 
 	mtx_assert(path->bus->sim->mtx, MA_OWNED);
-
 	path->device->ccbq.queue.qfrozen_cnt += count;
-
-	/*
-	 * Mark the last CCB in the queue as needing
-	 * to be requeued if the driver hasn't
-	 * changed it's state yet.  This fixes a race
-	 * where a ccb is just about to be queued to
-	 * a controller driver when it's interrupt routine
-	 * freezes the queue.  To completly close the
-	 * hole, controller drives must check to see
-	 * if a ccb's status is still CAM_REQ_INPROG
-	 * just before they queue
-	 * the CCB.  See ahc_action/ahc_freeze_devq for
-	 * an example.
-	 */
-	ccbh = TAILQ_LAST(&path->device->ccbq.active_ccbs, ccb_hdr_tailq);
-	if (ccbh && ccbh->status == CAM_REQ_INPROG)
-		ccbh->status = CAM_REQUEUE_REQ;
 	return (path->device->ccbq.queue.qfrozen_cnt);
 }
 
 u_int32_t
 xpt_freeze_simq(struct cam_sim *sim, u_int count)
 {
-	mtx_assert(sim->mtx, MA_OWNED);
 
+	mtx_assert(sim->mtx, MA_OWNED);
 	sim->devq->send_queue.qfrozen_cnt += count;
-	if (sim->devq->active_dev != NULL) {
-		struct ccb_hdr *ccbh;
-
-		ccbh = TAILQ_LAST(&sim->devq->active_dev->ccbq.active_ccbs,
-				  ccb_hdr_tailq);
-		if (ccbh && ccbh->status == CAM_REQ_INPROG)
-			ccbh->status = CAM_REQUEUE_REQ;
-	}
 	return (sim->devq->send_queue.qfrozen_cnt);
 }
 

Modified: stable/8/sys/cam/scsi/scsi_cd.c
==============================================================================
--- stable/8/sys/cam/scsi/scsi_cd.c	Wed Dec  2 08:52:06 2009	(r200020)
+++ stable/8/sys/cam/scsi/scsi_cd.c	Wed Dec  2 10:10:37 2009	(r200021)
@@ -1570,7 +1570,8 @@ cddone(struct cam_periph *periph, union 
 			bp->bio_resid = bp->bio_bcount;
 			bp->bio_error = error;
 			bp->bio_flags |= BIO_ERROR;
-			cam_release_devq(done_ccb->ccb_h.path,
+			if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
+				cam_release_devq(done_ccb->ccb_h.path,
 					 /*relsim_flags*/0,
 					 /*reduction*/0,
 					 /*timeout*/0,
@@ -1658,7 +1659,8 @@ cddone(struct cam_periph *periph, union 
 				struct ccb_getdev cgd;
 
 				/* Don't wedge this device's queue */
-				cam_release_devq(done_ccb->ccb_h.path,
+				if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
+					cam_release_devq(done_ccb->ccb_h.path,
 						 /*relsim_flags*/0,
 						 /*reduction*/0,
 						 /*timeout*/0,

Modified: stable/8/sys/cam/scsi/scsi_ch.c
==============================================================================
--- stable/8/sys/cam/scsi/scsi_ch.c	Wed Dec  2 08:52:06 2009	(r200020)
+++ stable/8/sys/cam/scsi/scsi_ch.c	Wed Dec  2 10:10:37 2009	(r200021)
@@ -606,7 +606,8 @@ chdone(struct cam_periph *periph, union 
 					retry_scheduled = 0;
 
 				/* Don't wedge this device's queue */
-				cam_release_devq(done_ccb->ccb_h.path,
+				if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0)
+					cam_release_devq(done_ccb->ccb_h.path,
 						 /*relsim_flags*/0,
 						 /*reduction*/0,
 						 /*timeout*/0,



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