nvme-fc: Question about __nvme_fc_abort_outstanding_ios()

nvme-fc: Question about __nvme_fc_abort_outstanding_ios()

[Mohamed Khalfella]

Hello,

I was looking at this code and I had a question about it.

drivers/nvme/host/fc.c
2473 static void
2474 __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
2475 {
...
...
2503                 blk_mq_tagset_busy_iter(&ctrl->tag_set,
2504                                 nvme_fc_terminate_exchange, &ctrl->ctrl);
2505                 blk_mq_tagset_wait_completed_request(&ctrl->tag_set);

nvme_fc_terminate_exchange() calls __nvme_fc_abort_op() to abort all active
ops with status FCPOP_STATE_ACTIVE. I think these active ops map to in-flight
requests MQ_RQ_IN_FLIGHT. After blk_mq_tagset_busy_iter() returns it is not
guaranteed that all ops had done callback functions called on them. Some
of these requests might still be in-flight.

blk_mq_tagset_wait_completed_request() makes sure that we do not have pending
completed requests, but it does not check for in-flight requests?

Am I missing something obvious here?

Why we need blk_mq_tagset_wait_completed_request() here?
Is it possible to have in-flight requests after the function returns?

Should this call be moved to nvme_fc_delete_association() after all the
requests are aborted by LLDD?

Thanks,
Mohamed Khalfella

Re: nvme-fc: Question about __nvme_fc_abort_outstanding_ios()

[Ming Lei]

On Wed, Jan 08, 2025 at 05:32:10PM -0800, Mohamed Khalfella wrote:
> Hello,
> 
> I was looking at this code and I had a question about it.
> 
> drivers/nvme/host/fc.c
> 2473 static void
> 2474 __nvme_fc_abort_outstanding_ios(struct nvme_fc_ctrl *ctrl, bool start_queues)
> 2475 {
> ...
> ...
> 2503                 blk_mq_tagset_busy_iter(&ctrl->tag_set,
> 2504                                 nvme_fc_terminate_exchange, &ctrl->ctrl);
> 2505                 blk_mq_tagset_wait_completed_request(&ctrl->tag_set);
> 
> nvme_fc_terminate_exchange() calls __nvme_fc_abort_op() to abort all active
> ops with status FCPOP_STATE_ACTIVE. I think these active ops map to in-flight
> requests MQ_RQ_IN_FLIGHT. After blk_mq_tagset_busy_iter() returns it is not
> guaranteed that all ops had done callback functions called on them. Some
> of these requests might still be in-flight.
> 
> blk_mq_tagset_wait_completed_request() makes sure that we do not have pending
> completed requests, but it does not check for in-flight requests?

It is because many callbacks call blk_mq_complete_request() to complete
request, and the request may be completed remotely via IPI, that is why
blk_mq_tagset_wait_completed_request() is needed.


Thanks, 
Ming

Re: nvme-fc: Question about __nvme_fc_abort_outstanding_ios()

[Mohamed Khalfella]

On 2025-01-09 09:59:03 +0800, Ming Lei wrote:
> 
> It is because many callbacks call blk_mq_complete_request() to complete
> request, and the request may be completed remotely via IPI, that is why
> blk_mq_tagset_wait_completed_request() is needed.
> 

Thanks for the response. I know that requests can be completed
asynchronously via IPI or softirq. This does not fully answer my
question about in-flight requests though.