Re: [PATCH v1 08/14] xprtrdma: Acquire MRs in rpcrdma_register_external()

[Sagi Grimberg <sagig@dev.mellanox.co.il>]

On 5/8/2015 6:40 PM, Chuck Lever wrote:

>>>
>>> Don't you need a call to flush_workqueue(frwr_recovery_wq) when you're
>>> about to destroy the endpoint (and the buffers and the MRs...)?
>>
>> I agree with Sagi here, in xprt_rdma_destroy() before calling
>> rpcrdma_destroy_buffer(), flush_workqueue and cancelling any pending
>> work seems required.
>
> The buffer list is destroyed only when all work has completed on the
> transport (no RPCs are outstanding, and the upper layer is shutting
> down). It’s pretty unlikely that there will be ongoing recovery work
> at this point.

It may be that there aren't any outstanding RPCs, but it is possible
that those that finished queued a frwr recovery work if the QP flushed
inflight frwr's.

>
> That said, would it be enough to add a defensive call to flush_workqueue()
> at the top of frwr_op_destroy() ?

If at this point you can guarantee that no one will queue another frwr
work (i.e. all flushe errors were consumed), then yes, I think
flush_workqueue() would do the job.

Sagi.