From: "jianchao.wang" <jianchao.w.wang@oracle.com> To: Keith Busch <keith.busch@linux.intel.com>, Ming Lei <ming.lei@redhat.com> Cc: Keith Busch <keith.busch@intel.com>, Jens Axboe <axboe@kernel.dk>, Sagi Grimberg <sagi@grimberg.me>, linux-nvme@lists.infradead.org, linux-block@vger.kernel.org, Christoph Hellwig <hch@lst.de> Subject: Re: [PATCH 1/2] nvme: pci: simplify timeout handling Date: Sat, 28 Apr 2018 22:31:26 +0800 [thread overview] Message-ID: <401cba4e-4a70-291d-54f0-5cb484006587@oracle.com> (raw) In-Reply-To: <20180428133514.GB5938@localhost.localdomain> Hi Ming and Keith Let me detail extend more here. :) On 04/28/2018 09:35 PM, Keith Busch wrote: >> Actually there isn't the case before, even for legacy path, one .timeout() >> handles one request only. Yes, .timeout should be invoked for every timeout request and .timeout should also handle this only one request in principle however, nvme_timeout will invoke nvme_dev_disable > That's not quite what I was talking about. > > Before, only the command that was about to be sent to the driver's > .timeout() was marked completed. The driver could (and did) compete > other timed out commands in a single .timeout(), and the tag would > clear, so we could hanlde all timeouts in a single .timeout(). I think Keith are saying that before this new blk-mq timeout implementation, the logic of blk_mq_timeout_work is get _only_ _one_ timeout request mark completed invoke .timeout, in nvme, it is nvme_timeout then nvme_dev_disable is invoked and thus other requests could be completed by blk_mq_complete_request because they have not been mark completed > > Now, blk-mq marks all timed out commands as aborted prior to calling > the driver's .timeout(). If the driver completes any of those commands, > the tag does not clear, so the driver's .timeout() just gets to be called > again for commands it already reaped. > After the new blk-mq timeout implementation, set the aborted_gstate of _all_ the timeout requests invoke the .timeout one by one for the first timeout request's .timeout, in nvme, it is nvme_timeout nvme_dev_disable is invoked and try to complete all the in-flight requests through blk_mq_complete_request but timeout requests that have been set aborted_gstate cannot handled by blk_mq_complete_request so some requests are leaked by nvme_dev_disable these residual timeout requests will still be handled by blk_mq_timeout_work through invoke .timeout one by one Thanks Jianchao >
WARNING: multiple messages have this Message-ID (diff)
From: jianchao.w.wang@oracle.com (jianchao.wang) Subject: [PATCH 1/2] nvme: pci: simplify timeout handling Date: Sat, 28 Apr 2018 22:31:26 +0800 [thread overview] Message-ID: <401cba4e-4a70-291d-54f0-5cb484006587@oracle.com> (raw) In-Reply-To: <20180428133514.GB5938@localhost.localdomain> Hi Ming and Keith Let me detail extend more here. :) On 04/28/2018 09:35 PM, Keith Busch wrote: >> Actually there isn't the case before, even for legacy path, one .timeout() >> handles one request only. Yes, .timeout should be invoked for every timeout request and .timeout should also handle this only one request in principle however, nvme_timeout will invoke nvme_dev_disable > That's not quite what I was talking about. > > Before, only the command that was about to be sent to the driver's > .timeout() was marked completed. The driver could (and did) compete > other timed out commands in a single .timeout(), and the tag would > clear, so we could hanlde all timeouts in a single .timeout(). I think Keith are saying that before this new blk-mq timeout implementation, the logic of blk_mq_timeout_work is get _only_ _one_ timeout request mark completed invoke .timeout, in nvme, it is nvme_timeout then nvme_dev_disable is invoked and thus other requests could be completed by blk_mq_complete_request because they have not been mark completed > > Now, blk-mq marks all timed out commands as aborted prior to calling > the driver's .timeout(). If the driver completes any of those commands, > the tag does not clear, so the driver's .timeout() just gets to be called > again for commands it already reaped. > After the new blk-mq timeout implementation, set the aborted_gstate of _all_ the timeout requests invoke the .timeout one by one for the first timeout request's .timeout, in nvme, it is nvme_timeout nvme_dev_disable is invoked and try to complete all the in-flight requests through blk_mq_complete_request but timeout requests that have been set aborted_gstate cannot handled by blk_mq_complete_request so some requests are leaked by nvme_dev_disable these residual timeout requests will still be handled by blk_mq_timeout_work through invoke .timeout one by one Thanks Jianchao >
next prev parent reply other threads:[~2018-04-28 14:31 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-04-26 12:39 [PATCH 0/2] nvme: pci: fix & improve timeout handling Ming Lei 2018-04-26 12:39 ` Ming Lei 2018-04-26 12:39 ` [PATCH 1/2] nvme: pci: simplify " Ming Lei 2018-04-26 12:39 ` Ming Lei 2018-04-26 15:07 ` jianchao.wang 2018-04-26 15:07 ` jianchao.wang 2018-04-26 15:57 ` Ming Lei 2018-04-26 15:57 ` Ming Lei 2018-04-26 16:16 ` Ming Lei 2018-04-26 16:16 ` Ming Lei 2018-04-27 1:37 ` jianchao.wang 2018-04-27 1:37 ` jianchao.wang 2018-04-27 14:57 ` Ming Lei 2018-04-27 14:57 ` Ming Lei 2018-04-28 14:00 ` jianchao.wang 2018-04-28 14:00 ` jianchao.wang 2018-04-28 21:57 ` Ming Lei 2018-04-28 21:57 ` Ming Lei 2018-04-28 22:27 ` Ming Lei 2018-04-28 22:27 ` Ming Lei 2018-04-29 1:36 ` Ming Lei 2018-04-29 1:36 ` Ming Lei 2018-04-29 2:21 ` jianchao.wang 2018-04-29 2:21 ` jianchao.wang 2018-04-29 14:13 ` Ming Lei 2018-04-29 14:13 ` Ming Lei 2018-04-27 17:51 ` Keith Busch 2018-04-27 17:51 ` Keith Busch 2018-04-28 3:50 ` Ming Lei 2018-04-28 3:50 ` Ming Lei 2018-04-28 13:35 ` Keith Busch 2018-04-28 13:35 ` Keith Busch 2018-04-28 14:31 ` jianchao.wang [this message] 2018-04-28 14:31 ` jianchao.wang 2018-04-28 21:39 ` Ming Lei 2018-04-28 21:39 ` Ming Lei 2018-04-30 19:52 ` Keith Busch 2018-04-30 19:52 ` Keith Busch 2018-04-30 23:14 ` Ming Lei 2018-04-30 23:14 ` Ming Lei 2018-05-08 15:30 ` Keith Busch 2018-05-08 15:30 ` Keith Busch 2018-05-10 20:52 ` Ming Lei 2018-05-10 20:52 ` Ming Lei 2018-05-10 21:05 ` Keith Busch 2018-05-10 21:05 ` Keith Busch 2018-05-10 21:10 ` Ming Lei 2018-05-10 21:10 ` Ming Lei 2018-05-10 21:18 ` Keith Busch 2018-05-10 21:18 ` Keith Busch 2018-05-10 21:24 ` Ming Lei 2018-05-10 21:24 ` Ming Lei 2018-05-10 21:44 ` Keith Busch 2018-05-10 21:44 ` Keith Busch 2018-05-10 21:50 ` Ming Lei 2018-05-10 21:50 ` Ming Lei 2018-05-10 21:53 ` Ming Lei 2018-05-10 21:53 ` Ming Lei 2018-05-10 22:03 ` Ming Lei 2018-05-10 22:03 ` Ming Lei 2018-05-10 22:43 ` Keith Busch 2018-05-10 22:43 ` Keith Busch 2018-05-11 0:14 ` Ming Lei 2018-05-11 0:14 ` Ming Lei 2018-05-11 2:10 ` Ming Lei 2018-05-11 2:10 ` Ming Lei 2018-04-26 12:39 ` [PATCH 2/2] nvme: pci: guarantee EH can make progress Ming Lei 2018-04-26 12:39 ` Ming Lei 2018-04-26 16:24 ` Keith Busch 2018-04-26 16:24 ` Keith Busch 2018-04-28 3:28 ` Ming Lei 2018-04-28 3:28 ` 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=401cba4e-4a70-291d-54f0-5cb484006587@oracle.com \ --to=jianchao.w.wang@oracle.com \ --cc=axboe@kernel.dk \ --cc=hch@lst.de \ --cc=keith.busch@intel.com \ --cc=keith.busch@linux.intel.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-nvme@lists.infradead.org \ --cc=ming.lei@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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.