* [PATCH] nvme_fc: avoid workqueue flush stalls
@ 2017-09-28 16:56 James Smart
2017-09-30 17:18 ` Sagi Grimberg
0 siblings, 1 reply; 4+ messages in thread
From: James Smart @ 2017-09-28 16:56 UTC (permalink / raw)
There's no need to wait for the full nvme_wq, which is now shared,
to flush. flush only the delete_work item.
Signed-off-by: James Smart <james.smart at broadcom.com>
---
drivers/nvme/host/fc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index ac898571b1e5..2bd94de09b8a 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -2846,7 +2846,7 @@ nvme_fc_del_nvme_ctrl(struct nvme_ctrl *nctrl)
ret = __nvme_fc_del_ctrl(ctrl);
if (!ret)
- flush_workqueue(nvme_wq);
+ flush_work(&ctrl->delete_work);
nvme_put_ctrl(&ctrl->ctrl);
--
2.13.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] nvme_fc: avoid workqueue flush stalls
2017-09-28 16:56 [PATCH] nvme_fc: avoid workqueue flush stalls James Smart
@ 2017-09-30 17:18 ` Sagi Grimberg
2017-10-19 20:24 ` James Smart
0 siblings, 1 reply; 4+ messages in thread
From: Sagi Grimberg @ 2017-09-30 17:18 UTC (permalink / raw)
> There's no need to wait for the full nvme_wq, which is now shared,
> to flush. flush only the delete_work item.
The patch looks correct, but can you explain what stalled that this
patch fixes?
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] nvme_fc: avoid workqueue flush stalls
2017-09-30 17:18 ` Sagi Grimberg
@ 2017-10-19 20:24 ` James Smart
2017-10-22 10:05 ` Sagi Grimberg
0 siblings, 1 reply; 4+ messages in thread
From: James Smart @ 2017-10-19 20:24 UTC (permalink / raw)
On 9/30/2017 10:18 AM, Sagi Grimberg wrote:
>
>> There's no need to wait for the full nvme_wq, which is now shared,
>> to flush. flush only the delete_work item.
>
> The patch looks correct, but can you explain what stalled that this
> patch fixes?
I don't have an explicit failure case. While reviewing the code for the
other stall issue with fatal_err_work, I realized that this flush is now
waiting on a work queue shared by everything nvmf.? It used to be solely
a nvme-fc transport workqueue. There's no need for waiting on anything
more than the delete work item. And having it raise in scope of the nvmf
thread perhaps makes it wait for a lot of other unnecessary things -
slowing it down.
If you're ok with it - can you Ack/Review it ?
-- james
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] nvme_fc: avoid workqueue flush stalls
2017-10-19 20:24 ` James Smart
@ 2017-10-22 10:05 ` Sagi Grimberg
0 siblings, 0 replies; 4+ messages in thread
From: Sagi Grimberg @ 2017-10-22 10:05 UTC (permalink / raw)
>>> There's no need to wait for the full nvme_wq, which is now shared,
>>> to flush. flush only the delete_work item.
>>
>> The patch looks correct, but can you explain what stalled that this
>> patch fixes?
>
> I don't have an explicit failure case. While reviewing the code for the
> other stall issue with fatal_err_work, I realized that this flush is now
> waiting on a work queue shared by everything nvmf.? It used to be solely
> a nvme-fc transport workqueue. There's no need for waiting on anything
> more than the delete work item. And having it raise in scope of the nvmf
> thread perhaps makes it wait for a lot of other unnecessary things -
> slowing it down.
>
> If you're ok with it - can you Ack/Review it ?
Reviewed-by: Sagi Grimberg <sagi at grimberg.me>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-10-22 10:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-28 16:56 [PATCH] nvme_fc: avoid workqueue flush stalls James Smart
2017-09-30 17:18 ` Sagi Grimberg
2017-10-19 20:24 ` James Smart
2017-10-22 10:05 ` Sagi Grimberg
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).