On 01/01/2017 12:39 PM, James Bottomley wrote:
On Sun, 2017-01-01 at 11:33 -0500, David Miller wrote:
From: Bart Van Assche <bart.vanass...@sandisk.com>
Date: Sun, 1 Jan 2017 14:22:11 +0000

My recommendation is to revert commit 18f6084a989b ("scsi: mpt3sas: Fix
secure erase premature termination"). Since the mpt3sas driver uses the
single-queue approach and since the SCSI core unlocks the block layer
request queue lock before the .queuecommand callback function is called,
multiple threads can execute that callback function (scsih_qcmd() in this
case) simultaneously. This means that using scsi_internal_device_block()
from inside .queuecommand to serialize SCSI command execution is wrong.

But this causes a regression for the thing being fixed by that
commit.

Right, we don't do that; that's why I didn't list it as one of the
options.

Why don't we figure out what that semantics that commit was trying to
achieve and implement that properly?

Now that I look at the reviews, each of the reviewers said what the
correct thing to do was: return SAM_STAT_BUSY if SATL commands are
outstanding like the spec says.  You all get negative brownie points
for not insisting on a rework.

Does this patch (compile tested only) fix the problems for everyone?


Hi,

Yes, with this patch applied my system boots :)

...

@@ -4083,6 +4077,16 @@ scsih_qcmd(struct Scsi_Host *shost, struct scsi_cmnd 
*scmd)
                return 0;
        }

+       /*
+        * Bug work around for firmware SATL handling
+        */
+       if (sas_device_priv_data->ata_command_pending) {
+               scmd->result = SAM_STAT_BUSY;
+               scmd->scsi_done(scmd);
+               return 0;
+       }
+       set_satl_pending(scmd, true);
+
        sas_target_priv_data = sas_device_priv_data->sas_target;

        /* invalid device handle */


I was also wondering if 2 threads could both fall through and do:
'set_satl_pending(scmd, true)'; ?

Afaict there is nothing preventing that race?

Thanks,

-Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to