From: Ming Lei <ming.lei@redhat.com>
To: Jens Axboe <axboe@kernel.dk>, Keith Busch <keith.busch@intel.com>,
Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>
Cc: Zhang Yi <yizhan@redhat.com>,
stable@vger.kernel.org, linux-block@vger.kernel.org,
linux-nvme@lists.infradead.org,
Johannes Thumshirn <jthumshirn@suse.de>
Subject: Re: [PATCH v2 2/3] nvme: avoid to use blk_mq_abort_requeue_list()
Date: Sat, 20 May 2017 13:34:15 +0800 [thread overview]
Message-ID: <20170520053409.GA22317@ming.t460p> (raw)
In-Reply-To: <20170520035605.21785-3-ming.lei@redhat.com>
On Sat, May 20, 2017 at 11:56:04AM +0800, Ming Lei wrote:
> NVMe may add request into requeue list simply and not kick off the
> requeue if hw queues are stopped. Then blk_mq_abort_requeue_list()
> is called in both nvme_kill_queues() and nvme_ns_remove() for
> dealing with this issue.
>
> Unfortunately blk_mq_abort_requeue_list() is absolutely a
> race maker, for example, one request may be requeued during
> the aborting. So this patch just calls blk_mq_kick_requeue_list() in
> nvme_kill_queues() to handle this issue like what nvme_start_queues()
> does. Now all requests in requeue list when queues are stopped will be
> handled by blk_mq_kick_requeue_list() when queues are restarted, either
> in nvme_start_queues() or in nvme_kill_queues().
>
> Cc: stable@vger.kernel.org
> Reported-by: Zhang Yi <yizhan@redhat.com>
> Signed-off-by: Ming Lei <ming.lei@redhat.com>
> ---
> drivers/nvme/host/core.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index ce0d96913ee6..9ce0a0a16984 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2098,7 +2098,6 @@ static void nvme_ns_remove(struct nvme_ns *ns)
> if (ns->ndev)
> nvme_nvm_unregister_sysfs(ns);
> del_gendisk(ns->disk);
> - blk_mq_abort_requeue_list(ns->queue);
> blk_cleanup_queue(ns->queue);
> }
>
> @@ -2436,7 +2435,6 @@ void nvme_kill_queues(struct nvme_ctrl *ctrl)
> continue;
> revalidate_disk(ns->disk);
> blk_set_queue_dying(ns->queue);
> - blk_mq_abort_requeue_list(ns->queue);
>
> /*
> * We have to force to start queues for avoiding hang
> @@ -2444,6 +2442,9 @@ void nvme_kill_queues(struct nvme_ctrl *ctrl)
> * be stopped forever from now on.
> */
> blk_mq_start_hw_queues(ns->queue);
> +
> + /* draining requests in requeue list */
> + blk_mq_kick_requeue_list(q);
oops, the above line causes build failure, please
take the following one.
---
>From e70cfe2edacf278e3e6605f8c08e01ebf65bff01 Mon Sep 17 00:00:00 2001
From: Ming Lei <ming.lei@redhat.com>
Date: Wed, 17 May 2017 09:02:07 +0800
Subject: [PATCH v2 2/3] nvme: avoid to use blk_mq_abort_requeue_list()
NVMe may add request into requeue list simply and not kick off the
requeue if hw queues are stopped. Then blk_mq_abort_requeue_list()
is called in both nvme_kill_queues() and nvme_ns_remove() for
dealing with this issue.
Unfortunately blk_mq_abort_requeue_list() is absolutely a
race maker, for example, one request may be requeued during
the aborting. So this patch just calls blk_mq_kick_requeue_list() in
nvme_kill_queues() to handle this issue like what nvme_start_queues()
does. Now all requests in requeue list when queues are stopped will be
handled by blk_mq_kick_requeue_list() when queues are restarted, either
in nvme_start_queues() or in nvme_kill_queues().
Cc: stable@vger.kernel.org
Reported-by: Zhang Yi <yizhan@redhat.com>
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
drivers/nvme/host/core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index ce0d96913ee6..e344f5d7c1bc 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -2098,7 +2098,6 @@ static void nvme_ns_remove(struct nvme_ns *ns)
if (ns->ndev)
nvme_nvm_unregister_sysfs(ns);
del_gendisk(ns->disk);
- blk_mq_abort_requeue_list(ns->queue);
blk_cleanup_queue(ns->queue);
}
@@ -2436,7 +2435,6 @@ void nvme_kill_queues(struct nvme_ctrl *ctrl)
continue;
revalidate_disk(ns->disk);
blk_set_queue_dying(ns->queue);
- blk_mq_abort_requeue_list(ns->queue);
/*
* We have to force to start queues for avoiding hang
@@ -2444,6 +2442,9 @@ void nvme_kill_queues(struct nvme_ctrl *ctrl)
* be stopped forever from now on.
*/
blk_mq_start_hw_queues(ns->queue);
+
+ /* draining requests in requeue list */
+ blk_mq_kick_requeue_list(ns->queue);
}
mutex_unlock(&ctrl->namespaces_mutex);
}
--
2.9.4
next prev parent reply other threads:[~2017-05-20 5:34 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20170520035605.21785-1-ming.lei@redhat.com>
2017-05-20 3:56 ` [PATCH v2 1/3] nvme: use blk_mq_start_hw_queues() in nvme_kill_queues() Ming Lei
2017-05-21 6:20 ` Christoph Hellwig
2017-05-22 1:35 ` Ming Lei
2017-05-22 12:51 ` Christoph Hellwig
2017-05-22 14:49 ` Ming Lei
2017-05-22 5:35 ` Keith Busch
2017-05-22 7:41 ` Johannes Thumshirn
2017-05-20 3:56 ` [PATCH v2 2/3] nvme: avoid to use blk_mq_abort_requeue_list() Ming Lei
2017-05-20 5:34 ` Ming Lei [this message]
2017-05-21 6:22 ` Christoph Hellwig
2017-05-22 7:42 ` Johannes Thumshirn
2017-05-22 5:35 ` Keith Busch
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=20170520053409.GA22317@ming.t460p \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=jthumshirn@suse.de \
--cc=keith.busch@intel.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
--cc=stable@vger.kernel.org \
--cc=yizhan@redhat.com \
/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;
as well as URLs for NNTP newsgroup(s).