[PATCH] migration/rdma: prevent from double free the same mr

[PATCH] migration/rdma: prevent from double free the same mr

[Li Zhijian]

backtrace:
'0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
478             void *addr              = mr->addr;
(gdb) bt
 #0  0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
 #1  0x0000555555891fcc in rdma_delete_block (block=<optimized out>, rdma=0x7fff38176010) at ../migration/rdma.c:691
 #2  qemu_rdma_cleanup (rdma=0x7fff38176010) at ../migration/rdma.c:2365
 #3  0x00005555558925b0 in qio_channel_rdma_close_rcu (rcu=0x555556b8b6c0) at ../migration/rdma.c:3073
 #4  0x0000555555d652a3 in call_rcu_thread (opaque=opaque@entry=0x0) at ../util/rcu.c:281
 #5  0x0000555555d5edf9 in qemu_thread_start (args=0x7fffe88bb4d0) at ../util/qemu-thread-posix.c:541
 #6  0x00007ffff54c73f9 in start_thread () at /lib64/libpthread.so.0
 #7  0x00007ffff53f3b03 in clone () at /lib64/libc.so.6 '

Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
 migration/rdma.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index b6cc4bef4a8..0f22b8227c0 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1143,6 +1143,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
 
     for (i--; i >= 0; i--) {
         ibv_dereg_mr(local->block[i].mr);
+        local->block[i].mr = NULL;
         rdma->total_registrations--;
     }
 
-- 
2.30.2

Re: [PATCH] migration/rdma: prevent from double free the same mr

[Dr. David Alan Gilbert]

* Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
> backtrace:
> '0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
> 478             void *addr              = mr->addr;

ANy idea why it deletes the same mr twice?
What was your commandline?

Dave

> (gdb) bt
>  #0  0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
>  #1  0x0000555555891fcc in rdma_delete_block (block=<optimized out>, rdma=0x7fff38176010) at ../migration/rdma.c:691
>  #2  qemu_rdma_cleanup (rdma=0x7fff38176010) at ../migration/rdma.c:2365
>  #3  0x00005555558925b0 in qio_channel_rdma_close_rcu (rcu=0x555556b8b6c0) at ../migration/rdma.c:3073
>  #4  0x0000555555d652a3 in call_rcu_thread (opaque=opaque@entry=0x0) at ../util/rcu.c:281
>  #5  0x0000555555d5edf9 in qemu_thread_start (args=0x7fffe88bb4d0) at ../util/qemu-thread-posix.c:541
>  #6  0x00007ffff54c73f9 in start_thread () at /lib64/libpthread.so.0
>  #7  0x00007ffff53f3b03 in clone () at /lib64/libc.so.6 '
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  migration/rdma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index b6cc4bef4a8..0f22b8227c0 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1143,6 +1143,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
>  
>      for (i--; i >= 0; i--) {
>          ibv_dereg_mr(local->block[i].mr);
> +        local->block[i].mr = NULL;
>          rdma->total_registrations--;
>      }
>  
> -- 
> 2.30.2
> 
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] migration/rdma: prevent from double free the same mr

[lizhijian]

On 09/07/2021 03:11, Dr. David Alan Gilbert wrote:
> * Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
>> backtrace:
>> '0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
>> 478             void *addr              = mr->addr;
> ANy idea why it deletes the same mr twice?

It's easy to reproduce it if we specify a nvdimm backing to a fsdax memory-backend-file which cannot support registering mr like:

[root@iaas-rpma ~]# mount | grep pmem0
/dev/pmem0 on /mnt/pmem0 type ext4 (rw,relatime,seclabel,dax=always)

[root@iaas-rpma ~]# ndctl list -n namespace0.0
[
   {
     "dev":"namespace0.0",
     "mode":"fsdax",
     "map":"mem",
     "size":536870912,
     "sector_size":512,
     "blockdev":"pmem0"
   }
]


`-object memory-backend-file,id=mem1,share=on,mem-path=/mnt/pmem0/nv-128m.img,size=128m,pmem=on,align=2m -device nvdimm,memdev=mem1,id=nv1`

and then enable rdma-pin-all.

(qemu) migrate_set_capability rdma-pin-all on
(qemu)

Now qemu has at least 2 ram block, pc.ram and mem1. the latter will be failed to register mr:
`Failed to register local dest ram block! : Invalid argument   `

in this case, the mr of pc.ram will be deleted twice.

Thanks
Li
>
> Dave
>
>> (gdb) bt
>>   #0  0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
>>   #1  0x0000555555891fcc in rdma_delete_block (block=<optimized out>, rdma=0x7fff38176010) at ../migration/rdma.c:691
>>   #2  qemu_rdma_cleanup (rdma=0x7fff38176010) at ../migration/rdma.c:2365
>>   #3  0x00005555558925b0 in qio_channel_rdma_close_rcu (rcu=0x555556b8b6c0) at ../migration/rdma.c:3073
>>   #4  0x0000555555d652a3 in call_rcu_thread (opaque=opaque@entry=0x0) at ../util/rcu.c:281
>>   #5  0x0000555555d5edf9 in qemu_thread_start (args=0x7fffe88bb4d0) at ../util/qemu-thread-posix.c:541
>>   #6  0x00007ffff54c73f9 in start_thread () at /lib64/libpthread.so.0
>>   #7  0x00007ffff53f3b03 in clone () at /lib64/libc.so.6 '
>>
>> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
>> ---
>>   migration/rdma.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index b6cc4bef4a8..0f22b8227c0 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -1143,6 +1143,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
>>   
>>       for (i--; i >= 0; i--) {
>>           ibv_dereg_mr(local->block[i].mr);
>> +        local->block[i].mr = NULL;
>>           rdma->total_registrations--;
>>       }
>>   
>> -- 
>> 2.30.2
>>
>>
>>

Re: [PATCH] migration/rdma: prevent from double free the same mr

[Dr. David Alan Gilbert]

* lizhijian@fujitsu.com (lizhijian@fujitsu.com) wrote:
> 
> 
> On 09/07/2021 03:11, Dr. David Alan Gilbert wrote:
> > * Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
> >> backtrace:
> >> '0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
> >> 478             void *addr              = mr->addr;
> > ANy idea why it deletes the same mr twice?
> 
> It's easy to reproduce it if we specify a nvdimm backing to a fsdax memory-backend-file which cannot support registering mr like:
> 
> [root@iaas-rpma ~]# mount | grep pmem0
> /dev/pmem0 on /mnt/pmem0 type ext4 (rw,relatime,seclabel,dax=always)
> 
> [root@iaas-rpma ~]# ndctl list -n namespace0.0
> [
>    {
>      "dev":"namespace0.0",
>      "mode":"fsdax",
>      "map":"mem",
>      "size":536870912,
>      "sector_size":512,
>      "blockdev":"pmem0"
>    }
> ]
> 
> 
> `-object memory-backend-file,id=mem1,share=on,mem-path=/mnt/pmem0/nv-128m.img,size=128m,pmem=on,align=2m -device nvdimm,memdev=mem1,id=nv1`
> 
> and then enable rdma-pin-all.
> 
> (qemu) migrate_set_capability rdma-pin-all on
> (qemu)
> 
> Now qemu has at least 2 ram block, pc.ram and mem1. the latter will be failed to register mr:
> `Failed to register local dest ram block! : Invalid argument   `
> 
> in this case, the mr of pc.ram will be deleted twice.

Ah OK, that makes more sense - from your original description I hadn't
noticed it was the failure path.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

> Thanks
> Li
> >
> > Dave
> >
> >> (gdb) bt
> >>   #0  0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
> >>   #1  0x0000555555891fcc in rdma_delete_block (block=<optimized out>, rdma=0x7fff38176010) at ../migration/rdma.c:691
> >>   #2  qemu_rdma_cleanup (rdma=0x7fff38176010) at ../migration/rdma.c:2365
> >>   #3  0x00005555558925b0 in qio_channel_rdma_close_rcu (rcu=0x555556b8b6c0) at ../migration/rdma.c:3073
> >>   #4  0x0000555555d652a3 in call_rcu_thread (opaque=opaque@entry=0x0) at ../util/rcu.c:281
> >>   #5  0x0000555555d5edf9 in qemu_thread_start (args=0x7fffe88bb4d0) at ../util/qemu-thread-posix.c:541
> >>   #6  0x00007ffff54c73f9 in start_thread () at /lib64/libpthread.so.0
> >>   #7  0x00007ffff53f3b03 in clone () at /lib64/libc.so.6 '
> >>
> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> >> ---
> >>   migration/rdma.c | 1 +
> >>   1 file changed, 1 insertion(+)
> >>
> >> diff --git a/migration/rdma.c b/migration/rdma.c
> >> index b6cc4bef4a8..0f22b8227c0 100644
> >> --- a/migration/rdma.c
> >> +++ b/migration/rdma.c
> >> @@ -1143,6 +1143,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
> >>   
> >>       for (i--; i >= 0; i--) {
> >>           ibv_dereg_mr(local->block[i].mr);
> >> +        local->block[i].mr = NULL;
> >>           rdma->total_registrations--;
> >>       }
> >>   
> >> -- 
> >> 2.30.2
> >>
> >>
> >>
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] migration/rdma: prevent from double free the same mr

[Dr. David Alan Gilbert]

* Li Zhijian (lizhijian@cn.fujitsu.com) wrote:
> backtrace:
> '0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
> 478             void *addr              = mr->addr;
> (gdb) bt
>  #0  0x00007ffff5f44ec2 in __ibv_dereg_mr_1_1 (mr=0x7fff1007d390) at /home/lizhijian/rdma-core/libibverbs/verbs.c:478
>  #1  0x0000555555891fcc in rdma_delete_block (block=<optimized out>, rdma=0x7fff38176010) at ../migration/rdma.c:691
>  #2  qemu_rdma_cleanup (rdma=0x7fff38176010) at ../migration/rdma.c:2365
>  #3  0x00005555558925b0 in qio_channel_rdma_close_rcu (rcu=0x555556b8b6c0) at ../migration/rdma.c:3073
>  #4  0x0000555555d652a3 in call_rcu_thread (opaque=opaque@entry=0x0) at ../util/rcu.c:281
>  #5  0x0000555555d5edf9 in qemu_thread_start (args=0x7fffe88bb4d0) at ../util/qemu-thread-posix.c:541
>  #6  0x00007ffff54c73f9 in start_thread () at /lib64/libpthread.so.0
>  #7  0x00007ffff53f3b03 in clone () at /lib64/libc.so.6 '
> 
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>

Queued

> ---
>  migration/rdma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index b6cc4bef4a8..0f22b8227c0 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1143,6 +1143,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
>  
>      for (i--; i >= 0; i--) {
>          ibv_dereg_mr(local->block[i].mr);
> +        local->block[i].mr = NULL;
>          rdma->total_registrations--;
>      }
>  
> -- 
> 2.30.2
> 
> 
> 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK