svn commit: r247113 - stable/9/sys/cam/scsi

[Alexander Motin]

Author: mav
Date: Thu Feb 21 18:49:05 2013
New Revision: 247113
URL: http://svnweb.freebsd.org/changeset/base/247113

Log:
  MFC r236138 (by ken) for recently merged scsi_enc.c:
  
  Work around a race condition in devfs by changing the way closes
  are handled in most CAM peripheral drivers that are not handled by
  GEOM's disk class.
  
  The usual character driver open and close semantics are that the
  driver gets N open calls, but only one close, when the last caller
  closes the device.
  
  CAM peripheral drivers expect that behavior to be honored to the
  letter, and the CAM peripheral driver code (specifically
  cam_periph_release_locked_busses()) panics if it is done incorrectly.
  
  Since devfs has to drop its locks while it calls a driver's close
  routine, and it does not have a way to delay or prevent open calls
  while it is calling the close routine, there is a race.
  
  The sequence of events, simplified a bit, is:
  
  - devfs acquires a lock
  - devfs checks the reference count, and if it is 1, continues to close.
  - devfs releases the lock
  
  - 2nd process open call on the device happens here
  
  - devfs calls the driver's close routine
  
  - devfs acquires a lock
  - devfs decrements the reference count
  - devfs releases the lock
  
  - 2nd process close call on the device happens here
  
  At the second close, we get a panic in
  cam_periph_release_locked_busses(), complaining that peripheral
  has been released when the reference count is already 0.  This is
  because we have gotten two closes in a row, which should not
  happen.
  
  The fix is to add the D_TRACKCLOSE flag to the driver's cdevsw, so
  that we get a close() call for each open().  That does happen
  reliably, so we can make sure that our reference counts are
  correct.
  
  Note that the sa(4) and pt(4) drivers only allow one context
  through the open routine.  So these drivers aren't exposed to the
  same race condition.
  
  scsi_ch.c,
  scsi_enc.c,
  scsi_enc_internal.h,
  scsi_pass.c,
  scsi_sg.c:
  		For these drivers, change the open() routine to
  		increment the reference count for every open, and
  		just decrement the reference count in the close.
  
  		Call cam_periph_release_locked() in some scenarios
  		to avoid additional lock and unlock calls.
  
  scsi_pt.c:	Call cam_periph_release_locked() in some scenarios
  		to avoid additional lock and unlock calls.

Modified:
  stable/9/sys/cam/scsi/scsi_enc.c

Modified: stable/9/sys/cam/scsi/scsi_enc.c
==============================================================================
--- stable/9/sys/cam/scsi/scsi_enc.c	Thu Feb 21 18:41:35 2013	(r247112)
+++ stable/9/sys/cam/scsi/scsi_enc.c	Thu Feb 21 18:49:05 2013	(r247113)
@@ -88,7 +88,7 @@ static struct cdevsw enc_cdevsw = {
 	.d_close =	enc_close,
 	.d_ioctl =	enc_ioctl,
 	.d_name =	"ses",
-	.d_flags =	0,
+	.d_flags =	D_TRACKCLOSE,
 };
 
 static void
@@ -249,12 +249,12 @@ enc_open(struct cdev *dev, int flags, in
 		error = ENXIO;
 		goto out;
 	}
-
 out:
+	if (error != 0)
+		cam_periph_release_locked(periph);
+
 	cam_periph_unlock(periph);
-	if (error) {
-		cam_periph_release(periph);
-	}
+
 	return (error);
 }
 
@@ -262,17 +262,11 @@ static int
 enc_close(struct cdev *dev, int flag, int fmt, struct thread *td)
 {
 	struct cam_periph *periph;
-	struct enc_softc *softc;
 
 	periph = (struct cam_periph *)dev->si_drv1;
 	if (periph == NULL)
 		return (ENXIO);
 
-	cam_periph_lock(periph);
-
-	softc = (struct enc_softc *)periph->softc;
-
-	cam_periph_unlock(periph);
 	cam_periph_release(periph);
 
 	return (0);