From mboxrd@z Thu Jan 1 00:00:00 1970 From: Radim =?utf-8?B?S3LEjW3DocWZ?= Subject: Re: [PATCH 3/6] KVM: Dirty memory tracking for performant checkpointing and improved live migration Date: Wed, 4 May 2016 15:13:15 +0200 Message-ID: <20160504131314.GA27590@potion> References: <201604261855.u3QItn85024244@dev1.sn.stratus.com> <33d8668e-2bba-af91-069e-6452609a6ff0@linux.intel.com> <20160429181911.GA2687@potion> <20160503141118.GA27975@potion> <32d8060e-648c-cf99-970a-3ddadc6a501a@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: "Cao, Lei" , Paolo Bonzini , "kvm@vger.kernel.org" To: "Huang, Kai" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:53333 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750728AbcEDNNT (ORCPT ); Wed, 4 May 2016 09:13:19 -0400 Content-Disposition: inline In-Reply-To: <32d8060e-648c-cf99-970a-3ddadc6a501a@linux.intel.com> Sender: kvm-owner@vger.kernel.org List-ID: 2016-05-04 19:45+1200, Huang, Kai: > On 5/4/2016 2:11 AM, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: >> 2016-05-03 18:06+1200, Huang, Kai: >> > Actually my concern is, with your new mechanism to track guest dir= ty pages, >> > there will be two logdirty mechanisms (using bitmap and your per-v= cpu list), >> > which I think is not good as it's a little bit redundant, given bo= th >> > mechanisms are used for dirty page logging. >> >=20 >> > I think your main concern of current bitmap mechanism is scanning = bitmap >> > takes lots of time, especially when only few pages get dirty, you = still have >> > to scan the entire bitmap, which results in bad performance if you= runs >> > checkpoint very frequently. My suggestion is, instead of introduci= ng two >> > logdirty data structures, maybe you can try to use another more ef= ficient >> > data structure instead of bitmap for both current logdirty mechani= sm and >> > your new interfaces. Maybe Xen's log-dirty tree is a good referenc= e. >>=20 >> A sparse structure (buffer, tree, ...) also needs a mechanism to gro= w >> (store new entries), so concurrent accesses become a problem, becaus= e >> there has to be synchronization. I think that per-vcpu structure >> becomes mandatory when thousands VCPUs dirty memory at the same time= =2E >=20 > Yes synchronization will be needed. But even for per-vcpu structure, = we > still need per-vcpu lock to access, say, gfn_list, right? For example= , one > thread from userspace trying to get and clear dirty pages would need = to loop > all vcpus and acquire each vcpu's lock for gfn_list. (see function > mt_reset_all_gfns in patch 3/6). Looks this is not scalable neither? Coarse locking is optional. The list can be designed allow concurrent addition and removal (circullar buffer with 3 atomic markers). If we had 'vcpu -> memslot -> structure' then we would design the userspace interface so it would only affect one memslot, which would avoid any scalability issue even if there was a vcpu+memslot lock in each structure. >> > Maybe Xen's log-dirty tree is a good referenc= e. >>=20 >> Is there some top-level overview? >>=20 >> > From a glance at the code, it looked like GPA bitmap sparsified wi= th >> radix tree in a manner similar to the page table hierarchy. >=20 > Yes it is just a radix tree. The point is the tree will be pretty sma= ll if > there are few dirty pages, so the scanning will be very quick, compar= ing to > bitmap. Bitmap had slow scanning, but any dynamic structure will have problems with insertion ... I think the tree might work if we pre-allotected bigger chunks to avoid allocation overhead and made it "lockless" (fine grained locking using cmpxchg) to avoid a bottleneck for concurrent writes. >> We should have dynamic sparse dirty log, to avoid wasting memory whe= n >> there are many small memslots, but a linear structure is probably st= ill >> fine. >=20 > The sparse dirty log structure can be allocated when necessary so I d= on't > think it will waste of memory. Take radix tree as example, if there's= no > dirty page in the slot, the pointer to radix can be NULL, or just roo= t > entry. (And we want to waste some memory, because allocations are slow, tradeoffs, tradeoffs ...) >> We don't care which vcpu dirtied the page, so it seems like a waste = to >> have them in the hierarchy, but I can't think of designs where the >> sparse dirty log is rooted in memslot and its updates scale well. >>=20 >> 'memslot -> sparse dirty log' usually evolve into buffering on the V= CPU >> side before writing to the memslot or aren't efficient for sparse >> dataset. >>=20 >> Where do you think is the balance between 'memslot -> bitmap' and >> 'vcpu -> memslot -> dirty buffer'? >=20 > In my opinion, we can first try 'memslot -> sparse dirty log'. Cao, L= ei > mentioned there were two bottlenecks: bitmap and bad multithread perf= ormance > due to mmu_lock. I think 'memslot->sparse dirty log' might help to im= prove > or solve the bitmap one. The bimap was chosen because it scales well with concurrent writes and it easy to export. Lei already hit scalability issues with mmu_lock, s= o I don't expect that we could afford to put all VCPUs onto one lock elsewhere. Good designs so far seem to be: memslot -> lockless radix tree and vcpu -> memslot -> list (memslot -> vcpu -> list) I'd like to see the lockless radix tree, but I expect the per-vcpu list to be *much* easier to implment. Do you see other designs on the pareto front? Thanks.