On Tue, Feb 01, 2022 at 05:50:48PM +0100, Karsten Graul wrote: > On 28/01/2022 07:55, Tony Lu wrote: > > On Thu, Jan 27, 2022 at 03:52:36PM +0100, Karsten Graul wrote: > >> On 27/01/2022 10:50, Tony Lu wrote: > >>> On Thu, Jan 27, 2022 at 11:25:41AM +0200, Leon Romanovsky wrote: > >>>> On Thu, Jan 27, 2022 at 05:14:35PM +0800, Tony Lu wrote: > >>>>> On Thu, Jan 27, 2022 at 10:47:09AM +0200, Leon Romanovsky wrote: > >>>>>> On Thu, Jan 27, 2022 at 03:59:36PM +0800, Tony Lu wrote: > >>>>> > >>>>> Sorry for that if I missed something about properly using existing > >>>>> in-kernel API. I am not sure the proper API is to use ib_cq_pool_get() > >>>>> and ib_cq_pool_put()? > >>>>> > >>>>> If so, these APIs doesn't suit for current smc's usage, I have to > >>>>> refactor logic (tasklet and wr_id) in smc. I think it is a huge work > >>>>> and should do it with full discussion. > >>>> > >>>> This discussion is not going anywhere. Just to summarize, we (Jason and I) > >>>> are asking to use existing API, from the beginning. > >>> > >>> Yes, I can't agree more with you about using existing API and I have > >>> tried them earlier. The existing APIs are easy to use if I wrote a new > >>> logic. I also don't want to repeat the codes. > >>> > >>> The main obstacle is that the packet and wr processing of smc is > >>> tightly bound to the old API and not easy to replace with existing API. > >>> > >>> To solve a real issue, I have to fix it based on the old API. If using > >>> existing API in this patch, I have to refactor smc logics which needs > >>> more time. Our production tree is synced with smc next. So I choose to > >>> fix this issue first, then refactor these logic to fit existing API once > >>> and for all. > >> > >> While I understand your approach to fix the issue first I need to say > >> that such interim fixes create an significant amount of effort that has to > >> be spent for review and test for others. And there is the increased risk > >> to introduce new bugs by just this only-for-now fix. > > > > Let's back to this patch itself. This approach spreads CQs to different > > vectors, it tries to solve this issue under current design and not to > > introduce more changes to make it easier to review and test. It severely > > limits the performance of SMC when replacing TCP. This patch tries to > > reduce the gap between SMC and TCP. > > > > To use newer API, it should have a lots of work to do with wr process > > logic, for example remove tasklet handler, refactor wr_id logic. I have > > no idea if we should do this? If it's okay and got your permission, I > > will do this in the next patch. > > Hi Tony, > > I think there was quite a discussion now about this patch series and the conclusion from > the RDMA list and from my side was that if this code is changed it should be done using > the new API. The current version re-implements code that is already available there. > > I agree that using the new API is the way to go, and I am in for any early discussions > about the changes that are needed. > Thank you for pointing me to the sure way. I am working on this. I will send the complete refactor version with the new API later. Best regards, Tony Lu
