On 4/29/2016 2:19 PM, Radim Krčmář 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 make it >>> embedded to kvm_memory_slot? In my understanding the main difference >>> 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 list >> 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 updated >> 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. >> It should be noted that what is saved on the dirty list is >> (mem slot id|offset), not gfn. > > Current compaction causes a problem. Slot id is u32, but gnflist stores > only bottom 16 bits while upper 16 contain important address space. > And 48 bit offset isn't enough for GPA sizes above 60 bits. > > Compaction can reserve up to PAGE_SHIFT bits. KVM must BUILD_BUG_ON() > if KVM_ADDRESS_SPACE_NUM and KVM_MEM_SLOTS_NUM don't fit. (We'll be > this reworking compaction as soon as per-vcpu address spaces arrive, > because KVM_MEM_SLOTS_NUM already takes 9 bits.) > memory slot id is currently defined as short. If I understand correctly, you are saying that it'll become u32 when per-vcpu address space arrives, right? It's certainly an issue for the current compaction. I'll need to rework it. Thanks! -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
