On 24/10/2018 00:55, Junaid Shahid wrote: > Yes, this would be incompatible with a ring buffer based dirty > logging API. But I think that it might still be potentially > worthwhile to have this available. At least during my testing, the > cost of the IOCTLs themselves had generally been less of a problem > for the brownout phase, but more for blackout. And at least for some > workloads, the performance reduction of write-protection can be > higher e.g. I had seen 50%+ performance reduction in some (admittedly > extreme) benchmarks. FWIW I have played with another optimization of dirty logging, namely decoupling the retrieval and clearing of dirty bits. There are many benefits: you avoid having to send the same page multiple times if it's modified by the guest between the retrieval of the dirty bitmap and the actual sending of the page; you therefore have less write protection faults, decreasing their overhead; and finally KVM_GET_DIRTY_LOG now need not take the mmu_lock at all. The change is very simple, basically a new enable-able capability that makes KVM_GET_DIRTY_LOG *not* clear the dirty bitmap, and a new ioctl KVM_CLEAR_DIRTY_LOG that takes slot, first_page, num_pages and a bitmaps of bits to clear. The bitmap will usually be whatever you get from KVM_GET_DIRTY_LOG, but it can also be a superset including bits from DMA destinations. Because it takes a first_page and num_pages, the time spent with the mmu_lock taken can be made smaller. However, if I remember correctly, within GCE you are already working around this by defining guests with multiple memory slots. So perhaps it wouldn't help the above extreme 50%+ case. Also, out of curiosity, since you have probably done benchmarks more recently than me, how bad is the impact of enabling write protection on large guests? I recall the initial impact at the beginning of live migration was pretty bad, and possibly even seeing soft lockups due to excessive time spent holding the mmu_lock. Perhaps we could reduce it by setting the dirty bitmap to all ones when KVM_MEM_LOG_DIRTY_PAGES is added to a memslot. Together with the above optimization, this would spread the cost of removing write access over the bulk RAM transfer. This change in semantics could be tied to the same capability that introduces manual dirty-log reprotection. Paolo > Perhaps we could extend the dynamic mode switching mechanism in the > last couple of patches to also switch between the two flavors of the > dirty logging API? Then we might be able to get the best of both > worlds: Dirty-bit based logging using the existing API during most of > brownout and Write Protection based logging using the ring buffer API > during the last stages of brownout and for blackout.
