Re: [RFC PATCH net-next 0/6] net/smc: Spread workload over multiple cores

From: Karsten Graul <kgraul@linux.ibm.com>
To: Tony Lu <tonylu@linux.alibaba.com>, Leon Romanovsky <leon@kernel.org>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
	kuba@kernel.org, davem@davemloft.net, netdev@vger.kernel.org,
	linux-s390@vger.kernel.org,
	RDMA mailing list <linux-rdma@vger.kernel.org>
Subject: Re: [RFC PATCH net-next 0/6] net/smc: Spread workload over multiple cores
Date: Thu, 27 Jan 2022 15:52:36 +0100	[thread overview]
Message-ID: <3fcfdf75-eb8c-426d-5874-3afdc49de743@linux.ibm.com> (raw)
In-Reply-To: <YfJq5pygXS13XRhp@TonyMac-Alibaba>

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.

Given the fact that right now you are the only one who is affected by this problem
I recommend to keep your fix in your environment for now, and come back with the
final version. In the meantime I can use the saved time to review the bunch 
of other patches that we received.

Thank you!