On 2019/12/18 22:00, Jason Gunthorpe wrote:
> On Mon, Dec 16, 2019 at 08:51:03PM +0800, Liuyixian (Eason) wrote:
>> Hi Jason,
>>
>> I want to make sure that is there any further comments on this patch set?
>
> I still dislike it alot.
>
Hi Jason,
Thanks for reply :)
Let us recall the previous discussions and check is there anything is pending on.
Comments in patch set V1:
1: why do you need a dedicated HIGHPRI work queue?
-- Accepted. The HIGHPRI flag was moved out from v2.
2. As far as I could tell the only thing the triggered the work to run
was some variable which was only set in another work queue 'hns_roce_irq_work_handle()'
-- Accepted. No new work queue is created but the irq WQ is reused from v2.
3. don't do allocations at all if you can't allow them to fail.
-- Accepted. The flush_work structure is initialized at compile time.
Comments in patch set V2:
1. It kind of looks like this can be called multiple times? It won't work
right unless it is called exactly once.
-- Agreed. So I proposed to fall back to V1 in V3 again.
2. 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?
-- Explained. This question has raised out the key point of our solution.
Flush operation should be implemented once the QP state is going to
be error or the producer index is updated for the QP in error state.
3. The work function does something that looks like it only has to happen
once per QP. One do you need to keep re-queing this thing every time the user posts
a WR?
-- Explained. The root cause is that hip08 needs the newest PI to flush the outstanding
wqe every time the PI is updated. That's re-queuing is needed.
In conclusion, we have accepted all the comments in V1, and explained to all 3 comments in V2
but no further response. Do I have missed to cover any of your concerns?
I would really appreciate it if you could help to point out the unsolved issue.
Thanks.
