From patchwork Wed Jul 18 22:53:11 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Keith Busch X-Patchwork-Id: 10533399 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 A736D600D0 for ; Wed, 18 Jul 2018 22:53:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 992A929A66 for ; Wed, 18 Jul 2018 22:53:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8D0C829A6A; Wed, 18 Jul 2018 22:53:22 +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=-7.9 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI autolearn=ham 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 1D68829A66 for ; Wed, 18 Jul 2018 22:53:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728304AbeGRXd0 (ORCPT ); Wed, 18 Jul 2018 19:33:26 -0400 Received: from mga03.intel.com ([134.134.136.65]:49657 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728103AbeGRXd0 (ORCPT ); Wed, 18 Jul 2018 19:33:26 -0400 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jul 2018 15:53:20 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,371,1526367600"; d="scan'208";a="57609347" Received: from unknown (HELO localhost.localdomain) ([10.232.112.44]) by orsmga007.jf.intel.com with ESMTP; 18 Jul 2018 15:53:19 -0700 Date: Wed, 18 Jul 2018 16:53:11 -0600 From: Keith Busch To: "hch@lst.de" Cc: Bart Van Assche , "keith.busch@intel.com" , "linux-block@vger.kernel.org" , "linux-nvme@lists.infradead.org" , "axboe@kernel.dk" , "ming.lei@redhat.com" , jianchao.w.wang@oracle.com Subject: Re: [RFC PATCH 3/3] blk-mq: Remove generation seqeunce Message-ID: <20180718225311.GD32160@localhost.localdomain> References: <20180712192437.GA16839@localhost.localdomain> <7cad25b821c3a640e036f28ff1bbe51e7362d25d.camel@wdc.com> <20180713154346.GA18955@localhost.localdomain> <20180713184712.GA19419@localhost.localdomain> <291a13b35af1b65fbbe99a3a9cfc7d570a620cd9.camel@wdc.com> <20180713235807.GA19967@localhost.localdomain> <20180718195650.GA20336@lst.de> <20180718203936.GA21011@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180718203936.GA21011@lst.de> User-Agent: Mutt/1.9.1 (2017-09-22) Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Jul 18, 2018 at 10:39:36PM +0200, hch@lst.de wrote: > On Wed, Jul 18, 2018 at 09:56:50PM +0200, hch@lst.de wrote: > > On Fri, Jul 13, 2018 at 05:58:08PM -0600, Keith Busch wrote: > > > Of the two you mentioned, yours is preferable IMO. While I appreciate > > > Jianchao's detailed analysis, it's hard to take a proposal seriously > > > that so colourfully calls everyone else "dangerous" while advocating > > > for silently losing requests on purpose. > > > > > > But where's the option that fixes scsi to handle hardware completions > > > concurrently with arbitrary timeout software? Propping up that house of > > > cards can't be the only recourse. > > > > The important bit is that we need to fix this issue quickly. We are > > past -rc5 so I'm rather concerned about anything too complicated. > > > > I'm not even sure SCSI has a problem with multiple completions happening > > at the same time, but it certainly has a problem with bypassing > > blk_mq_complete_request from the EH path. > > > > I think we can solve this properly, but I also think we are way to late > > in the 4.18 cycle to fix it properly. For now I fear we'll just have > > to revert the changes and try again for 4.19 or even 4.20 if we don't > > act quickly enough. > > So here is a quick attempt at the revert while also trying to keep > nvme working. Keith, Bart, Jianchao - does this looks reasonable > as a 4.18 band aid? > > http://git.infradead.org/users/hch/block.git/shortlog/refs/heads/blk-eh-revert Hm, I not really a fan. The far majority of blk-mq drivers don't even implement a timeout, and reverting it will lose their requests forever if they complete at the same time as a timeout. Of the remaining drivers, most of those don't want the reverted behavior either. It actually looks like scsi is the only mq driver that wants to block completions. In the short term, scsi can make that happen with just three lines of code. --- -- diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 8932ae81a15a..03986af3076c 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -286,6 +286,10 @@ enum blk_eh_timer_return scsi_times_out(struct request *req) enum blk_eh_timer_return rtn = BLK_EH_DONE; struct Scsi_Host *host = scmd->device->host; + if (req->q->mq_ops && cmpxchg(&rq->state, MQ_RQ_IN_FLIGHT, MQ_RQ_COMPLETE) != + MQ_RQ_IN_FLIGHT); + return rtn; + trace_scsi_dispatch_cmd_timeout(scmd); scsi_log_completion(scmd, TIMEOUT_ERROR);