Skip to content

Commit ffb5845

Browse files
James Bottomleymartinkpetersen
authored andcommitted
scsi: mpt3sas: fix hang on ata passthrough commands
mpt3sas has a firmware failure where it can only handle one pass through ATA command at a time. If another comes in, contrary to the SAT standard, it will hang until the first one completes (causing long commands like secure erase to timeout). The original fix was to block the device when an ATA command came in, but this caused a regression with commit 669f044 Author: Bart Van Assche <[email protected]> Date: Tue Nov 22 16:17:13 2016 -0800 scsi: srp_transport: Move queuecommand() wait code to SCSI core So fix the original fix of the secure erase timeout by properly returning SAM_STAT_BUSY like the SAT recommends. The original patch also had a concurrency problem since scsih_qcmd is lockless at that point (this is fixed by using atomic bitops to set and test the flag). [mkp: addressed feedback wrt. test_bit and fixed whitespace] Fixes: 18f6084 (mpt3sas: Fix secure erase premature termination) Signed-off-by: James Bottomley <[email protected]> Acked-by: Sreekanth Reddy <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Reported-by: Ingo Molnar <[email protected]> Tested-by: Ingo Molnar <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent 8667f51 commit ffb5845

File tree

2 files changed

+38
-14
lines changed

2 files changed

+38
-14
lines changed

drivers/scsi/mpt3sas/mpt3sas_base.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ struct MPT3SAS_TARGET {
393393
* @eedp_enable: eedp support enable bit
394394
* @eedp_type: 0(type_1), 1(type_2), 2(type_3)
395395
* @eedp_block_length: block size
396+
* @ata_command_pending: SATL passthrough outstanding for device
396397
*/
397398
struct MPT3SAS_DEVICE {
398399
struct MPT3SAS_TARGET *sas_target;
@@ -404,6 +405,17 @@ struct MPT3SAS_DEVICE {
404405
u8 ignore_delay_remove;
405406
/* Iopriority Command Handling */
406407
u8 ncq_prio_enable;
408+
/*
409+
* Bug workaround for SATL handling: the mpt2/3sas firmware
410+
* doesn't return BUSY or TASK_SET_FULL for subsequent
411+
* commands while a SATL pass through is in operation as the
412+
* spec requires, it simply does nothing with them until the
413+
* pass through completes, causing them possibly to timeout if
414+
* the passthrough is a long executing command (like format or
415+
* secure erase). This variable allows us to do the right
416+
* thing while a SATL command is pending.
417+
*/
418+
unsigned long ata_command_pending;
407419

408420
};
409421

drivers/scsi/mpt3sas/mpt3sas_scsih.c

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3899,9 +3899,18 @@ _scsih_temp_threshold_events(struct MPT3SAS_ADAPTER *ioc,
38993899
}
39003900
}
39013901

3902-
static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
3902+
static int _scsih_set_satl_pending(struct scsi_cmnd *scmd, bool pending)
39033903
{
3904-
return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
3904+
struct MPT3SAS_DEVICE *priv = scmd->device->hostdata;
3905+
3906+
if (scmd->cmnd[0] != ATA_12 && scmd->cmnd[0] != ATA_16)
3907+
return 0;
3908+
3909+
if (pending)
3910+
return test_and_set_bit(0, &priv->ata_command_pending);
3911+
3912+
clear_bit(0, &priv->ata_command_pending);
3913+
return 0;
39053914
}
39063915

39073916
/**
@@ -3925,9 +3934,7 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc)
39253934
if (!scmd)
39263935
continue;
39273936
count++;
3928-
if (ata_12_16_cmd(scmd))
3929-
scsi_internal_device_unblock(scmd->device,
3930-
SDEV_RUNNING);
3937+
_scsih_set_satl_pending(scmd, false);
39313938
mpt3sas_base_free_smid(ioc, smid);
39323939
scsi_dma_unmap(scmd);
39333940
if (ioc->pci_error_recovery)
@@ -4063,13 +4070,6 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
40634070
if (ioc->logging_level & MPT_DEBUG_SCSI)
40644071
scsi_print_command(scmd);
40654072

4066-
/*
4067-
* Lock the device for any subsequent command until command is
4068-
* done.
4069-
*/
4070-
if (ata_12_16_cmd(scmd))
4071-
scsi_internal_device_block(scmd->device);
4072-
40734073
sas_device_priv_data = scmd->device->hostdata;
40744074
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
40754075
scmd->result = DID_NO_CONNECT << 16;
@@ -4083,6 +4083,19 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
40834083
return 0;
40844084
}
40854085

4086+
/*
4087+
* Bug work around for firmware SATL handling. The loop
4088+
* is based on atomic operations and ensures consistency
4089+
* since we're lockless at this point
4090+
*/
4091+
do {
4092+
if (test_bit(0, &sas_device_priv_data->ata_command_pending)) {
4093+
scmd->result = SAM_STAT_BUSY;
4094+
scmd->scsi_done(scmd);
4095+
return 0;
4096+
}
4097+
} while (_scsih_set_satl_pending(scmd, true));
4098+
40864099
sas_target_priv_data = sas_device_priv_data->sas_target;
40874100

40884101
/* invalid device handle */
@@ -4650,8 +4663,7 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
46504663
if (scmd == NULL)
46514664
return 1;
46524665

4653-
if (ata_12_16_cmd(scmd))
4654-
scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
4666+
_scsih_set_satl_pending(scmd, false);
46554667

46564668
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
46574669

0 commit comments

Comments
 (0)