Hi Peter, > -----Original Message----- > From: Peter Xu [mailto:pet...@redhat.com] > Sent: Wednesday, June 5, 2024 3:32 AM > To: Gonglei (Arei) <arei.gong...@huawei.com> > Cc: qemu-devel@nongnu.org; yu.zh...@ionos.com; mgal...@akamai.com; > elmar.ger...@ionos.com; zhengchuan <zhengch...@huawei.com>; > berra...@redhat.com; arm...@redhat.com; lizhij...@fujitsu.com; > pbonz...@redhat.com; m...@redhat.com; Xiexiangyou > <xiexiang...@huawei.com>; linux-r...@vger.kernel.org; lixiao (H) > <lixia...@huawei.com>; jinpu.w...@ionos.com; Wangjialin > <wangjiali...@huawei.com>; Fabiano Rosas <faro...@suse.de> > Subject: Re: [PATCH 0/6] refactor RDMA live migration based on rsocket API > > Hi, Lei, Jialin, > > Thanks a lot for working on this! > > I think we'll need to wait a bit on feedbacks from Jinpu and his team on RDMA > side, also Daniel for iochannels. Also, please remember to copy Fabiano > Rosas in any relevant future posts. We'd also like to know whether he has any > comments too. I have him copied in this reply. > > On Tue, Jun 04, 2024 at 08:14:06PM +0800, Gonglei wrote: > > From: Jialin Wang <wangjiali...@huawei.com> > > > > Hi, > > > > This patch series attempts to refactor RDMA live migration by > > introducing a new QIOChannelRDMA class based on the rsocket API. > > > > The /usr/include/rdma/rsocket.h provides a higher level rsocket API > > that is a 1-1 match of the normal kernel 'sockets' API, which hides > > the detail of rdma protocol into rsocket and allows us to add support > > for some modern features like multifd more easily. > > > > Here is the previous discussion on refactoring RDMA live migration > > using the rsocket API: > > > > https://lore.kernel.org/qemu-devel/20240328130255.52257-1-philmd@linar > > o.org/ > > > > We have encountered some bugs when using rsocket and plan to submit > > them to the rdma-core community. > > > > In addition, the use of rsocket makes our programming more convenient, > > but it must be noted that this method introduces multiple memory > > copies, which can be imagined that there will be a certain performance > > degradation, hoping that friends with RDMA network cards can help verify, > thank you! > > It'll be good to elaborate if you tested it in-house. What people should > expect > on the numbers exactly? Is that okay from Huawei's POV? > > Besides that, the code looks pretty good at a first glance to me. Before > others chim in, here're some high level comments.. > > Firstly, can we avoid using coroutine when listen()? Might be relevant when I > see that rdma_accept_incoming_migration() runs in a loop to do raccept(), but > would that also hang the qemu main loop even with the coroutine, before all > channels are ready? I'm not a coroutine person, but I think the hope is that > we can make dest QEMU run in a thread in the future just like the src QEMU, so > the less coroutine the better in this path. >
Because rsocket is set to non-blocking, raccept will return EAGAIN when no connection is received, coroutine will yield, and will not hang the qemu main loop. > I think I also left a comment elsewhere on whether it would be possible to > allow > iochannels implement their own poll() functions to avoid the per-channel poll > thread that is proposed in this series. > > https://lore.kernel.org/r/ZldY21xVExtiMddB@x1n > We noticed that, and it's a big operation. I'm not sure that's a better way. > Personally I think even with the thread proposal it's better than the old rdma > code, but I just still want to double check with you guys. E.g., maybe that > just > won't work at all? Again, that'll also be based on the fact that we move > migration incoming into a thread first to keep the dest QEMU main loop intact, > I think, but I hope we will reach that irrelevant of rdma, IOW it'll be nice > to > happen even earlier if possible. > Yep. This is a fairly big change, I wonder what other people's suggestions are? > Another nitpick is that qio_channel_rdma_listen_async() doesn't look used and > may prone to removal. > Yes. This is because when we wrote the test case, we wanted to test qio_channel_rdma_connect_async, and also I added qio_channel_rdma_listen_async. It is not used in the RDMA hot migration code. Regards, -Gonglei