Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 24 Aug 2013 04:52:22 +0000 (UTC)
From:      "Kenneth D. Merry" <ken@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r254760 - in head: share/man/man4 sys/cam/scsi sys/kern sys/sys
Message-ID:  <201308240452.r7O4qMji033904@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: ken
Date: Sat Aug 24 04:52:22 2013
New Revision: 254760
URL: http://svnweb.freebsd.org/changeset/base/254760

Log:
  Add support to physio(9) for devices that don't want I/O split and
  configure sa(4) to request no I/O splitting by default.
  
  For tape devices, the user needs to be able to clearly understand
  what blocksize is actually being used when writing to a tape
  device.  The previous behavior of physio(9) was that it would split
  up any I/O that was too large for the device, or too large to fit
  into MAXPHYS.  This means that if, for instance, the user wrote a
  1MB block to a tape device, and MAXPHYS was 128KB, the 1MB write
  would be split into 8 128K chunks.  This would be done without
  informing the user.
  
  This has suboptimal effects, especially when trying to communicate
  status to the user.  In the event of an error writing to a tape
  (e.g. physical end of tape) in the middle of a 1MB block that has
  been split into 8 pieces, the user could have the first two 128K
  pieces written successfully, the third returned with an error, and
  the last 5 returned with 0 bytes written.  If the user is using
  a standard write(2) system call, all he will see is the ENOSPC
  error.  He won't have a clue how much actually got written.  (With
  a writev(2) system call, he should be able to determine how much
  got written in addition to the error.)
  
  The solution is to prevent physio(9) from splitting the I/O.  The
  new cdev flag, SI_NOSPLIT, tells physio that the driver does not
  want I/O to be split beforehand.
  
  Although the sa(4) driver now enables SI_NOSPLIT by default,
  that can be disabled by two loader tunables for now.  It will not
  be configurable starting in FreeBSD 11.0.  kern.cam.sa.allow_io_split
  allows the user to configure I/O splitting for all sa(4) driver
  instances.  kern.cam.sa.%d.allow_io_split allows the user to
  configure I/O splitting for a specific sa(4) instance.
  
  There are also now three sa(4) driver sysctl variables that let the
  users see some sa(4) driver values.  kern.cam.sa.%d.allow_io_split
  shows whether I/O splitting is turned on.  kern.cam.sa.%d.maxio shows
  the maximum I/O size allowed by kernel configuration parameters
  (e.g. MAXPHYS, DFLTPHYS) and the capabilities of the controller.
  kern.cam.sa.%d.cpi_maxio shows the maximum I/O size supported by
  the controller.
  
  Note that a better long term solution would be to implement support
  for chaining buffers, so that that MAXPHYS is no longer a limiting
  factor for I/O size to tape and disk devices.  At that point, the
  controller and the tape drive would become the limiting factors.
  
  sys/conf.h:	Add a new cdev flag, SI_NOSPLIT, that allows a
  		driver to tell physio not to split up I/O.
  
  sys/param.h:	Bump __FreeBSD_version to 1000049 for the addition
  		of the SI_NOSPLIT cdev flag.
  
  kern_physio.c:	If the SI_NOSPLIT flag is set on the cdev, return
  		any I/O that is larger than si_iosize_max or
  		MAXPHYS, has more than one segment, or would have
  		to be split because of misalignment with EFBIG.
  		(File too large).
  
  		In the event of an error, print a console message to
  		give the user a clue about what happened.
  
  scsi_sa.c:	Set the SI_NOSPLIT cdev flag on the devices created
  		for the sa(4) driver by default.
  
  		Add tunables to control whether we allow I/O splitting
  		in physio(9).
  
  		Explain in the comments that allowing I/O splitting
  		will be deprecated for the sa(4) driver in FreeBSD
  		11.0.
  
  		Add sysctl variables to display the maximum I/O
  		size we can do (which could be further limited by
  		read block limits) and the maximum I/O size that
  		the controller can do.
  
  		Limit our maximum I/O size (recorded in the cdev's
  		si_iosize_max) by MAXPHYS.  This isn't strictly
  		necessary, because physio(9) will limit it to
  		MAXPHYS, but it will provide some clarity for the
  		application.
  
  		Record the controller's maximum I/O size reported
  		in the Path Inquiry CCB.
  
  sa.4:		Document the block size behavior, and explain that
  		the option of allowing physio(9) to split the I/O
  		will disappear in FreeBSD 11.0.
  
  Sponsored by:	Spectra Logic

Modified:
  head/share/man/man4/sa.4
  head/sys/cam/scsi/scsi_sa.c
  head/sys/kern/kern_physio.c
  head/sys/sys/conf.h
  head/sys/sys/param.h

Modified: head/share/man/man4/sa.4
==============================================================================
--- head/share/man/man4/sa.4	Sat Aug 24 01:50:31 2013	(r254759)
+++ head/share/man/man4/sa.4	Sat Aug 24 04:52:22 2013	(r254760)
@@ -25,7 +25,7 @@
 .\"
 .\" $FreeBSD$
 .\"
-.Dd June 6, 1999
+.Dd August 23, 2013
 .Dt SA 4
 .Os
 .Sh NAME
@@ -159,6 +159,70 @@ of 0.
 (As above, if the file mark is never read, it remains for the next
 process to read if in no-rewind mode.)
 .El
+.Sh BLOCK SIZES
+By default, the driver will NOT accept reads or writes to a tape device that
+are larger than may be written to or read from the mounted tape using a single
+write or read request.
+Because of this, the application author may have confidence that his wishes
+are respected in terms of the block size written to tape.
+For example, if the user tries to write a 256KB block to the tape, but the
+controller can handle no more than 128KB, the write will fail.
+The previous
+.Fx
+behavior, prior to
+.Fx
+10.0,
+was to break up large reads or writes into smaller blocks when going to the
+tape.
+The problem with that behavior, though, is that it hides the actual on-tape
+block size from the application writer, at least in variable block mode.
+.Pp
+If the user would like his large reads and writes broken up into separate
+pieces, he may set the following loader tunables.
+Note that these tunables WILL GO AWAY in
+.Fx 11.0 .
+They are provided for transition purposes only.
+.Bl -tag -width 12
+.It kern.cam.sa.allow_io_split
+.Pp
+This variable, when set to 1, will configure all
+.Nm
+devices to split large buffers into smaller pieces when needed.
+.It kern.cam.sa.%d.allow_io_split
+.Pp
+This variable, when set to 1, will configure the given
+.Nm
+unit to split large buffers into multiple pieces.
+This will override the global setting, if it exists.
+.El
+.Pp
+There are several
+.Xr sysctl 8
+variables available to view block handling parameters:
+.Bl -tag -width 12
+.It kern.cam.sa.%d.allow_io_split
+.Pp
+This variable allows the user to see, but not modify, the current I/O split
+setting.
+The user is not permitted to modify this setting so that there is no chance
+of behavior changing for the application while a tape is mounted.
+.It kern.cam.sa.%d.maxio
+.Pp
+This variable shows the maximum I/O size in bytes that is allowed by the
+combination of kernel tuning parameters (MAXPHYS, DFLTPHYS) and the
+capabilities of the controller that is attached to the tape drive.
+Applications may look at this value for a guide on how large an I/O may be
+permitted, but should keep in mind that the actual maximum may be
+restricted further by the tape drive via the
+.Tn SCSI
+READ BLOCK LIMITS command.
+.It kern.cam.sa.%d.cpi_maxio
+.Pp
+This variable shows the maximum I/O size supported by the controller, in
+bytes, that is reported via the CAM Path Inquiry CCB (XPT_PATH_INQ).
+If this is 0, that means that the controller has not reported a maximum I/O
+size.
+.El
 .Sh FILE MARK HANDLING
 The handling of file marks on write is automatic.
 If the user has

Modified: head/sys/cam/scsi/scsi_sa.c
==============================================================================
--- head/sys/cam/scsi/scsi_sa.c	Sat Aug 24 01:50:31 2013	(r254759)
+++ head/sys/cam/scsi/scsi_sa.c	Sat Aug 24 04:52:22 2013	(r254760)
@@ -43,6 +43,8 @@ __FBSDID("$FreeBSD$");
 #include <sys/mtio.h>
 #ifdef _KERNEL
 #include <sys/conf.h>
+#include <sys/sysctl.h>
+#include <sys/taskqueue.h>
 #endif
 #include <sys/fcntl.h>
 #include <sys/devicestat.h>
@@ -223,6 +225,8 @@ struct sa_softc {
 	u_int32_t	max_blk;
 	u_int32_t	min_blk;
 	u_int32_t	maxio;
+	u_int32_t	cpi_maxio;
+	int		allow_io_split;
 	u_int32_t	comp_algorithm;
 	u_int32_t	saved_comp_algorithm;
 	u_int32_t	media_blksize;
@@ -268,6 +272,10 @@ struct sa_softc {
 		open_rdonly		: 1,	/* open read-only */
 		open_pending_mount	: 1,	/* open pending mount */
 		ctrl_mode		: 1;	/* control device open */
+
+	struct task		sysctl_task;
+	struct sysctl_ctx_list	sysctl_ctx;
+	struct sysctl_oid	*sysctl_tree;
 };
 
 struct sa_quirk_entry {
@@ -426,6 +434,22 @@ static int		sardpos(struct cam_periph *p
 static int		sasetpos(struct cam_periph *periph, int, u_int32_t *);
 
 
+#ifndef	SA_DEFAULT_IO_SPLIT
+#define	SA_DEFAULT_IO_SPLIT	0
+#endif
+
+static int sa_allow_io_split = SA_DEFAULT_IO_SPLIT;
+
+/*
+ * Tunable to allow the user to set a global allow_io_split value.  Note
+ * that this WILL GO AWAY in FreeBSD 11.0.  Silently splitting the I/O up
+ * is bad behavior, because it hides the true tape block size from the
+ * application.
+ */
+TUNABLE_INT("kern.cam.sa.allow_io_split", &sa_allow_io_split);
+static SYSCTL_NODE(_kern_cam, OID_AUTO, sa, CTLFLAG_RD, 0,
+		  "CAM Sequential Access Tape Driver");
+
 static struct periph_driver sadriver =
 {
 	sainit, "sa",
@@ -1448,6 +1472,49 @@ saasync(void *callback_arg, u_int32_t co
 	}
 }
 
+static void
+sasysctlinit(void *context, int pending)
+{
+	struct cam_periph *periph;
+	struct sa_softc *softc;
+	char tmpstr[80], tmpstr2[80];
+
+	periph = (struct cam_periph *)context;
+	/*
+	 * If the periph is invalid, no need to setup the sysctls.
+	 */
+	if (periph->flags & CAM_PERIPH_INVALID)
+		goto bailout;
+
+	softc = (struct sa_softc *)periph->softc;
+
+	snprintf(tmpstr, sizeof(tmpstr), "CAM SA unit %d", periph->unit_number);
+	snprintf(tmpstr2, sizeof(tmpstr2), "%u", periph->unit_number);
+
+	sysctl_ctx_init(&softc->sysctl_ctx);
+	softc->sysctl_tree = SYSCTL_ADD_NODE(&softc->sysctl_ctx,
+	    SYSCTL_STATIC_CHILDREN(_kern_cam_sa), OID_AUTO, tmpstr2,
+                    CTLFLAG_RD, 0, tmpstr);
+	if (softc->sysctl_tree == NULL)
+		goto bailout;
+
+	SYSCTL_ADD_INT(&softc->sysctl_ctx, SYSCTL_CHILDREN(softc->sysctl_tree),
+	    OID_AUTO, "allow_io_split", CTLTYPE_INT | CTLFLAG_RDTUN, 
+	    &softc->allow_io_split, 0, "Allow Splitting I/O");
+	SYSCTL_ADD_INT(&softc->sysctl_ctx, SYSCTL_CHILDREN(softc->sysctl_tree),
+	    OID_AUTO, "maxio", CTLTYPE_INT | CTLFLAG_RD, 
+	    &softc->maxio, 0, "Maximum I/O size");
+	SYSCTL_ADD_INT(&softc->sysctl_ctx, SYSCTL_CHILDREN(softc->sysctl_tree),
+	    OID_AUTO, "cpi_maxio", CTLTYPE_INT | CTLFLAG_RD, 
+	    &softc->cpi_maxio, 0, "Maximum Controller I/O size");
+
+bailout:
+	/*
+	 * Release the reference that was held when this task was enqueued.
+	 */
+	cam_periph_release(periph);
+}
+
 static cam_status
 saregister(struct cam_periph *periph, void *arg)
 {
@@ -1455,6 +1522,7 @@ saregister(struct cam_periph *periph, vo
 	struct ccb_getdev *cgd;
 	struct ccb_pathinq cpi;
 	caddr_t match;
+	char tmpstr[80];
 	int i;
 	
 	cgd = (struct ccb_getdev *)arg;
@@ -1509,21 +1577,55 @@ saregister(struct cam_periph *periph, vo
 	    XPORT_DEVSTAT_TYPE(cpi.transport), DEVSTAT_PRIORITY_TAPE);
 
 	/*
-	 * If maxio isn't set, we fall back to DFLTPHYS.  If it is set, we
-	 * take it whether or not it's larger than MAXPHYS.  physio will
-	 * break it down into pieces small enough to fit in a buffer.
+	 * Load the default value that is either compiled in, or loaded 
+	 * in the global kern.cam.sa.allow_io_split tunable.
+	 */
+	softc->allow_io_split = sa_allow_io_split;
+
+	/*
+	 * Load a per-instance tunable, if it exists.  NOTE that this
+	 * tunable WILL GO AWAY in FreeBSD 11.0.
+	 */ 
+	snprintf(tmpstr, sizeof(tmpstr), "kern.cam.sa.%u.allow_io_split",
+		 periph->unit_number);
+	TUNABLE_INT_FETCH(tmpstr, &softc->allow_io_split);
+
+	/*
+	 * If maxio isn't set, we fall back to DFLTPHYS.  Otherwise we take
+	 * the smaller of cpi.maxio or MAXPHYS.
 	 */
 	if (cpi.maxio == 0)
 		softc->maxio = DFLTPHYS;
+	else if (cpi.maxio > MAXPHYS)
+		softc->maxio = MAXPHYS;
 	else
 		softc->maxio = cpi.maxio;
 
 	/*
+	 * Record the controller's maximum I/O size so we can report it to
+	 * the user later.
+	 */
+	softc->cpi_maxio = cpi.maxio;
+
+	/*
+	 * By default we tell physio that we do not want our I/O split.
+	 * The user needs to have a 1:1 mapping between the size of his
+	 * write to a tape character device and the size of the write
+	 * that actually goes down to the drive.
+	 */
+	if (softc->allow_io_split == 0)
+		softc->si_flags = SI_NOSPLIT;
+	else
+		softc->si_flags = 0;
+
+	TASK_INIT(&softc->sysctl_task, 0, sasysctlinit, periph);
+
+	/*
 	 * If the SIM supports unmapped I/O, let physio know that we can
 	 * handle unmapped buffers.
 	 */
 	if (cpi.hba_misc & PIM_UNMAPPED)
-		softc->si_flags = SI_UNMAPPED;
+		softc->si_flags |= SI_UNMAPPED;
 
 	softc->devs.ctl_dev = make_dev(&sa_cdevsw, SAMINOR(SA_CTLDEV,
 	    0, SA_ATYPE_R), UID_ROOT, GID_OPERATOR,
@@ -1586,6 +1688,13 @@ saregister(struct cam_periph *periph, vo
 	cam_periph_lock(periph);
 
 	/*
+	 * Bump the peripheral refcount for the sysctl thread, in case we
+	 * get invalidated before the thread has a chance to run.
+	 */
+	cam_periph_acquire(periph);
+	taskqueue_enqueue(taskqueue_thread, &softc->sysctl_task);
+
+	/*
 	 * Add an async callback so that we get
 	 * notified if this device goes away.
 	 */

Modified: head/sys/kern/kern_physio.c
==============================================================================
--- head/sys/kern/kern_physio.c	Sat Aug 24 01:50:31 2013	(r254759)
+++ head/sys/kern/kern_physio.c	Sat Aug 24 04:52:22 2013	(r254760)
@@ -54,6 +54,36 @@ physio(struct cdev *dev, struct uio *uio
 		dev->si_iosize_max = DFLTPHYS;
 	}
 
+	/*
+	 * If the driver does not want I/O to be split, that means that we
+	 * need to reject any requests that will not fit into one buffer.
+	 */
+	if ((dev->si_flags & SI_NOSPLIT) &&
+	    ((uio->uio_resid > dev->si_iosize_max) ||
+	     (uio->uio_resid > MAXPHYS) ||
+	     (uio->uio_iovcnt > 1))) {
+		/*
+		 * Tell the user why his I/O was rejected.
+		 */
+		if (uio->uio_resid > dev->si_iosize_max)
+			printf("%s: request size %zd > si_iosize_max=%d, "
+			    "cannot split request\n", devtoname(dev),
+			    uio->uio_resid, dev->si_iosize_max);
+
+		if (uio->uio_resid > MAXPHYS)
+			printf("%s: request size %zd > MAXPHYS=%d, "
+			    "cannot split request\n", devtoname(dev),
+			    uio->uio_resid, MAXPHYS);
+
+		if (uio->uio_iovcnt > 1)
+			printf("%s: request vectors=%d > 1, "
+			    "cannot split request\n", devtoname(dev),
+			    uio->uio_iovcnt);
+
+		error = EFBIG;
+		goto doerror;
+	}
+
 	for (i = 0; i < uio->uio_iovcnt; i++) {
 		while (uio->uio_iov[i].iov_len) {
 			bp->b_flags = 0;
@@ -83,6 +113,17 @@ physio(struct cdev *dev, struct uio *uio
 			 */
 			iolen = ((vm_offset_t) bp->b_data) & PAGE_MASK;
 			if ((bp->b_bcount + iolen) > bp->b_kvasize) {
+				/*
+				 * This device does not want I/O to be split.
+				 */
+				if (dev->si_flags & SI_NOSPLIT) {
+					printf("%s: request ptr %#jx is not "
+					    "on a page boundary, cannot split "
+					    "request\n", devtoname(dev),
+					    (uintmax_t)bp->b_data);
+					error = EFBIG;
+					goto doerror;
+				}
 				bp->b_bcount = bp->b_kvasize;
 				if (iolen != 0)
 					bp->b_bcount -= PAGE_SIZE;

Modified: head/sys/sys/conf.h
==============================================================================
--- head/sys/sys/conf.h	Sat Aug 24 01:50:31 2013	(r254759)
+++ head/sys/sys/conf.h	Sat Aug 24 04:52:22 2013	(r254760)
@@ -62,6 +62,7 @@ struct cdev {
 #define	SI_DUMPDEV	0x0080	/* is kernel dumpdev */
 #define	SI_CLONELIST	0x0200	/* on a clone list */
 #define	SI_UNMAPPED	0x0400	/* can handle unmapped I/O */
+#define	SI_NOSPLIT	0x0800	/* I/O should not be split up */
 	struct timespec	si_atime;
 	struct timespec	si_ctime;
 	struct timespec	si_mtime;

Modified: head/sys/sys/param.h
==============================================================================
--- head/sys/sys/param.h	Sat Aug 24 01:50:31 2013	(r254759)
+++ head/sys/sys/param.h	Sat Aug 24 04:52:22 2013	(r254760)
@@ -58,7 +58,7 @@
  *		in the range 5 to 9.
  */
 #undef __FreeBSD_version
-#define __FreeBSD_version 1000048	/* Master, propagated to newvers */
+#define __FreeBSD_version 1000049	/* Master, propagated to newvers */
 
 /*
  * __FreeBSD_kernel__ indicates that this system uses the kernel of FreeBSD,



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