d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Update sa(4) comments and man page after review.

Browse Source 
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
This commit is contained in:
Kenneth D. Merry 2022-01-18 10:44:31 -05:00
parent bcff64c54a
commit 6e8a2f0400
2 changed files with 60 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
share/man/man4/sa.4
View File

@ -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

55
sys/cam/scsi/scsi_sa.c
View File

@ -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) {
/*