From: Keith Busch <kbusch@kernel.org>
To: Nilay Shroff <nilay@linux.ibm.com>
Cc: linux-nvme@lists.infradead.org, hch@lst.de, sagi@grimberg.me,
axboe@fb.com, chaitanyak@nvidia.com, dlemoal@kernel.org,
gjoyce@linux.ibm.com
Subject: Re: [PATCH 3/3] nvme: use helper nvme_ctrl_state in nvme_keep_alive_end_io function
Date: Tue, 29 Oct 2024 09:01:51 -0600 [thread overview]
Message-ID: <ZyD433W-caGMfVsv@kbusch-mbp> (raw)
In-Reply-To: <20241027170209.440776-4-nilay@linux.ibm.com>
On Sun, Oct 27, 2024 at 10:32:06PM +0530, Nilay Shroff wrote:
> We no more need acquiring ctrl->lock before accessing the
> NVMe controller state and instead we can now use the helper
> nvme_ctrl_state. So replace the use of ctrl->lock from
> nvme_keep_alive_end_io function with nvme_ctrl_state call.
>
> Signed-off-by: Nilay Shroff <nilay@linux.ibm.com>
> ---
> drivers/nvme/host/core.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 865c00ea19e3..6e814daf1002 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -1296,10 +1296,9 @@ static enum rq_end_io_ret nvme_keep_alive_end_io(struct request *rq,
> blk_status_t status)
> {
> struct nvme_ctrl *ctrl = rq->end_io_data;
> - unsigned long flags;
> - bool startka = false;
> unsigned long rtt = jiffies - (rq->deadline - rq->timeout);
> unsigned long delay = nvme_keep_alive_work_period(ctrl);
> + enum nvme_ctrl_state state = nvme_ctrl_state(ctrl->state);
>
> /*
> * Subtract off the keepalive RTT so nvme_keep_alive_work runs
> @@ -1324,13 +1323,9 @@ static enum rq_end_io_ret nvme_keep_alive_end_io(struct request *rq,
>
> ctrl->ka_last_check_time = jiffies;
> ctrl->comp_seen = false;
> - spin_lock_irqsave(&ctrl->lock, flags);
> - if (ctrl->state == NVME_CTRL_LIVE ||
> - ctrl->state == NVME_CTRL_CONNECTING)
> - startka = true;
> - spin_unlock_irqrestore(&ctrl->lock, flags);
> - if (startka)
> + if (state == NVME_CTRL_LIVE || state == NVME_CTRL_CONNECTING)
> queue_delayed_work(nvme_wq, &ctrl->ka_work, delay);
> +
> return RQ_END_IO_NONE;
> }
This restores what you had in the reverted patch, which was a
significant portion of the diff stat. If we're keeping this chunk, maybe
a full revert isn't necessary, and we can squash the series to just one
fixing commit.
next prev parent reply other threads:[~2024-10-29 15:06 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-27 17:02 [PATCH 0/3] nvme: fix system fault observed while shutting down controller Nilay Shroff
2024-10-27 17:02 ` [PATCH 1/3] Revert "nvme: make keep-alive synchronous operation" Nilay Shroff
2024-10-27 22:05 ` Sagi Grimberg
2024-10-29 6:48 ` Ming Lei
2024-10-29 12:46 ` Nilay Shroff
2024-10-27 17:02 ` [PATCH 2/3] nvme-fabrics: fix kernel crash while shutting down controller Nilay Shroff
2024-10-27 22:05 ` Sagi Grimberg
2024-10-29 7:23 ` Ming Lei
2024-10-29 12:40 ` Nilay Shroff
2024-10-30 2:20 ` Ming Lei
2024-10-30 10:38 ` Nilay Shroff
2024-10-30 12:51 ` Ming Lei
2024-10-31 10:10 ` Nilay Shroff
2024-10-27 17:02 ` [PATCH 3/3] nvme: use helper nvme_ctrl_state in nvme_keep_alive_end_io function Nilay Shroff
2024-10-27 22:07 ` Sagi Grimberg
2024-10-28 4:43 ` Nilay Shroff
2024-10-29 14:58 ` Keith Busch
2024-10-29 15:01 ` Keith Busch [this message]
2024-10-30 6:07 ` Nilay Shroff
2024-11-01 19:13 ` Keith Busch
2024-11-03 17:54 ` Nilay Shroff
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=ZyD433W-caGMfVsv@kbusch-mbp \
--to=kbusch@kernel.org \
--cc=axboe@fb.com \
--cc=chaitanyak@nvidia.com \
--cc=dlemoal@kernel.org \
--cc=gjoyce@linux.ibm.com \
--cc=hch@lst.de \
--cc=linux-nvme@lists.infradead.org \
--cc=nilay@linux.ibm.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