On 24/10/19 21:38, Sean Christopherson wrote: > only > * its new index into the array is update. s/update/tracked/? Returns the changed memslot's > * current index into the memslots array. > */ > static inline int kvm_memslot_move_backward(struct kvm_memslots *slots, > struct kvm_memory_slot *memslot) > { > struct kvm_memory_slot *mslots = slots->memslots; > int i; > > if (WARN_ON_ONCE(slots->id_to_index[memslot->id] == -1) || > WARN_ON_ONCE(!slots->used_slots)) > return -1; > > for (i = slots->id_to_index[memslot->id]; i < slots->used_slots - 1; i++) { > if (memslot->base_gfn > mslots[i + 1].base_gfn) > break; > > WARN_ON_ONCE(memslot->base_gfn == mslots[i + 1].base_gfn); > > /* Shift the next memslot forward one and update its index. */ > mslots[i] = mslots[i + 1]; > slots->id_to_index[mslots[i].id] = i; > } > return i; > } > > /* > * Move a changed memslot forwards in the array by shifting existing slots with > * a lower GFN toward the back of the array. Note, the changed memslot itself > * is not preserved in the array, i.e. not swapped at this time, only its new > * index into the array is updated Same here? > * Note, slots are sorted from highest->lowest instead of lowest->highest for > * historical reasons. Not just that, the largest slot (with all RAM above 4GB) is also often at the highest address at least on x86. But we could sort them by size now, so I agree to call these historical reasons. The code itself is fine, thanks for the work on documenting it. Paolo