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
This commit is contained in:
parent
3557869e5f
commit
e4b58dfe33
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=335154
@ -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 *done_ccb)
|
||||
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 *done_ccb)
|
||||
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 *done_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 ccb *done_ccb)
|
||||
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 *done_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 *done_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 ccb *done_ccb)
|
||||
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 ccb *done_ccb)
|
||||
}
|
||||
}
|
||||
}
|
||||
cam_periph_unlock(periph);
|
||||
|
||||
free(csio->data_ptr, M_SCSIDA);
|
||||
|
||||
@ -5305,7 +5318,8 @@ dadone_probeataiddir(struct cam_periph *periph, union ccb *done_ccb)
|
||||
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 ccb *done_ccb)
|
||||
}
|
||||
}
|
||||
}
|
||||
cam_periph_unlock(periph);
|
||||
|
||||
free(csio->data_ptr, M_SCSIDA);
|
||||
|
||||
@ -5396,6 +5409,8 @@ dadone_probeatasup(struct cam_periph *periph, union ccb *done_ccb)
|
||||
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 ccb *done_ccb)
|
||||
* 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 ccb *done_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) {
|
||||
struct ata_zoned_info_log *zi_log;
|
||||
uint32_t valid_len;
|
||||
@ -5568,10 +5583,8 @@ dadone_probeatazone(struct cam_periph *periph, union ccb *done_ccb)
|
||||
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 ccb *done_ccb)
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
free(csio->data_ptr, M_SCSIDA);
|
||||
|
||||
daprobedone(periph, done_ccb);
|
||||
@ -5602,6 +5616,8 @@ dadone_probezone(struct cam_periph *periph, union ccb *done_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_ccb)
|
||||
|
||||
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,
|
||||
|
Loading…
Reference in New Issue
Block a user