From: Zheng Chuan <zhengchuan@huawei.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: yubihong@huawei.com, berrange@redhat.com,
zhang.zhanghailiang@huawei.com, quintela@redhat.com,
qemu-devel@nongnu.org, xiexiangyou@huawei.com,
alex.chen@huawei.com, wanghao232@huawei.com
Subject: Re: [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration
Date: Sat, 6 Mar 2021 16:45:29 +0800 [thread overview]
Message-ID: <99dbb52f-e5fd-a7b4-8b50-ef9bfb484d0f@huawei.com> (raw)
In-Reply-To: <20210204103243.GF3039@work-vm>
On 2021/2/4 18:32, Dr. David Alan Gilbert wrote:
> * Chuan Zheng (zhengchuan@huawei.com) wrote:
>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
>> ---
>> migration/multifd.c | 6 ++++++
>> migration/multifd.h | 1 +
>> migration/rdma.c | 16 +++++++++++++++-
>> 3 files changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/multifd.c b/migration/multifd.c
>> index 1186246..4031648 100644
>> --- a/migration/multifd.c
>> +++ b/migration/multifd.c
>> @@ -577,6 +577,9 @@ void multifd_save_cleanup(void)
>> p->packet_len = 0;
>> g_free(p->packet);
>> p->packet = NULL;
>> +#ifdef CONFIG_RDMA
>> + multifd_rdma_cleanup(p->rdma);
>> +#endif
>
> You may find it easier to add an entry into stubs/ for
> multifd_rdma_cleanup; it then avoids the need for the ifdef.
>
OK. I will do that in V5.
>> multifd_send_state->ops->send_cleanup(p, &local_err);
>> if (local_err) {
>> migrate_set_error(migrate_get_current(), local_err);
>> @@ -1039,6 +1042,9 @@ int multifd_load_cleanup(Error **errp)
>> p->packet_len = 0;
>> g_free(p->packet);
>> p->packet = NULL;
>> +#ifdef CONFIG_RDMA
>> + multifd_rdma_cleanup(p->rdma);
>> +#endif
>> multifd_recv_state->ops->recv_cleanup(p);
>> }
>> qemu_sem_destroy(&multifd_recv_state->sem_sync);
>> diff --git a/migration/multifd.h b/migration/multifd.h
>> index 26d4489..0ecec5e 100644
>> --- a/migration/multifd.h
>> +++ b/migration/multifd.h
>> @@ -183,6 +183,7 @@ typedef struct {
>>
>> #ifdef CONFIG_RDMA
>> extern MultiFDSetup multifd_rdma_ops;
>> +void multifd_rdma_cleanup(void *opaque);
>> #endif
>> void multifd_send_terminate_threads(Error *err);
>> int multifd_send_initial_packet(MultiFDSendParams *p, Error **errp);
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index c19a91f..f14357f 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -2369,7 +2369,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
>> {
>> int idx;
>>
>> - if (rdma->cm_id && rdma->connected) {
>> + if (rdma->channel && rdma->cm_id && rdma->connected) {
>> if ((rdma->error_state ||
>> migrate_get_current()->state == MIGRATION_STATUS_CANCELLING) &&
>> !rdma->received_error) {
>> @@ -4599,6 +4599,20 @@ static void multifd_rdma_recv_channel_setup(QIOChannel *ioc,
>> return;
>> }
>>
>> +void multifd_rdma_cleanup(void *opaque)
>
> I think you need to make it clear that this is only to cleanup one
> channel, rather than the whole multifd-rdma connection;
> multifd_load_cleanup for example cleans up all the channels, where as I
> think this is only doing one?
>
Yes, the multifd_load_cleanup cleans up all the channels with the iteration of migrate_multifd_channels.
Now, we just put multifd_rdma_cleanup into that iteration of multifd_load_cleanup to clear it one by one.
do you mean the name of multifd_rdma_cleanup is misleading and should changed it in order to distinguish with multifd_load_cleanup?
> Don't use a 'void *opaque' except for something that's called via
> a registration/callback scheme that's designed to be generic
> (e.g. multifd_send_thread does it because it's called from
> qemu_thread_create that doesn't know the type). Where you know
> the type, use it!
>
I agree with you.
I will do that in V5 with typedefs.h.
>> +{
>> + RDMAContext *rdma = (RDMAContext *)opaque;
>> +
>> + if (!migrate_use_rdma()) {
>> + return;
>> + }
>> +
>> + rdma->listen_id = NULL;
>> + rdma->channel = NULL;
>> + qemu_rdma_cleanup(rdma);
>> + g_free(rdma);
>> +}
>> +
>> MultiFDSetup multifd_rdma_ops = {
>> .send_thread = multifd_rdma_send_thread,
>> .recv_thread = multifd_rdma_recv_thread,
>> --
>> 1.8.3.1
>>
--
Regards.
Chuan
prev parent reply other threads:[~2021-03-06 8:47 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-03 8:01 [PATCH v4 00/18] Support Multifd for RDMA migration Chuan Zheng
2021-02-03 8:01 ` [PATCH v4 01/18] migration/rdma: add the 'migrate_rdma_pin_all' function Chuan Zheng
2021-02-03 8:01 ` [PATCH v4 02/18] migration/rdma: judge whether or not the RDMA is used for migration Chuan Zheng
2021-02-03 17:49 ` Dr. David Alan Gilbert
2021-03-01 12:25 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 03/18] migration/rdma: create multifd_setup_ops for Tx/Rx thread Chuan Zheng
2021-02-03 8:01 ` [PATCH v4 04/18] migration/rdma: add multifd_setup_ops for rdma Chuan Zheng
2021-02-03 17:58 ` Dr. David Alan Gilbert
2021-02-03 8:01 ` [PATCH v4 05/18] migration/rdma: do not need sync main " Chuan Zheng
2021-02-03 18:10 ` Dr. David Alan Gilbert
2021-03-06 8:45 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 06/18] migration/rdma: export MultiFDSendParams/MultiFDRecvParams Chuan Zheng
2021-02-03 18:23 ` Dr. David Alan Gilbert
2021-03-01 12:26 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 07/18] migration/rdma: add rdma field into multifd send/recv param Chuan Zheng
2021-02-03 18:32 ` Dr. David Alan Gilbert
2021-03-01 12:26 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 08/18] migration/rdma: export getQIOChannel to get QIOchannel in rdma Chuan Zheng
2021-02-03 18:49 ` Dr. David Alan Gilbert
2021-03-01 12:26 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 09/18] migration/rdma: add multifd_rdma_load_setup() to setup multifd rdma Chuan Zheng
2021-02-03 8:01 ` [PATCH v4 10/18] migration/rdma: Create the multifd recv channels for RDMA Chuan Zheng
2021-02-03 18:59 ` Dr. David Alan Gilbert
2021-03-06 8:45 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 11/18] migration/rdma: record host_port for multifd RDMA Chuan Zheng
2021-02-03 19:04 ` Dr. David Alan Gilbert
2021-03-01 12:26 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 12/18] migration/rdma: Create the multifd send channels for RDMA Chuan Zheng
2021-02-03 19:52 ` Dr. David Alan Gilbert
2021-03-01 12:26 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 13/18] migration/rdma: Add the function for dynamic page registration Chuan Zheng
2021-02-03 20:06 ` Dr. David Alan Gilbert
2021-03-01 12:26 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 14/18] migration/rdma: register memory for multifd RDMA channels Chuan Zheng
2021-02-03 20:12 ` Dr. David Alan Gilbert
2021-03-06 8:45 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 15/18] migration/rdma: only register the memory for multifd channels Chuan Zheng
2021-02-04 10:09 ` Dr. David Alan Gilbert
2021-02-03 8:01 ` [PATCH v4 16/18] migration/rdma: add rdma_channel into Migrationstate field Chuan Zheng
2021-02-03 20:19 ` Dr. David Alan Gilbert
2021-03-01 12:27 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 17/18] migration/rdma: send data for both rdma-pin-all and NOT rdma-pin-all mode Chuan Zheng
2021-02-04 10:18 ` Dr. David Alan Gilbert
2021-03-06 8:45 ` Zheng Chuan
2021-02-03 8:01 ` [PATCH v4 18/18] migration/rdma: RDMA cleanup for multifd migration Chuan Zheng
2021-02-04 10:32 ` Dr. David Alan Gilbert
2021-03-06 8:45 ` Zheng Chuan [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=99dbb52f-e5fd-a7b4-8b50-ef9bfb484d0f@huawei.com \
--to=zhengchuan@huawei.com \
--cc=alex.chen@huawei.com \
--cc=berrange@redhat.com \
--cc=dgilbert@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=quintela@redhat.com \
--cc=wanghao232@huawei.com \
--cc=xiexiangyou@huawei.com \
--cc=yubihong@huawei.com \
--cc=zhang.zhanghailiang@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).