svn commit: r335154 - head/sys/cam/scsi
Kenneth D. Merry
ken at FreeBSD.org
Thu Jun 14 17:08:45 UTC 2018
Author: ken
Date: Thu Jun 14 17:08:44 2018
New Revision: 335154
URL: https://svnweb.freebsd.org/changeset/base/335154
Log:
Fix da(4) locking when probing SMR drives.
Probing host aware and host managed SMR drives got broken in revision
330796.
The added cam_periph_lock() calls were in areas in dadone() where
the peripheral lock was already held.
Since then, dadone() has been split into separate functions that are
dedicated to each probe state.
The result is that when probing a host aware drive, I ran into a recursive
lock acquisition in dadone_probeatalogdir(). I would have run into the
same problem in dadone_probeataiddir(), and in dadone_probeatasup() and
dadone_probeatazone() in the error paths had the probe continued.
The solution is to take out all of the extra cam_periph_lock() calls. I
also added cam_periph_assert(periph, MA_OWNED) near the top of each of
the dadone_* calls. These make it clear to anyone coming along in the
the future that the lock is held in the probe done functions.
Also add a locking assert in daprobedone(), to make it clear that it must
be called with the periph lock held.
Sponsored by: Spectra Logic
Differential Revision: https://reviews.freebsd.org/D15764
Modified:
head/sys/cam/scsi/scsi_da.c
Modified: head/sys/cam/scsi/scsi_da.c
==============================================================================
--- head/sys/cam/scsi/scsi_da.c Thu Jun 14 17:06:19 2018 (r335153)
+++ head/sys/cam/scsi/scsi_da.c Thu Jun 14 17:08:44 2018 (r335154)
@@ -2428,6 +2428,8 @@ daprobedone(struct cam_periph *periph, union ccb *ccb)
softc = (struct da_softc *)periph->softc;
+ cam_periph_assert(periph, MA_OWNED);
+
dadeletemethodchoose(softc, DA_DELETE_NONE);
if (bootverbose && (softc->flags & DA_FLAG_ANNOUNCED) == 0) {
@@ -4505,6 +4507,8 @@ dadone_probewp(struct cam_periph *periph, union ccb *d
priority = done_ccb->ccb_h.pinfo.priority;
csio = &done_ccb->csio;
+ cam_periph_assert(periph, MA_OWNED);
+
if (softc->minimum_cmd_size > 6) {
mode_hdr10 = (struct scsi_mode_header_10 *)csio->data_ptr;
dev_spec = mode_hdr10->dev_spec;
@@ -4578,6 +4582,8 @@ dadone_proberc(struct cam_periph *periph, union ccb *d
rcaplong = (struct scsi_read_capacity_data_long *)
csio->data_ptr;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
struct disk_params *dp;
uint32_t block_size;
@@ -4835,6 +4841,8 @@ dadone_probelbp(struct cam_periph *periph, union ccb *
csio = &done_ccb->csio;
lbp = (struct scsi_vpd_logical_block_prov *)csio->data_ptr;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
/*
* T10/1799-D Revision 31 states at least one of these
@@ -4891,6 +4899,8 @@ dadone_probeblklimits(struct cam_periph *periph, union
csio = &done_ccb->csio;
block_limits = (struct scsi_vpd_block_limits *)csio->data_ptr;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
uint32_t max_txfer_len = scsi_4btoul(
block_limits->max_txfer_len);
@@ -4983,6 +4993,8 @@ dadone_probebdc(struct cam_periph *periph, union ccb *
csio = &done_ccb->csio;
bdc = (struct scsi_vpd_block_device_characteristics *)csio->data_ptr;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
uint32_t valid_len;
@@ -5088,6 +5100,8 @@ dadone_probeata(struct cam_periph *periph, union ccb *
continue_probe = 0;
error = 0;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
uint16_t old_rate;
@@ -5223,7 +5237,7 @@ dadone_probeatalogdir(struct cam_periph *periph, union
priority = done_ccb->ccb_h.pinfo.priority;
csio = &done_ccb->csio;
- cam_periph_lock(periph);
+ cam_periph_assert(periph, MA_OWNED);
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
error = 0;
softc->valid_logdir_len = 0;
@@ -5276,7 +5290,6 @@ dadone_probeatalogdir(struct cam_periph *periph, union
}
}
}
- cam_periph_unlock(periph);
free(csio->data_ptr, M_SCSIDA);
@@ -5305,7 +5318,8 @@ dadone_probeataiddir(struct cam_periph *periph, union
priority = done_ccb->ccb_h.pinfo.priority;
csio = &done_ccb->csio;
- cam_periph_lock(periph);
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
off_t entries_offset, max_entries;
error = 0;
@@ -5368,7 +5382,6 @@ dadone_probeataiddir(struct cam_periph *periph, union
}
}
}
- cam_periph_unlock(periph);
free(csio->data_ptr, M_SCSIDA);
@@ -5396,6 +5409,8 @@ dadone_probeatasup(struct cam_periph *periph, union cc
priority = done_ccb->ccb_h.pinfo.priority;
csio = &done_ccb->csio;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
uint32_t valid_len;
size_t needed_size;
@@ -5466,9 +5481,7 @@ dadone_probeatasup(struct cam_periph *periph, union cc
* Supported Capabilities page, clear the
* flag...
*/
- cam_periph_lock(periph);
softc->flags &= ~DA_FLAG_CAN_ATA_SUPCAP;
- cam_periph_unlock(periph);
/*
* And clear zone capabilities.
*/
@@ -5508,6 +5521,8 @@ dadone_probeatazone(struct cam_periph *periph, union c
softc = (struct da_softc *)periph->softc;
csio = &done_ccb->csio;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
struct ata_zoned_info_log *zi_log;
uint32_t valid_len;
@@ -5568,10 +5583,8 @@ dadone_probeatazone(struct cam_periph *periph, union c
if (error == ERESTART)
return;
else if (error != 0) {
- cam_periph_lock(periph);
softc->flags &= ~DA_FLAG_CAN_ATA_ZONE;
softc->flags &= ~DA_ZONE_FLAG_SET_MASK;
- cam_periph_unlock(periph);
if ((done_ccb->ccb_h.status & CAM_DEV_QFRZN) != 0) {
/* Don't wedge this device's queue */
@@ -5584,6 +5597,7 @@ dadone_probeatazone(struct cam_periph *periph, union c
}
}
+
free(csio->data_ptr, M_SCSIDA);
daprobedone(periph, done_ccb);
@@ -5602,6 +5616,8 @@ dadone_probezone(struct cam_periph *periph, union ccb
softc = (struct da_softc *)periph->softc;
csio = &done_ccb->csio;
+ cam_periph_assert(periph, MA_OWNED);
+
if ((csio->ccb_h.status & CAM_STATUS_MASK) == CAM_REQ_CMP) {
uint32_t valid_len;
size_t needed_len;
@@ -5672,6 +5688,9 @@ dadone_tur(struct cam_periph *periph, union ccb *done_
softc = (struct da_softc *)periph->softc;
csio = &done_ccb->csio;
+
+ cam_periph_assert(periph, MA_OWNED);
+
if ((done_ccb->ccb_h.status & CAM_STATUS_MASK) != CAM_REQ_CMP) {
if (daerror(done_ccb, CAM_RETRY_SELTO,
More information about the svn-src-all
mailing list