nvmet: Don't queue fatal error work if csts.cfs is set
Subsystems (2)
Sagi Grimberg
maintainerPATCH 1/1 nvmet: Don't queue fatal error work if csts.cfs is set
In the transport, in case of an interal queue error like error completion in rdma we trigger a fatal error. However, multiple queues in the same controller can serr error completions and we don't want to trigger fatal error work more than once.
--- drivers/nvme/target/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/nvme/target/core.c +3 -1
@@ -840,7 +840,9 @@ static void nvmet_fatal_error_handler(struct work_struct *work)void nvmet_ctrl_fatal_error (struct nvmet_ctrl *ctrl) { - ctrl->csts |=NVME_CSTS_CFS ; +if (test_and_set_bit (NVME_CSTS_CFS , (unsigned long *)&ctrl->csts)) +return ; +INIT_WORK (&ctrl->fatal_err_work, nvmet_fatal_error_handler);schedule_work (&ctrl->fatal_err_work); }
Signature · 1 line
2.7.4
Christoph Hellwig
maintainerPATCH 1/1 nvmet: Don't queue fatal error work if csts.cfs is set
drivers/nvme/target/core.c static void nvmet_fatal_error_handler(struct work_struct *work) · line 845
Quoted 5 lines
{
drivers/nvme/target/core.c +3
-1- ctrl->csts |=NVME_CSTS_CFS ; +if (test_and_set_bit (NVME_CSTS_CFS , (unsigned long *)&ctrl->csts)) +return ; +
This can't work - test_and_set_bit takes a bit index and NVME_CSTS_CFS is the actual value. I think we'll need a lock to protect ->csts instead.
Sagi Grimberg
maintainerPATCH 1/1 nvmet: Don't queue fatal error work if csts.cfs is set
Quoted 10 lines
drivers/nvme/target/core.c static void nvmet_fatal_error_handler(struct work_struct *work) · line 845
Quoted 6 lines
{
drivers/nvme/target/core.c +3
-1- ctrl->csts |=NVME_CSTS_CFS ; +if (test_and_set_bit (NVME_CSTS_CFS , (unsigned long *)&ctrl->csts)) +return ; +This can't work -
test_and_set_bittakes a bit index andNVME_CSTS_CFSis the actual value. I think we'll need a lock to protect ->csts instead.
Oops... it still prevents from fatal_err work from requeueing so at least I got 50% :)
I'll send a proper patch with a lock in place...
PATCH 1/1 nvmet: Don't queue fatal error work if csts.cfs is set
Quoted 10 lines
Quoted 9 lines
drivers/nvme/target/core.c static void nvmet_fatal_error_handler(struct work_struct *work) · line 845
Quoted 5 lines
{
drivers/nvme/target/core.c +3
-1- ctrl->csts |=NVME_CSTS_CFS ; +if (test_and_set_bit (NVME_CSTS_CFS , (unsigned long *)&ctrl->csts)) +return ; +This can't work -
test_and_set_bittakes a bit index andNVME_CSTS_CFSis the actual value. I think we'll need a lock to protect ->csts instead.
Quoted 5 lines
Oops... it still prevents from
fatal_errwork from requeueing so at least I got 50% :)I'll send a proper patch with a lock in place...
I wonder if this is causing my most recent target crashes during kato reconnect/recovery testing?
PATCH 1/1 nvmet: Don't queue fatal error work if csts.cfs is set
In reply to Christoph HellwigQuoted 12 lines · Christoph Hellwig
drivers/nvme/target/core.c static void nvmet_fatal_error_handler(struct work_struct *work) · line 845
Quoted 7 lines
?{
(quoted diff) +1
-1- ctrl->csts |= NVME_CSTS_CFS; + if (test_and_set_bit(NVME_CSTS_CFS, (unsigned long*)&ctrl->csts))
drivers/nvme/target/core.c +2
-0+return ; +This can't work -
test_and_set_bittakes a bit index andNVME_CSTS_CFS
is the actual value.??I think we'll need a lock to protect ->csts instead.
Yeah, I agree, it feels more like a lock solution.
Quoted 6 lines
_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme