On Wed, Jan 04, 2012 at 03:06:43PM +0900, Takuya Yoshikawa wrote:
> It is possible that the __set_bit() in mark_page_dirty() is called
> simultaneously on the same region of memory, which may result in only
> one bit being set, because some callers do not take mmu_lock before
> mark_page_dirty().
>
> This problem is hard to produce because when we reach mark_page_dirty()
> beginning from, e.g., tdp_page_fault(), mmu_lock is being held during
> __direct_map(): making kvm-unit-tests' dirty log api test write to two
> pages concurrently was not useful for this reason.
>
> So we have confirmed that there can actually be race condition by
> checking if some callers really reach there without holding mmu_lock
> using spin_is_locked(): probably they were from kvm_write_guest_page().
>
> To fix this race, this patch changes the bit operation to the atomic
> version: note that nr_dirty_pages also suffers from the race but we do
> not need exactly correct numbers for now.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@xxxxxxxxxxxxx>
> ---
> virt/kvm/kvm_main.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 7287bf5..a91f980 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1543,7 +1543,7 @@ void mark_page_dirty_in_slot(struct kvm *kvm, struct kvm_memory_slot *memslot,
> if (memslot && memslot->dirty_bitmap) {
> unsigned long rel_gfn = gfn - memslot->base_gfn;
>
> - if (!__test_and_set_bit_le(rel_gfn, memslot->dirty_bitmap))
> + if (!test_and_set_bit_le(rel_gfn, memslot->dirty_bitmap))
> memslot->nr_dirty_pages++;
> }
> }
Looks good to me.
--
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
