On 03/02/2017 12:01, Cao, Lei wrote: > +struct kvm_dirty_list { Maybe kvm_dirty_ring? > + union { > + struct { > + __u16 avail_index; /* set by kernel */ > + __u16 fetch_index; /* set by userspace */ > + } indices; > + struct kvm_dirty_gfn dirty_gfns[0]; > + }; > +}; Please explain what this means. For example, adopting the free running counter technique explained in the review of patch 5: --- The two indices in the ring buffer are free running counters. They are _not_ limited to the range 0..size-1 where "size" is the number of element of the ring buffer. This makes it simpler to compute the number of entries in the ring buffer, which is simply (u16)(ring->avail_index - ring->fetch_index). In pseudocode, processing the ring buffer looks like this: idx = load-acquire(&ring->fetch_index); while (idx != ring->avail_index) { struct kvm_dirty_gfn *entry; entry = &ring->dirty_gfns[idx & (size - 1)]; ... idx++; } ring->fetch_index = idx; --- > The size of the ring should be page aligned > +and be 16 pages at a minimum. The larger the ring, the less > +likely the ring is full and the VM is forced to exit to > +userspace. The optimal size is workload dependent. --- The size of the ring must be a power of two. The larger the ring, the less likely the ring is full and the VM is forced to exit to userspace. The optimal size depends on the workload, but it is recommended that it be at least 64 KiB (4096 entries). --- Paolo