From: Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>
To: Hannes Reinecke <hare@suse.de>,
"linux-nvme@lists.infradead.org" <linux-nvme@lists.infradead.org>
Cc: "hch@lst.de" <hch@lst.de>,
"Victor.Gladkov@kioxia.com" <Victor.Gladkov@kioxia.com>,
"sagi@grimberg.me" <sagi@grimberg.me>,
"snitzer@redhat.com" <snitzer@redhat.com>,
"james.smart@broadcom.com" <james.smart@broadcom.com>
Subject: Re: [PATCH V4] nvme-fabrics: reject I/O to offline device
Date: Tue, 10 Mar 2020 00:09:09 +0000 [thread overview]
Message-ID: <BYAPR04MB5749681A4F3255DDEE989A6086FF0@BYAPR04MB5749.namprd04.prod.outlook.com> (raw)
In-Reply-To: 64155ff6-f4b9-27d1-9dcb-97afddb5840f@suse.de
On 03/07/2020 04:15 AM, Hannes Reinecke wrote:
>> int nvme_reset_ctrl(struct nvme_ctrl *ctrl)
>> > {
>> > if (!nvme_change_ctrl_state(ctrl, NVME_CTRL_RESETTING))
>> >@@ -395,8 +428,21 @@ bool nvme_change_ctrl_state(struct nvme_ctrl *ctrl,
>> > }
>> >
>> > spin_unlock_irqrestore(&ctrl->lock, flags);
>> >- if (changed && ctrl->state == NVME_CTRL_LIVE)
>> >- nvme_kick_requeue_lists(ctrl);
>> >+ if (changed) {
>> >+ switch (ctrl->state) {
>> >+ case NVME_CTRL_LIVE:
>> >+ nvme_kick_requeue_lists(ctrl);
>> >+ if (old_state == NVME_CTRL_CONNECTING)
>> >+ nvme_stop_failfast_work(ctrl);
>> >+ break;
> This is a bit awkward; nvme_stop_failfast_work() will call
> cancel_delayed_work_sync(), which in turn might need to wait for an
> already running workqueue function, which already has the ctrl lock.
> Deadlock.
> So you need to call 'nvme_stop_failfast_work()' without the ctrl lock held.
>
There are two calls for the nvme_stop_failfast_work() in host/core.c :-
1. nvme_change_ctrl_state() :-
Here we unlock the ctrl->lock, then call nvme_stop_failfast_work().
2. nvme_stop_ctrl() :-
This is not taking ctrl->lock.
I'll read some more code, but please correct me if I did not
understand the scenario that you have mentioned.
> Other than that I'm very much in favour of this approach.
>
> Cheers,
>
> Hannes
_______________________________________________
linux-nvme mailing list
linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
next prev parent reply other threads:[~2020-03-10 0:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-02 3:30 [PATCH V4] nvme-fabrics: reject I/O to offline device Chaitanya Kulkarni
2020-03-07 12:15 ` Hannes Reinecke
2020-03-10 0:09 ` Chaitanya Kulkarni [this message]
2020-03-31 18:21 ` Alex Lyakas
2020-04-02 4:48 ` Chaitanya Kulkarni
2020-04-02 17:00 ` Alex Lyakas
[not found] ` <bb9948035bea461a8864a8cc88513e1b@kioxia.com>
2020-06-29 15:09 ` Victor Gladkov
2020-06-29 19:16 ` Sagi Grimberg
2020-06-30 13:20 ` Victor Gladkov
2020-07-02 14:35 ` Hannes Reinecke
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=BYAPR04MB5749681A4F3255DDEE989A6086FF0@BYAPR04MB5749.namprd04.prod.outlook.com \
--to=chaitanya.kulkarni@wdc.com \
--cc=Victor.Gladkov@kioxia.com \
--cc=hare@suse.de \
--cc=hch@lst.de \
--cc=james.smart@broadcom.com \
--cc=linux-nvme@lists.infradead.org \
--cc=sagi@grimberg.me \
--cc=snitzer@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).