Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 18 Jan 2018 21:59:45 -0700
From:   Keith Busch <keith.busch@intel.com>
To:     Jianchao Wang <jianchao.w.wang@oracle.com>
Cc:     axboe@fb.com, hch@lst.de, sagi@grimberg.me, maxg@mellanox.com,
        james.smart@broadcom.com, linux-nvme@lists.infradead.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH V5 2/2] nvme-pci: fixup the timeout case when reset is
 ongoing
Message-ID: <20180119045944.GC12043@localhost.localdomain>
References: <1516270202-8051-1-git-send-email-jianchao.w.wang@oracle.com>
 <1516270202-8051-3-git-send-email-jianchao.w.wang@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1516270202-8051-3-git-send-email-jianchao.w.wang@oracle.com>
User-Agent: Mutt/1.9.1 (2017-09-22)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Jan 18, 2018 at 06:10:02PM +0800, Jianchao Wang wrote:
> +	 * - When the ctrl.state is NVME_CTRL_RESETTING, the expired
> +	 *   request should come from the previous work and we handle
> +	 *   it as nvme_cancel_request.
> +	 * - When the ctrl.state is NVME_CTRL_RECONNECTING, the expired
> +	 *   request should come from the initializing procedure such as
> +	 *   setup io queues, because all the previous outstanding
> +	 *   requests should have been cancelled.
>  	 */
> -	if (dev->ctrl.state == NVME_CTRL_RESETTING) {
> -		dev_warn(dev->ctrl.device,
> -			 "I/O %d QID %d timeout, disable controller\n",
> -			 req->tag, nvmeq->qid);
> -		nvme_dev_disable(dev, false);
> +	switch (dev->ctrl.state) {
> +	case NVME_CTRL_RESETTING:
> +		nvme_req(req)->status = NVME_SC_ABORT_REQ;
> +		return BLK_EH_HANDLED;
> +	case NVME_CTRL_RECONNECTING:
> +		WARN_ON_ONCE(nvmeq->qid);
>  		nvme_req(req)->flags |= NVME_REQ_CANCELLED;
>  		return BLK_EH_HANDLED;
> +	default:
> +		break;
>  	}

The driver may be giving up on the command here, but that doesn't mean
the controller has. We can't just end the request like this because that
will release the memory the controller still owns. We must wait until
after nvme_dev_disable clears bus master because we can't say for sure
the controller isn't going to write to that address right after we end
the request.