* [PATCH] nvme-core: Share code between nvme_wait_ready() and nvme_shutdown_ctrl()
@ 2019-02-07 0:13 Andrey Smirnov
2019-03-08 13:23 ` Christoph Hellwig
0 siblings, 1 reply; 3+ messages in thread
From: Andrey Smirnov @ 2019-02-07 0:13 UTC (permalink / raw)
To: linux-nvme
Cc: Andrey Smirnov, Keith Busch, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, linux-kernel
Code polling NVME_CSTS_SHST_CMPLT in nvme_shutdown_ctrl() is very
similar to polling loop in nvme_wait_ready(). Move shared polling loop
code into __nvme_wait_ready() and re-implement both
nvme_shutdown_ctrl() and nvme_wait_ready() on top of it to avoid code
repetition.
Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Jens Axboe <axboe@fb.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Sagi Grimberg <sagi@grimberg.me>
Cc: linux-nvme@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/nvme/host/core.c | 54 ++++++++++++++++++++++------------------
1 file changed, 30 insertions(+), 24 deletions(-)
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index b47c59f0cbd8..633fbc21425c 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1797,28 +1797,41 @@ const struct block_device_operations nvme_ns_head_ops = {
};
#endif /* CONFIG_NVME_MULTIPATH */
-static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
+static int __nvme_wait_ready(struct nvme_ctrl *ctrl, u32 mask, u32 value,
+ unsigned long timeout)
{
- unsigned long timeout =
- ((NVME_CAP_TIMEOUT(cap) + 1) * HZ / 2) + jiffies;
- u32 csts, bit = enabled ? NVME_CSTS_RDY : 0;
+ u32 csts;
int ret;
while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
if (csts == ~0)
return -ENODEV;
- if ((csts & NVME_CSTS_RDY) == bit)
+ if ((csts & mask) == value)
break;
msleep(100);
if (fatal_signal_pending(current))
return -EINTR;
- if (time_after(jiffies, timeout)) {
- dev_err(ctrl->device,
- "Device not ready; aborting %s\n", enabled ?
- "initialisation" : "reset");
- return -ENODEV;
- }
+ if (time_after(jiffies, timeout))
+ return -ETIMEDOUT;
+ }
+
+ return ret;
+}
+
+static int nvme_wait_ready(struct nvme_ctrl *ctrl, u64 cap, bool enabled)
+{
+ unsigned long timeout =
+ ((NVME_CAP_TIMEOUT(cap) + 1) * HZ / 2) + jiffies;
+ u32 bit = enabled ? NVME_CSTS_RDY : 0;
+ int ret;
+
+ ret = __nvme_wait_ready(ctrl, NVME_CSTS_RDY, bit, timeout);
+ if (ret == -ETIMEDOUT) {
+ dev_err(ctrl->device,
+ "Device not ready; aborting %s\n", enabled ?
+ "initialisation" : "reset");
+ ret = -ENODEV;
}
return ret;
@@ -1883,7 +1896,6 @@ EXPORT_SYMBOL_GPL(nvme_enable_ctrl);
int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
{
unsigned long timeout = jiffies + (ctrl->shutdown_timeout * HZ);
- u32 csts;
int ret;
ctrl->ctrl_config &= ~NVME_CC_SHN_MASK;
@@ -1893,18 +1905,12 @@ int nvme_shutdown_ctrl(struct nvme_ctrl *ctrl)
if (ret)
return ret;
- while ((ret = ctrl->ops->reg_read32(ctrl, NVME_REG_CSTS, &csts)) == 0) {
- if ((csts & NVME_CSTS_SHST_MASK) == NVME_CSTS_SHST_CMPLT)
- break;
-
- msleep(100);
- if (fatal_signal_pending(current))
- return -EINTR;
- if (time_after(jiffies, timeout)) {
- dev_err(ctrl->device,
- "Device shutdown incomplete; abort shutdown\n");
- return -ENODEV;
- }
+ ret = __nvme_wait_ready(ctrl, NVME_CSTS_SHST_MASK,
+ NVME_CSTS_SHST_CMPLT, timeout);
+ if (ret == -ETIMEDOUT) {
+ dev_err(ctrl->device,
+ "Device shutdown incomplete; abort shutdown\n");
+ ret = -ENODEV;
}
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] nvme-core: Share code between nvme_wait_ready() and nvme_shutdown_ctrl()
2019-02-07 0:13 [PATCH] nvme-core: Share code between nvme_wait_ready() and nvme_shutdown_ctrl() Andrey Smirnov
@ 2019-03-08 13:23 ` Christoph Hellwig
2019-03-08 18:58 ` Andrey Smirnov
0 siblings, 1 reply; 3+ messages in thread
From: Christoph Hellwig @ 2019-03-08 13:23 UTC (permalink / raw)
To: Andrey Smirnov
Cc: linux-nvme, Keith Busch, Jens Axboe, Christoph Hellwig,
Sagi Grimberg, linux-kernel
On Wed, Feb 06, 2019 at 04:13:12PM -0800, Andrey Smirnov wrote:
> Code polling NVME_CSTS_SHST_CMPLT in nvme_shutdown_ctrl() is very
> similar to polling loop in nvme_wait_ready(). Move shared polling loop
> code into __nvme_wait_ready() and re-implement both
> nvme_shutdown_ctrl() and nvme_wait_ready() on top of it to avoid code
> repetition.
Is there any deeper reason why we would want this? It only saves
6 lines of code, but makes the functions much harder to read.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] nvme-core: Share code between nvme_wait_ready() and nvme_shutdown_ctrl()
2019-03-08 13:23 ` Christoph Hellwig
@ 2019-03-08 18:58 ` Andrey Smirnov
0 siblings, 0 replies; 3+ messages in thread
From: Andrey Smirnov @ 2019-03-08 18:58 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-nvme, Keith Busch, Jens Axboe, Sagi Grimberg, linux-kernel
On Fri, Mar 8, 2019 at 5:23 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Wed, Feb 06, 2019 at 04:13:12PM -0800, Andrey Smirnov wrote:
> > Code polling NVME_CSTS_SHST_CMPLT in nvme_shutdown_ctrl() is very
> > similar to polling loop in nvme_wait_ready(). Move shared polling loop
> > code into __nvme_wait_ready() and re-implement both
> > nvme_shutdown_ctrl() and nvme_wait_ready() on top of it to avoid code
> > repetition.
>
> Is there any deeper reason why we would want this? It only saves
> 6 lines of code, but makes the functions much harder to read.
No, there's no deeper reason.
Thanks,
Andrey Smirnov
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-08 18:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-07 0:13 [PATCH] nvme-core: Share code between nvme_wait_ready() and nvme_shutdown_ctrl() Andrey Smirnov
2019-03-08 13:23 ` Christoph Hellwig
2019-03-08 18:58 ` Andrey Smirnov
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).