Hi,
On 10/01/2014 18:57, Andrea Arcangeli wrote:
> Hi!
>
> On Fri, Jan 10, 2014 at 06:38:06PM +0200, Izik Eidus wrote:
>> It look like commit 6bdb913f0a70a4dfb7f066fb15e2d6f960701d00 break the
>> semantic of set_pte_at_notify.
>> The change of calling first to mmu_notifier_invalidate_range_start, then
>> to set_pte_at_notify, and then to mmu_notifier_invalidate_range_end
>> not only increase the amount of locks kvm have to take and release by
>> factor of 3, but in addition mmu_notifier_invalidate_range_start is zapping
>> the pte entry from kvm, so when set_pte_at_notify get called, it doesn`t
>> have any spte to set and it acctuly get called for nothing, the result is
>> increasing of vmexits for kvm from both do_wp_page and replace_page, and
>> broken semantic of set_pte_at_notify.
>
> Agreed.
>
> I would suggest to change set_pte_at_notify to return if change_pte
> was missing in some mmu notifier attached to this mm, so we can do
> something like:
>
> ptep = page_check_address(page, mm, addr, &ptl, 0);
> [..]
> notify_missing = false;
> if (... ) {
> entry = ptep_clear_flush(...);
> [..]
> notify_missing = set_pte_at_notify(mm, addr, ptep, entry);
> }
> pte_unmap_unlock(ptep, ptl);
> if (notify_missing)
> mmu_notifier_invalidate_page_if_missing_change_pte(mm, addr);
>
> and drop the range calls. This will provide sleepability and at the
> same time it won't screw the ability of change_pte to update sptes (by
> leaving those established by the time change_pte runs).
I think it would be better for notifiers that do not support change_pte
to keep getting both range_start and range_end notifiers. Otherwise, the
invalidate_page notifier might end up marking the old page as dirty
after it was already replaced in the primary page table.
Perhaps we can have a flag in the mmu_notifier, similar to the
notify_missing returned value here, that determines in these cases
whether to call the invalidate_range_start/end pair, or just the
set_pte_at_notify.
Thanks,
Haggai
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
