public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
From: Ming Lei <ming.lei@redhat.com>
To: Keith Busch <keith.busch@linux.intel.com>
Cc: Keith Busch <keith.busch@intel.com>, Jens Axboe <axboe@kernel.dk>,
	Laurence Oberman <loberman@redhat.com>,
	Sagi Grimberg <sagi@grimberg.me>,
	James Smart <james.smart@broadcom.com>,
	"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>,
	"linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
	Jianchao Wang <jianchao.w.wang@oracle.com>,
	Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH V6 11/11] nvme: pci: support nested EH
Date: Fri, 18 May 2018 08:20:05 +0800	[thread overview]
Message-ID: <20180518001958.GA2742@ming.t460p> (raw)
In-Reply-To: <20180517022030.GB21959@localhost.localdomain>

On Wed, May 16, 2018 at 08:20:31PM -0600, Keith Busch wrote:
> Hi Ming,
> 
> I'm developing the answers in code the issues you raised. It will just
> take a moment to complete flushing those out. In the meantime just want
> to point out why I think block/011 isn't a real test.
> 
> On Thu, May 17, 2018 at 07:10:59AM +0800, Ming Lei wrote:
> > All simulation in block/011 may happen in reality.
> 
> If this test actually simulates reality, then the following one line
> patch (plus explanation for why) would be a real "fix" as this is very
> successful in passing block/011. :)
> 
> ---
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 1faa32cd07da..dcc5746304c4 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -2118,6 +2118,12 @@ static int nvme_pci_enable(struct nvme_dev *dev)
>  
>  	if (pci_enable_device_mem(pdev))
>  		return result;
> +	/*
> +	 * blktests block/011 disables the device without the driver knowing.
> +	 * We'll just enable the device twice to get the enable_cnt > 1
> +	 * so that the test's disabling does absolutely nothing.
> +	 */
> +	pci_enable_device_mem(pdev);

What I think block/011 is helpful is that it can trigger IO timeout
during reset, which can be triggered in reality too.

That is one big problem of NVMe driver, IMO.

And this patch does fix this issue, together other timeout related
races.


Thanks,
Ming

  parent reply	other threads:[~2018-05-18  0:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-16  4:03 [PATCH V6 00/11] nvme: pci: fix & improve timeout handling Ming Lei
2018-05-16  4:03 ` [PATCH V6 01/11] block: introduce blk_quiesce_timeout() and blk_unquiesce_timeout() Ming Lei
2018-05-16  4:03 ` [PATCH V6 02/11] nvme: pci: cover timeout for admin commands running in EH Ming Lei
2018-05-24 15:39   ` Keith Busch
2018-05-16  4:03 ` [PATCH V6 03/11] nvme: pci: unquiesce admin queue after controller is shutdown Ming Lei
2018-05-16  4:03 ` [PATCH V6 04/11] nvme: pci: set nvmeq->cq_vector after alloc cq/sq Ming Lei
2018-05-16  4:03 ` [PATCH V6 05/11] nvme: pci: only wait freezing if queue is frozen Ming Lei
2018-05-16  4:03 ` [PATCH V6 06/11] nvme: pci: freeze queue in nvme_dev_disable() in case of error recovery Ming Lei
2018-05-16  4:03 ` [PATCH V6 07/11] nvme: pci: prepare for supporting error recovery from resetting context Ming Lei
2018-05-16  4:03 ` [PATCH V6 08/11] nvme: pci: move error handling out of nvme_reset_dev() Ming Lei
2018-05-16  4:03 ` [PATCH V6 09/11] nvme: pci: don't unfreeze queue until controller state updating succeeds Ming Lei
2018-05-16  4:03 ` [PATCH V6 10/11] nvme: core: introduce nvme_force_change_ctrl_state() Ming Lei
2018-05-16  4:03 ` [PATCH V6 11/11] nvme: pci: support nested EH Ming Lei
2018-05-16 14:12   ` Keith Busch
2018-05-16 23:10     ` Ming Lei
2018-05-17  2:20       ` Keith Busch
2018-05-17  8:41         ` Christoph Hellwig
2018-05-17 14:20           ` Keith Busch
2018-05-17 14:23             ` Johannes Thumshirn
2018-05-18 16:28               ` Keith Busch
2018-05-22  7:35                 ` Johannes Thumshirn
2018-05-18  0:20         ` Ming Lei [this message]
2018-05-18  1:01           ` Ming Lei
2018-05-18 13:57           ` Keith Busch
2018-05-18 16:58             ` Jens Axboe
2018-05-18 22:26             ` Ming Lei
2018-05-18 23:45               ` Keith Busch
2018-05-18 23:51                 ` Ming Lei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180518001958.GA2742@ming.t460p \
    --to=ming.lei@redhat.com \
    --cc=axboe@kernel.dk \
    --cc=hch@lst.de \
    --cc=james.smart@broadcom.com \
    --cc=jianchao.w.wang@oracle.com \
    --cc=keith.busch@intel.com \
    --cc=keith.busch@linux.intel.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=loberman@redhat.com \
    --cc=sagi@grimberg.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox