git: 6e8a2f040017 - main - Update sa(4) comments and man page after review.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Tue, 18 Jan 2022 18:58:12 UTC
The branch main has been updated by ken:
URL: https://cgit.FreeBSD.org/src/commit/?id=6e8a2f04001735353e445570f0d83aa88d4b9b37
commit 6e8a2f04001735353e445570f0d83aa88d4b9b37
Author: Kenneth D. Merry <ken@FreeBSD.org>
AuthorDate: 2022-01-18 15:44:31 +0000
Commit: Kenneth D. Merry <ken@FreeBSD.org>
CommitDate: 2022-01-18 18:50:31 +0000
Update sa(4) comments and man page after review.
sys/cam/scsi/scsi_sa.c:
Add comments explaining the priority order of the various
sources of timeout values. Also, explain that the probe
that pulls in drive recommended timeouts via the REPORT
SUPPORTED OPERATION CODES command is in a race with the
thread that creates the sysctl variables. Because of that
race, it is important that the sysctl thread not load any
timeout values from the kernel environment.
share/man/man4/sa.4:
Use the Sy macro to emphasize thousandths of a second
instead of capitalizing it.
Requested by: Warner Losh <imp@freebsd.org>
Requested by: Daniel Ebdrup Jensen <debdrup@freebsd.org>
Sponsored by: Spectra Logic
MFC after: 1 week
Differential Revision: https://reviews.freebsd.org/D33883
---
share/man/man4/sa.4 | 14 ++++++-------
sys/cam/scsi/scsi_sa.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++--
2 files changed, 60 insertions(+), 9 deletions(-)
diff --git a/share/man/man4/sa.4 b/share/man/man4/sa.4
index 6b88fd016522..35ffcdb877d6 100644
--- a/share/man/man4/sa.4
+++ b/share/man/man4/sa.4
@@ -25,7 +25,7 @@
.\"
.\" $FreeBSD$
.\"
-.Dd January 14, 2022
+.Dd January 18, 2022
.Dt SA 4
.Os
.Sh NAME
@@ -338,8 +338,9 @@ If the drive does report timeout descriptors, the
.Nm
driver will use the drive's recommended timeouts for commands.
.Pp
-The timeouts in use are reported in units of THOUSANDTHS of a second via
-the
+The timeouts in use are reported in units of
+.Sy thousandths
+of a second via the
.Va kern.cam.sa.%d.timeout.*
.Xr sysctl 8
variables.
@@ -375,7 +376,6 @@ Loader tunables:
.Pp
.Bl -tag -compact
.It kern.cam.sa.timeout.erase
-.It kern.cam.sa.timeout.load
.It kern.cam.sa.timeout.locate
.It kern.cam.sa.timeout.mode_select
.It kern.cam.sa.timeout.mode_sense
@@ -398,7 +398,6 @@ values:
.Pp
.Bl -tag -compact
.It kern.cam.sa.%d.timeout.erase
-.It kern.cam.sa.%d.timeout.load
.It kern.cam.sa.%d.timeout.locate
.It kern.cam.sa.%d.timeout.mode_select
.It kern.cam.sa.%d.timeout.mode_sense
@@ -415,8 +414,9 @@ values:
.It kern.cam.sa.%d.timeout.write_filemarks
.El
.Pp
-As mentioned above, the timeouts are set and reported in THOUSANDTHS of a
-second, so be sure to account for that when setting them.
+As mentioned above, the timeouts are set and reported in
+.Sy thousandths
+of a second, so be sure to account for that when setting them.
.Sh IOCTLS
The
.Nm
diff --git a/sys/cam/scsi/scsi_sa.c b/sys/cam/scsi/scsi_sa.c
index b7475102af14..04d013d03a30 100644
--- a/sys/cam/scsi/scsi_sa.c
+++ b/sys/cam/scsi/scsi_sa.c
@@ -175,6 +175,22 @@ typedef enum {
SA_TIMEOUT_TYPE_MAX
} sa_timeout_types;
+/*
+ * These are the default timeout values that apply to all tape drives.
+ *
+ * We get timeouts from the following places in order of increasing
+ * priority:
+ * 1. Driver default timeouts. (Set in the structure below.)
+ * 2. Timeouts loaded from the drive via REPORT SUPPORTED OPERATION
+ * CODES. (If the drive supports it, SPC-4/LTO-5 and newer should.)
+ * 3. Global loader tunables, used for all sa(4) driver instances on
+ * a machine.
+ * 4. Instance-specific loader tunables, used for say sa5.
+ * 5. On the fly user sysctl changes.
+ *
+ * Each step will overwrite the timeout value set from the one
+ * before, so you go from general to most specific.
+ */
static struct sa_timeout_desc {
const char *desc;
int value;
@@ -2333,6 +2349,12 @@ sasetupdev(struct sa_softc *softc, struct cdev *dev)
softc->num_devs_to_destroy++;
}
+/*
+ * Load the global (for all sa(4) instances) and per-instance tunable
+ * values for timeouts for various sa(4) commands. This should be run
+ * after the default timeouts are fetched from the drive, so the user's
+ * preference will override the drive's defaults.
+ */
static void
saloadtotunables(struct sa_softc *softc)
{
@@ -2342,12 +2364,17 @@ saloadtotunables(struct sa_softc *softc)
for (i = 0; i < SA_TIMEOUT_TYPE_MAX; i++) {
int tmpval, retval;
+ /* First grab any global timeout setting */
snprintf(tmpstr, sizeof(tmpstr), "kern.cam.sa.timeout.%s",
sa_default_timeouts[i].desc);
retval = TUNABLE_INT_FETCH(tmpstr, &tmpval);
if (retval != 0)
softc->timeout_info[i] = tmpval;
+ /*
+ * Then overwrite any global timeout settings with
+ * per-instance timeout settings.
+ */
snprintf(tmpstr, sizeof(tmpstr), "kern.cam.sa.%u.timeout.%s",
softc->periph->unit_number, sa_default_timeouts[i].desc);
retval = TUNABLE_INT_FETCH(tmpstr, &tmpval);
@@ -2408,6 +2435,15 @@ sasysctlinit(void *context, int pending)
snprintf(tmpstr, sizeof(tmpstr), "%s timeout",
sa_default_timeouts[i].desc);
+ /*
+ * Do NOT change this sysctl declaration to also load any
+ * tunable values for this sa(4) instance. In other words,
+ * do not change this to CTLFLAG_RWTUN. This function is
+ * run in parallel with the probe routine that fetches
+ * recommended timeout values from the tape drive, and we
+ * don't want the values from the drive to override the
+ * user's preference.
+ */
SYSCTL_ADD_INT(&softc->sysctl_timeout_ctx,
SYSCTL_CHILDREN(softc->sysctl_timeout_tree),
OID_AUTO, sa_default_timeouts[i].desc, CTLFLAG_RW,
@@ -2660,7 +2696,9 @@ saregister(struct cam_periph *periph, void *arg)
softc->density_type_bits[3] = SRDS_MEDIUM_TYPE | SRDS_MEDIA;
/*
* Bump the peripheral refcount for the sysctl thread, in case we
- * get invalidated before the thread has a chance to run.
+ * get invalidated before the thread has a chance to run. Note
+ * that this runs in parallel with the probe for the timeout
+ * values.
*/
cam_periph_acquire(periph);
taskqueue_enqueue(taskqueue_thread, &softc->sysctl_task);
@@ -2673,7 +2711,20 @@ saregister(struct cam_periph *periph, void *arg)
/*
* See comment above, try fetching timeout values for drives that
- * might support it. Otherwise, use the defaults.
+ * might support it. Otherwise, use the defaults.
+ *
+ * We get timeouts from the following places in order of increasing
+ * priority:
+ * 1. Driver default timeouts.
+ * 2. Timeouts loaded from the drive via REPORT SUPPORTED OPERATION
+ * CODES. (We kick that off here if SA_FLAG_RSOC_TO_TRY is set.)
+ * 3. Global loader tunables, used for all sa(4) driver instances on
+ * a machine.
+ * 4. Instance-specific loader tunables, used for say sa5.
+ * 5. On the fly user sysctl changes.
+ *
+ * Each step will overwrite the timeout value set from the one
+ * before, so you go from general to most specific.
*/
if (softc->flags & SA_FLAG_RSOC_TO_TRY) {
/*