Skip site navigation (1)Skip section navigation (2) 
Date:      Fri, 30 Sep 2016 21:00:10 +0000 (UTC)
From:      Mark Johnston <markj@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r306529 - head/sys/cam
Message-ID:  <201609302100.u8UL0A3w034815@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: markj
Date: Fri Sep 30 21:00:09 2016
New Revision: 306529
URL: https://svnweb.freebsd.org/changeset/base/306529

Log:
  cam_periph_ccbwait could return while ccb in progress
  
  In cam_periph_runccb, cam_periph_ccbwait was using the value of the ccb
  pinfo.index and status fields to determine whether the ccb was done,
  but these fields are updated without a contending lock and could glitch
  into states that would be erroneously interpreted as done.  Instead,
  have cam_periph_ccbwait look for the explicit result of the function
  cam_periph_done.
  
  Submitted by:	Ryan Libby <rlibby@gmail.com>
  Reviewed by:	mav
  MFC after:	3 weeks
  Sponsored by:	Dell EMC Isilon
  Differential Revision:	https://reviews.freebsd.org/D8020

Modified:
  head/sys/cam/cam_periph.c
  head/sys/cam/cam_periph.h

Modified: head/sys/cam/cam_periph.c
==============================================================================
--- head/sys/cam/cam_periph.c	Fri Sep 30 20:35:12 2016	(r306528)
+++ head/sys/cam/cam_periph.c	Fri Sep 30 21:00:09 2016	(r306529)
@@ -975,16 +975,6 @@ cam_periph_unmapmem(union ccb *ccb, stru
 	PRELE(curproc);
 }
 
-void
-cam_periph_ccbwait(union ccb *ccb)
-{
-
-	if ((ccb->ccb_h.pinfo.index != CAM_UNQUEUED_INDEX)
-	 || ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_INPROG))
-		xpt_path_sleep(ccb->ccb_h.path, &ccb->ccb_h.cbfcnp, PRIBIO,
-		    "cbwait", 0);
-}
-
 int
 cam_periph_ioctl(struct cam_periph *periph, u_long cmd, caddr_t addr,
 		 int (*error_routine)(union ccb *ccb, 
@@ -1048,13 +1038,38 @@ cam_periph_ioctl(struct cam_periph *peri
 }
 
 static void
+cam_periph_done_panic(struct cam_periph *periph, union ccb *done_ccb)
+{
+
+	panic("%s: already done with ccb %p", __func__, done_ccb);
+}
+
+static void
 cam_periph_done(struct cam_periph *periph, union ccb *done_ccb)
 {
 
 	/* Caller will release the CCB */
+	xpt_path_assert(done_ccb->ccb_h.path, MA_OWNED);
+	done_ccb->ccb_h.cbfcnp = cam_periph_done_panic;
 	wakeup(&done_ccb->ccb_h.cbfcnp);
 }
 
+static void
+cam_periph_ccbwait(union ccb *ccb)
+{
+
+	if ((ccb->ccb_h.func_code & XPT_FC_QUEUED) != 0) {
+		while (ccb->ccb_h.cbfcnp != cam_periph_done_panic)
+			xpt_path_sleep(ccb->ccb_h.path, &ccb->ccb_h.cbfcnp,
+			    PRIBIO, "cbwait", 0);
+	}
+	KASSERT(ccb->ccb_h.pinfo.index == CAM_UNQUEUED_INDEX &&
+	    (ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_INPROG,
+	    ("%s: proceeding with incomplete ccb: ccb=%p, func_code=%#x, "
+	     "status=%#x, index=%d", __func__, ccb, ccb->ccb_h.func_code,
+	     ccb->ccb_h.status, ccb->ccb_h.pinfo.index));
+}
+
 int
 cam_periph_runccb(union ccb *ccb,
 		  int (*error_routine)(union ccb *ccb,
@@ -1069,6 +1084,9 @@ cam_periph_runccb(union ccb *ccb,
  
 	starttime = NULL;
 	xpt_path_assert(ccb->ccb_h.path, MA_OWNED);
+	KASSERT((ccb->ccb_h.flags & CAM_UNLOCKED) == 0,
+	    ("%s: ccb=%p, func_code=%#x, flags=%#x", __func__, ccb,
+	     ccb->ccb_h.func_code, ccb->ccb_h.flags));
 
 	/*
 	 * If the user has supplied a stats structure, and if we understand
@@ -1088,9 +1106,10 @@ cam_periph_runccb(union ccb *ccb,
 		cam_periph_ccbwait(ccb);
 		if ((ccb->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP)
 			error = 0;
-		else if (error_routine != NULL)
+		else if (error_routine != NULL) {
+			ccb->ccb_h.cbfcnp = cam_periph_done;
 			error = (*error_routine)(ccb, camflags, sense_flags);
-		else
+		} else
 			error = 0;
 
 	} while (error == ERESTART);

Modified: head/sys/cam/cam_periph.h
==============================================================================
--- head/sys/cam/cam_periph.h	Fri Sep 30 20:35:12 2016	(r306528)
+++ head/sys/cam/cam_periph.h	Fri Sep 30 21:00:09 2016	(r306529)
@@ -166,7 +166,6 @@ void		cam_periph_unmapmem(union ccb *ccb
 				    struct cam_periph_map_info *mapinfo);
 union ccb	*cam_periph_getccb(struct cam_periph *periph,
 				   u_int32_t priority);
-void		cam_periph_ccbwait(union ccb *ccb);
 int		cam_periph_runccb(union ccb *ccb,
 				  int (*error_routine)(union ccb *ccb,
 						       cam_flags camflags,

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