* [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
@ 2018-08-07 20:42 ` Tal Shorer
0 siblings, 0 replies; 6+ messages in thread
From: Tal Shorer @ 2018-08-07 20:42 UTC (permalink / raw)
When the user supplics ctrl_loss_tmo < 0, we warn them that this will
cause the fabrics layer to attempt reconnection forever.
However, in reality the fabrics layer never attempts to reconnect
because the condition to test whether we should reconnect is backwards
in this case.
Signed-off-by: Tal Shorer <tal.shorer at gmail.com>
---
drivers/nvme/host/fabrics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index f7efe5a58cc7..206d63cb1afc 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -474,7 +474,7 @@ EXPORT_SYMBOL_GPL(nvmf_connect_io_queue);
bool nvmf_should_reconnect(struct nvme_ctrl *ctrl)
{
- if (ctrl->opts->max_reconnects != -1 &&
+ if (ctrl->opts->max_reconnects == -1 ||
ctrl->nr_reconnects < ctrl->opts->max_reconnects)
return true;
--
2.14.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
@ 2018-08-07 20:42 ` Tal Shorer
0 siblings, 0 replies; 6+ messages in thread
From: Tal Shorer @ 2018-08-07 20:42 UTC (permalink / raw)
To: keith.busch, axboe, hch, sagi, linux-nvme, linux-kernel; +Cc: Tal Shorer
When the user supplics ctrl_loss_tmo < 0, we warn them that this will
cause the fabrics layer to attempt reconnection forever.
However, in reality the fabrics layer never attempts to reconnect
because the condition to test whether we should reconnect is backwards
in this case.
Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
drivers/nvme/host/fabrics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index f7efe5a58cc7..206d63cb1afc 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -474,7 +474,7 @@ EXPORT_SYMBOL_GPL(nvmf_connect_io_queue);
bool nvmf_should_reconnect(struct nvme_ctrl *ctrl)
{
- if (ctrl->opts->max_reconnects != -1 &&
+ if (ctrl->opts->max_reconnects == -1 ||
ctrl->nr_reconnects < ctrl->opts->max_reconnects)
return true;
--
2.14.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
2018-08-07 20:42 ` Tal Shorer
@ 2018-08-07 22:49 ` Chaitanya Kulkarni
-1 siblings, 0 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2018-08-07 22:49 UTC (permalink / raw)
Looks good to me.
Just 's/supplics/supplies/'.
Without looking into the entire code, If we can change the type of max_reconnect and nr_reconnects to unsigned int then we can just get away with the one comparison (ctrl->nr_reconnects < ctrl->opts->max_reconnects).
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
From: Linux-nvme <linux-nvme-bounces@lists.infradead.org> on behalf of Tal Shorer <tal.shorer@gmail.com>
Sent: Tuesday, August 7, 2018 1:42 PM
To: keith.busch at intel.com; axboe at fb.com; hch at lst.de; sagi at grimberg.me; linux-nvme at lists.infradead.org; linux-kernel at vger.kernel.org
Cc: Tal Shorer
Subject: [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
?
When the user supplics ctrl_loss_tmo < 0, we warn them that this will
cause the fabrics layer to attempt reconnection forever.
However, in reality the fabrics layer never attempts to reconnect
because the condition to test whether we should reconnect is backwards
in this case.
Signed-off-by: Tal Shorer <tal.shorer at gmail.com>
---
?drivers/nvme/host/fabrics.c | 2 +-
?1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index f7efe5a58cc7..206d63cb1afc 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -474,7 +474,7 @@ EXPORT_SYMBOL_GPL(nvmf_connect_io_queue);
?
?bool nvmf_should_reconnect(struct nvme_ctrl *ctrl)
?{
-?????? if (ctrl->opts->max_reconnects != -1 &&
+?????? if (ctrl->opts->max_reconnects == -1 ||
???????????? ctrl->nr_reconnects < ctrl->opts->max_reconnects)
???????????????? return true;
?
--
2.14.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
@ 2018-08-07 22:49 ` Chaitanya Kulkarni
0 siblings, 0 replies; 6+ messages in thread
From: Chaitanya Kulkarni @ 2018-08-07 22:49 UTC (permalink / raw)
To: Tal Shorer, keith.busch@intel.com, axboe@fb.com, hch@lst.de,
sagi@grimberg.me, linux-nvme@lists.infradead.org,
linux-kernel@vger.kernel.org
Looks good to me.
Just 's/supplics/supplies/'.
Without looking into the entire code, If we can change the type of max_reconnect and nr_reconnects to unsigned int then we can just get away with the one comparison (ctrl->nr_reconnects < ctrl->opts->max_reconnects).
Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
From: Linux-nvme <linux-nvme-bounces@lists.infradead.org> on behalf of Tal Shorer <tal.shorer@gmail.com>
Sent: Tuesday, August 7, 2018 1:42 PM
To: keith.busch@intel.com; axboe@fb.com; hch@lst.de; sagi@grimberg.me; linux-nvme@lists.infradead.org; linux-kernel@vger.kernel.org
Cc: Tal Shorer
Subject: [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
When the user supplics ctrl_loss_tmo < 0, we warn them that this will
cause the fabrics layer to attempt reconnection forever.
However, in reality the fabrics layer never attempts to reconnect
because the condition to test whether we should reconnect is backwards
in this case.
Signed-off-by: Tal Shorer <tal.shorer@gmail.com>
---
drivers/nvme/host/fabrics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fabrics.c b/drivers/nvme/host/fabrics.c
index f7efe5a58cc7..206d63cb1afc 100644
--- a/drivers/nvme/host/fabrics.c
+++ b/drivers/nvme/host/fabrics.c
@@ -474,7 +474,7 @@ EXPORT_SYMBOL_GPL(nvmf_connect_io_queue);
bool nvmf_should_reconnect(struct nvme_ctrl *ctrl)
{
- if (ctrl->opts->max_reconnects != -1 &&
+ if (ctrl->opts->max_reconnects == -1 ||
ctrl->nr_reconnects < ctrl->opts->max_reconnects)
return true;
--
2.14.1
_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever
2018-08-07 20:42 ` Tal Shorer
@ 2018-08-08 10:08 ` Christoph Hellwig
-1 siblings, 0 replies; 6+ messages in thread
From: Christoph Hellwig @ 2018-08-08 10:08 UTC (permalink / raw)
Thanks, applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-08-08 10:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-07 20:42 [PATCH] nvme: fabrics: fix ctrl_loss_tmo < 0 to reconnect forever Tal Shorer
2018-08-07 20:42 ` Tal Shorer
2018-08-07 22:49 ` Chaitanya Kulkarni
2018-08-07 22:49 ` Chaitanya Kulkarni
2018-08-08 10:08 ` Christoph Hellwig
2018-08-08 10:08 ` Christoph Hellwig
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.