Re: [PATCH 2/2] nvme: pci: guarantee EH can make progress

Keith Busch
2018-04-26T16:24:43Z

On Thu, Apr 26, 2018 at 08:39:56PM +0800, Ming Lei wrote:
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 5d05a04f8e72..1e058deb4718 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -1265,6 +1265,20 @@ static enum blk_eh_timer_return nvme_timeout(struct request *req, bool reserved)
> struct nvme_command cmd;
> u32 csts = readl(dev->bar + NVME_REG_CSTS);
>
> + /*
> + * If error recovery is in-progress and this request needn't to
> + * be retried, return BLK_EH_HANDLED immediately, so that error
> + * handler kthread can always make progress since we still need
> + * to send FAILFAST request to admin queue for handling error.
> + */
> + spin_lock(&dev->eh_lock);
> + if (dev->eh_in_recovery && blk_noretry_request(req)) {
> + spin_unlock(&dev->eh_lock);
> + nvme_req(req)->status |= NVME_SC_DNR;
> + return BLK_EH_HANDLED;
> + }
> + spin_unlock(&dev->eh_lock);

This doesn't really look safe. Even if a command times out, the
controller still owns that command, and calling it done while pci bus
master enable is still set can cause memory corruption.