From patchwork Wed Feb 8 06:53:56 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Willy Tarreau X-Patchwork-Id: 9561813 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id EA08860236 for ; Wed, 8 Feb 2017 06:56:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DC91328495 for ; Wed, 8 Feb 2017 06:56:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id D14B22849C; Wed, 8 Feb 2017 06:56:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E2B0628495 for ; Wed, 8 Feb 2017 06:56:44 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932096AbdBHG4b (ORCPT ); Wed, 8 Feb 2017 01:56:31 -0500 Received: from wtarreau.pck.nerim.net ([62.212.114.60]:31294 "EHLO 1wt.eu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752160AbdBHG42 (ORCPT ); Wed, 8 Feb 2017 01:56:28 -0500 Received: (from willy@localhost) by pcw.home.local (8.15.2/8.15.2/Submit) id v186ru7F032157; Wed, 8 Feb 2017 07:53:56 +0100 Date: Wed, 8 Feb 2017 07:53:56 +0100 From: Willy Tarreau To: James Bottomley Cc: Sathya Prakash Veerichetty , linux-kernel@vger.kernel.org, stable@vger.kernel.org, linux@roeck-us.net, Andrey Grodzovsky , linux-scsi@vger.kernel.org, Chaitra Basappa , Suganath Prabu Subramani , Sreekanth Reddy , Hannes Reinecke , "Martin K . Petersen" Subject: Re: [PATCH 3.10 141/319] scsi: mpt3sas: Fix secure erase premature termination Message-ID: <20170208065356.GA4838@1wt.eu> References: <1486322541-8206-1-git-send-email-w@1wt.eu> <1486322541-8206-42-git-send-email-w@1wt.eu> <20170206222623.GA4769@1wt.eu> <1486449528.2474.125.camel@HansenPartnership.com> <20170207065946.GC4850@1wt.eu> <1486486971.2488.19.camel@HansenPartnership.com> <20170207171234.GB4041@1wt.eu> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170207171234.GB4041@1wt.eu> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Feb 07, 2017 at 06:12:34PM +0100, Willy Tarreau wrote: > On Tue, Feb 07, 2017 at 09:02:51AM -0800, James Bottomley wrote: > > On Tue, 2017-02-07 at 07:59 +0100, Willy Tarreau wrote: > > > Hi James, > > > > > > On Mon, Feb 06, 2017 at 10:38:48PM -0800, James Bottomley wrote: > > > > On Mon, 2017-02-06 at 23:26 +0100, Willy Tarreau wrote: > > > (...) > > > > > 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. > > > > > > OK so I tried to backport it to 3.10. I dropped a few parts which > > > were addressing this one marked for stable 4.4+ : > > > 7ff723a ("scsi: mpt3sas: Unblock device after controller reset") > > > > > > And I got the attached patch. All I know is that it builds. I'd > > > appreciate it if someone could confirm its validity, in which case > > > I'll add it. > > > > The two patches apply without fuzz to your tree and the combination is > > a far better bug fix than the original regardless of whether 7ff723a > > exists in your tree or not. By messing with the patches all you do is > > add the potential for introducing new bugs for no benefit, so why take > > risk for no upside? > > Just because I'm suggested to apply this fix which is supposed to fix > a regression brought by 7ff723a which itself is marked to fix 4.4+ only > and which doesn't apply to 3.10. So now I'm getting confused because > you say that these patches apply without fuzz but one part definitely > is rejected and the other one has to be applied by hand. I want not > to take a risk but I'm faced with these options : > - drop all these patches and stay as 3.10.104 is > - merge the "secure erase premature" + the the part of the patch > that supposedly fixes the regression it introduced > - merge this fix + 7ff723a + whatever it depends on (not fond of > it) > > In all cases I don't even have the hardware to validate anything. I'd > be more tempted with the first two options. If you think I'm taking > risks by backporting the relevant part of the fix, I'll simply drop > them all and leave the code as it is now. So I could backport the fix marked for 4.4+ (7ff723a) and the one suggested by Sathya (ffb5845). The context was slightly different but the changes obvious enough to look good. If everyone is OK, I'll add these two commits. Here are the backports. Willy From acd34b89fe261c88398e26bd3055000052eb7808 Mon Sep 17 00:00:00 2001 From: Suganath Prabu S Date: Thu, 17 Nov 2016 16:15:58 +0530 Subject: scsi: mpt3sas: Unblock device after controller reset commit 7ff723ad0f87feba43dda45fdae71206063dd7d4 upstream. While issuing any ATA passthrough command to firmware the driver will block the device. But it will unblock the device only if the I/O completes through the ISR path. If a controller reset occurs before command completion the device will remain in blocked state. Make sure we unblock the device following a controller reset if an ATA passthrough command was queued. [mkp: clarified patch description] Cc: # v4.4+ Fixes: ac6c2a93bd07 ("mpt3sas: Fix for SATA drive in blocked state, after diag reset") Signed-off-by: Suganath Prabu S Signed-off-by: Martin K. Petersen [wt: adjust context] Signed-off-by: Willy Tarreau --- drivers/scsi/mpt3sas/mpt3sas_scsih.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c index e414b71..8979403 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c @@ -3390,6 +3390,11 @@ _scsih_check_volume_delete_events(struct MPT3SAS_ADAPTER *ioc, le16_to_cpu(event_data->VolDevHandle)); } +static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd) +{ + return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16); +} + /** * _scsih_flush_running_cmds - completing outstanding commands. * @ioc: per adapter object @@ -3411,6 +3416,9 @@ _scsih_flush_running_cmds(struct MPT3SAS_ADAPTER *ioc) if (!scmd) continue; count++; + if (ata_12_16_cmd(scmd)) + scsi_internal_device_unblock(scmd->device, + SDEV_RUNNING); mpt3sas_base_free_smid(ioc, smid); scsi_dma_unmap(scmd); if (ioc->pci_error_recovery) @@ -3515,11 +3523,6 @@ _scsih_eedp_error_handling(struct scsi_cmnd *scmd, u16 ioc_status) SAM_STAT_CHECK_CONDITION; } -static inline bool ata_12_16_cmd(struct scsi_cmnd *scmd) -{ - return (scmd->cmnd[0] == ATA_12 || scmd->cmnd[0] == ATA_16); -} - /** * _scsih_qcmd_lck - main scsi request entry point * @scmd: pointer to scsi command object