Re: [PATCH v2 for-next 1/2] RDMA/hns: Add the workqueue framework for flush cqe handler

["Liuyixian (Eason)" <liuyixian@huawei.com>]

On 2019/11/19 17:43, Zengtao (B) wrote:
>> -----Original Message-----
>> From: linux-rdma-owner@vger.kernel.org
>> [mailto:linux-rdma-owner@vger.kernel.org] On Behalf Of Liuyixian (Eason)
>> Sent: Tuesday, November 19, 2019 4:00 PM
>> To: Jason Gunthorpe
>> Cc: dledford@redhat.com; leon@kernel.org; linux-rdma@vger.kernel.org;
>> Linuxarm
>> Subject: Re: [PATCH v2 for-next 1/2] RDMA/hns: Add the workqueue
>> framework for flush cqe handler
>>
>>
>>
>> On 2019/11/19 1:02, Jason Gunthorpe wrote:
>>> On Mon, Nov 18, 2019 at 09:50:24PM +0800, Liuyixian (Eason) wrote:
>>>>> It kind of looks like this can be called multiple times? It won't work
>>>>> right unless it is called exactly once
>>>>>
>>>>> Jason
>>>>
>>>> Yes, you are right.
>>>>
>>>> So I think the reasonable solution is to allocate it dynamically, and I think
>>>> it is a very very little chance that the allocation will be failed. If this
>> happened,
>>>> I think the application also needs to be over.
>>>
>>> Why do you need more than one work in parallel for this? Once you
>>> start to move the HW to error that only has to happen once, surely?
>>>
>>> Jason
>>>
>> The flush operation moves QP, not the HW to error.
>>
>> For the QP, maybe the process A is posting send while the other
>> process B is modifying qp to error, both of these two operation
>> needs to initialize one flush work. That's why it could be called
>> multiple times.
>>
>> Furthermore, according to IB protocol 9.9.2.4.2, it may can't stop
>> posting wr into the qp even it already transitions to error state.
>> That's why it also needs to be called multiple times.
>>
>> Once the work is implemented successfully, we will free the work,
>> it is not a big problem to allocate it dynamically again and again.
>>
> 
> So can i understand that this function is designed to be reentrant?
> If so, I suggest to introduce a per dev/qp lock to protect.

Yes, currently we exactly use the following spinlock per qp to protect the verbs
which can be reentrant.

	spin_lock_irqsave(&qp->sq.lock, flags);

> 
>> Thanks.
>>
>>
>