From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Hildenbrand Subject: Re: An emulation failure occurs, if I hotplug vcpus immediately after the VM start Date: Thu, 7 Jun 2018 14:55:47 +0200 Message-ID: References: <7CECC2DFC21538489F72729DF5EFB4D9C1486C@DGGEMM501-MBX.china.huawei.com> <20180601122307.3e6ade66@redhat.com> <33183CC9F5247A488A2544077AF19020DB00F4E4@dggeml511-mbx.china.huawei.com> <50481bea-bb5b-dd71-b712-6418c3bb29ac@redhat.com> <77604174-3c15-a8d3-3ea3-53a1759cd885@redhat.com> <56943de0-74f6-0746-c2c2-eeb9fdeabd74@redhat.com> <4409ea2d-971d-8898-bd1a-9e9b9e9172dc@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "Huangweidong \(C\)" , Zhanghailiang , "kvm@vger.kernel.org" , "wangxin \(U\)" , "qemu-devel@nongnu.org" , lidonglin To: Paolo Bonzini , "Gonglei (Arei)" , Igor Mammedov , xuyandong Return-path: In-Reply-To: <4409ea2d-971d-8898-bd1a-9e9b9e9172dc@redhat.com> Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel2=m.gmane.org@nongnu.org Sender: "Qemu-devel" List-Id: kvm.vger.kernel.org On 07.06.2018 14:36, Paolo Bonzini wrote: > On 07/06/2018 13:36, David Hildenbrand wrote: >>> The dirty bitmap would be synced in kvm_region_del (so it's not true >>> that kvm_region_del would disappear, but almost :)). >> I was rather concerned when doing a KVM_SET_USER_MEMORY_REGIONS while >> some (already present) memory region is performing dirty tracking and >> therefore has a dirty_bitmap pointer assigned. >> >> As we have to expect that all different kinds of parameters can change >> (e.g. the size of a slot as I pointed out), the old bitmap cannot be >> reused. At least not atomically - we could create a new one and the >> simply or the old content. >> >> Well, we could make that dirty tracking a special case ("all dirty >> tracking data will be lost in case doing a KVM_SET_USER_MEMORY_REGIONS" >> - but this again could lead to races (if the bitmap sync happens before >> KVM_SET_USER_MEMORY_REGIONS)). It's tricky to get it right :) > > At the point where QEMU calls region_del, the guest is not supposed to > access the region anymore, so it's okay to do the final sync there. The > same race exists now already. The point is that KVM_SET_USER_MEMORY_REGIONS is the big hammer, not the fine grained region_add/region_del. All regions are suddenly involved. So we have to handle dirty_bitmaps somehow. But I agree that those that would actually change (split/resized/whatever) should not be currently dirty tracked (or at if they are, they should be able to live with the possible race). Devil is in the detail :) > > Paolo > >>> The rmap is more interesting. Perhaps it can be just rebuilt on every >>> KVM_SET_USER_MEMORY_REGIONS call. > -- Thanks, David / dhildenb From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fQuSH-0000jl-65 for qemu-devel@nongnu.org; Thu, 07 Jun 2018 08:55:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fQuSE-0004uI-4I for qemu-devel@nongnu.org; Thu, 07 Jun 2018 08:55:53 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:39154 helo=mx1.redhat.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1fQuSD-0004uB-Ta for qemu-devel@nongnu.org; Thu, 07 Jun 2018 08:55:50 -0400 References: <7CECC2DFC21538489F72729DF5EFB4D9C1486C@DGGEMM501-MBX.china.huawei.com> <20180601122307.3e6ade66@redhat.com> <33183CC9F5247A488A2544077AF19020DB00F4E4@dggeml511-mbx.china.huawei.com> <50481bea-bb5b-dd71-b712-6418c3bb29ac@redhat.com> <77604174-3c15-a8d3-3ea3-53a1759cd885@redhat.com> <56943de0-74f6-0746-c2c2-eeb9fdeabd74@redhat.com> <4409ea2d-971d-8898-bd1a-9e9b9e9172dc@redhat.com> From: David Hildenbrand Message-ID: Date: Thu, 7 Jun 2018 14:55:47 +0200 MIME-Version: 1.0 In-Reply-To: <4409ea2d-971d-8898-bd1a-9e9b9e9172dc@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] An emulation failure occurs, if I hotplug vcpus immediately after the VM start List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , "Gonglei (Arei)" , Igor Mammedov , xuyandong Cc: Zhanghailiang , "wangxin (U)" , lidonglin , "kvm@vger.kernel.org" , "qemu-devel@nongnu.org" , "Huangweidong (C)" On 07.06.2018 14:36, Paolo Bonzini wrote: > On 07/06/2018 13:36, David Hildenbrand wrote: >>> The dirty bitmap would be synced in kvm_region_del (so it's not true >>> that kvm_region_del would disappear, but almost :)). >> I was rather concerned when doing a KVM_SET_USER_MEMORY_REGIONS while >> some (already present) memory region is performing dirty tracking and >> therefore has a dirty_bitmap pointer assigned. >> >> As we have to expect that all different kinds of parameters can change >> (e.g. the size of a slot as I pointed out), the old bitmap cannot be >> reused. At least not atomically - we could create a new one and the >> simply or the old content. >> >> Well, we could make that dirty tracking a special case ("all dirty >> tracking data will be lost in case doing a KVM_SET_USER_MEMORY_REGIONS" >> - but this again could lead to races (if the bitmap sync happens before >> KVM_SET_USER_MEMORY_REGIONS)). It's tricky to get it right :) > > At the point where QEMU calls region_del, the guest is not supposed to > access the region anymore, so it's okay to do the final sync there. The > same race exists now already. The point is that KVM_SET_USER_MEMORY_REGIONS is the big hammer, not the fine grained region_add/region_del. All regions are suddenly involved. So we have to handle dirty_bitmaps somehow. But I agree that those that would actually change (split/resized/whatever) should not be currently dirty tracked (or at if they are, they should be able to live with the possible race). Devil is in the detail :) > > Paolo > >>> The rmap is more interesting. Perhaps it can be just rebuilt on every >>> KVM_SET_USER_MEMORY_REGIONS call. > -- Thanks, David / dhildenb