Re: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature termination

James Bottomley
2017-02-07T06:38:59Z

On Mon, 2017-02-06 at 23:26 +0100, Willy Tarreau wrote:
> Hi Sathya,
>
> On Mon, Feb 06, 2017 at 09:21:44AM -0700, Sathya Prakash Veerichetty
> wrote:
> > Willy,
> > I think this patch had a problem and later modified to a different
> > blocking mechanism. Could you please pull in the latest change for
> > this?
>
> Much appreciated, thanks. I've checked and found the patch you're
> talking about :
>
> commit ffb58456589443ca572221fabbdef3db8483a779
> Author: James Bottomley <James.Bottomley@HansenPartnership.com>
> Date: Sun Jan 1 09:39:24 2017 -0800
>
> 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 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>
>
> We don't have the referenced commit above in 3.10 so we should be
> safe. Additionally I checked that neither 4.4 nor 3.12 have them
> either, so that makes me feel confident that we can skip it in 3.10
> as well.

The original was also racy with respect to multiple commands, so the
above fixed the race as well.

James