From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Huang, Kai" Subject: Re: [PATCH 3/6] KVM: Dirty memory tracking for performant checkpointing and improved live migration Date: Tue, 3 May 2016 18:06:21 +1200 Message-ID: References: <201604261855.u3QItn85024244@dev1.sn.stratus.com> <33d8668e-2bba-af91-069e-6452609a6ff0@linux.intel.com> <20160429181911.GA2687@potion> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-2; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Paolo Bonzini , "kvm@vger.kernel.org" To: "Cao, Lei" , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Return-path: Received: from mga03.intel.com ([134.134.136.65]:22110 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932223AbcECGGd (ORCPT ); Tue, 3 May 2016 02:06:33 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 5/3/2016 3:24 AM, Cao, Lei wrote: > On 4/29/2016 2:19 PM, Radim Kr=E8m=E1=F8 wrote: >> 2016-04-28 19:58+0000, Cao, Lei: >>> On 4/28/2016 5:13 AM, Huang, Kai wrote: >>>> One general comment: won't it be better if you devide kvm_mt and m= ake it >>>> embedded to kvm_memory_slot? In my understanding the main differen= ce >>>> between bitmap and your log-dirty mechanism is you are using list = not >>>> bitmap, and I think make the dirty_gfn_list embedded to kvm_memory= _slot >>>> should simplify lots of your code. >>> >>> It's true that one difference of the new mechanism is the use of li= st >>> instead of bitmap. Another difference is that the dirty list is per >>> vcpu, instead of per memory slot. This is so that the list can be u= pdated >>> without holding a lock. >> >> A writeup of alternatives behind your decision would be welcome. >> >> Per-vcpu structures seems crucial with higher amount of VCPUs and >> putting per-vcpu into kvm_memory_slots would either take much more >> memory, or wouldn't scale as well. >> >> Separating from kvm_memory_slot has its share of issues too, though, >> because slot id can stand for different slots during runtime. >> (Most/all can be solved by forbidding uses that lead to them, so >> userspace would be to blame if memory tracking worked incorrectly. >> E.g. reconfiguring a slot without pausing vcpus and draining dirty >> pages.) >> > > Good point. Our per-vcpu dirty list is separate from kvm_memory_slot.= I'll > look into the changing slot id issue. Actually my concern is, with your new mechanism to track guest dirty=20 pages, there will be two logdirty mechanisms (using bitmap and your=20 per-vcpu list), which I think is not good as it's a little bit=20 redundant, given both mechanisms are used for dirty page logging. I think your main concern of current bitmap mechanism is scanning bitma= p=20 takes lots of time, especially when only few pages get dirty, you still= =20 have to scan the entire bitmap, which results in bad performance if you= =20 runs checkpoint very frequently. My suggestion is, instead of=20 introducing two logdirty data structures, maybe you can try to use=20 another more efficient data structure instead of bitmap for both curren= t=20 logdirty mechanism and your new interfaces. Maybe Xen's log-dirty tree=20 is a good reference. Of course this is just my concern and I'll leave it to maintainers. Thanks, -Kai