Skip to content

Commit 18639c4

Browse files
jejbgregkh
authored andcommitted
scsi: mpt3sas: fix hang on ata passthrough commands
commit ffb58456589443ca572221fabbdef3db8483a779 upstream. 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 669f044170d8933c3d66d231b69ea97cb8447338 Author: Bart Van Assche <bart.vanassche@sandisk.com> 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: 18f6084a989ba1b (mpt3sas: Fix secure erase premature termination) Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> Acked-by: Sreekanth Reddy <Sreekanth.Reddy@broadcom.com> Reviewed-by: Christoph Hellwig <hch@lst.de> Reported-by: Ingo Molnar <mingo@kernel.org> Tested-by: Ingo Molnar <mingo@kernel.org> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> Cc: Joe Korty <joe.korty@ccur.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 1eed198 commit 18639c4

2 files changed

Lines changed: 38 additions & 14 deletions

File tree

drivers/scsi/mpt3sas/mpt3sas_base.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ struct MPT3SAS_TARGET {
390390
* @eedp_enable: eedp support enable bit
391391
* @eedp_type: 0(type_1), 1(type_2), 2(type_3)
392392
* @eedp_block_length: block size
393+
* @ata_command_pending: SATL passthrough outstanding for device
393394
*/
394395
struct MPT3SAS_DEVICE {
395396
struct MPT3SAS_TARGET *sas_target;
@@ -398,6 +399,17 @@ struct MPT3SAS_DEVICE {
398399
u8 configured_lun;
399400
u8 block;
400401
u8 tlr_snoop_check;
402+
/*
403+
* Bug workaround for SATL handling: the mpt2/3sas firmware
404+
* doesn't return BUSY or TASK_SET_FULL for subsequent
405+
* commands while a SATL pass through is in operation as the
406+
* spec requires, it simply does nothing with them until the
407+
* pass through completes, causing them possibly to timeout if
408+
* the passthrough is a long executing command (like format or
409+
* secure erase). This variable allows us to do the right
410+
* thing while a SATL command is pending.
411+
*/
412+
unsigned long ata_command_pending;
401413
};
402414

403415
#define MPT3_CMD_NOT_USED 0x8000 /* free */

drivers/scsi/mpt3sas/mpt3sas_scsih.c

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3707,9 +3707,18 @@ _scsih_temp_threshold_events(struct MPT3SAS_ADAPTER *ioc,
37073707
}
37083708
}
37093709

3710-
static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd)
3710+
static int _scsih_set_satl_pending(struct scsi_cmnd *scmd, bool pending)
37113711
{
3712-
return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16);
3712+
struct MPT3SAS_DEVICE *priv = scmd->device->hostdata;
3713+
3714+
if (scmd->cmnd[0] != ATA_12 && scmd->cmnd[0] != ATA_16)
3715+
return 0;
3716+
3717+
if (pending)
3718+
return test_and_set_bit(0, &priv->ata_command_pending);
3719+
3720+
clear_bit(0, &priv->ata_command_pending);
3721+
return 0;
37133722
}
37143723

37153724
/**
@@ -3733,9 +3742,7 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc)
37333742
if (!scmd)
37343743
continue;
37353744
count++;
3736-
if (ata_12_16_cmd(scmd))
3737-
scsi_internal_device_unblock(scmd->device,
3738-
SDEV_RUNNING);
3745+
_scsih_set_satl_pending(scmd, false);
37393746
mpt3sas_base_free_smid(ioc, smid);
37403747
scsi_dma_unmap(scmd);
37413748
if (ioc->pci_error_recovery)
@@ -3866,13 +3873,6 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
38663873
if (ioc->logging_level & MPT_DEBUG_SCSI)
38673874
scsi_print_command(scmd);
38683875

3869-
/*
3870-
* Lock the device for any subsequent command until command is
3871-
* done.
3872-
*/
3873-
if (ata_12_16_cmd(scmd))
3874-
scsi_internal_device_block(scmd->device);
3875-
38763876
sas_device_priv_data = scmd->device->hostdata;
38773877
if (!sas_device_priv_data || !sas_device_priv_data->sas_target) {
38783878
scmd->result = DID_NO_CONNECT << 16;
@@ -3886,6 +3886,19 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd *scmd)
38863886
return 0;
38873887
}
38883888

3889+
/*
3890+
* Bug work around for firmware SATL handling. The loop
3891+
* is based on atomic operations and ensures consistency
3892+
* since we're lockless at this point
3893+
*/
3894+
do {
3895+
if (test_bit(0, &sas_device_priv_data->ata_command_pending)) {
3896+
scmd->result = SAM_STAT_BUSY;
3897+
scmd->scsi_done(scmd);
3898+
return 0;
3899+
}
3900+
} while (_scsih_set_satl_pending(scmd, true));
3901+
38893902
sas_target_priv_data = sas_device_priv_data->sas_target;
38903903

38913904
/* invalid device handle */
@@ -4445,8 +4458,7 @@ _scsih_io_done(struct MPT3SAS_ADAPTER *ioc, u16 smid, u8 msix_index, u32 reply)
44454458
if (scmd == NULL)
44464459
return 1;
44474460

4448-
if (ata_12_16_cmd(scmd))
4449-
scsi_internal_device_unblock(scmd->device, SDEV_RUNNING);
4461+
_scsih_set_satl_pending(scmd, false);
44504462

44514463
mpi_request = mpt3sas_base_get_msg_frame(ioc, smid);
44524464

0 commit comments

Comments
 (0)