Re: [PATCH] mm/mmu_notifier: avoid double notification when it is useless

From: Nadav Amit
Date: Tue Oct 03 2017 - 20:43:52 EST


Jerome Glisse <jglisse@xxxxxxxxxx> wrote:

> On Wed, Oct 04, 2017 at 01:42:15AM +0200, Andrea Arcangeli wrote:
>
>> I'd like some more explanation about the inner working of "that new
>> user" as per comment above.
>>
>> It would be enough to drop mmu_notifier_invalidate_range from above
>> without adding it to the filebacked case. The above gives higher prio
>> to the hypothetical and uncertain future case, than to the current
>> real filebacked case that doesn't need ->invalidate_range inside the
>> PT lock, or do you see something that might already need such
>> ->invalidate_range?
>
> No i don't see any new user today that might need such invalidate but
> i was trying to be extra cautious as i have a tendency to assume that
> someone might do a patch that use try_to_unmap() without going through
> all the comments in the function and thus possibly using it in a an
> unexpected way from mmu_notifier callback point of view. I am fine
> with putting the burden on new user to get it right and adding an
> extra warning in the function description to try to warn people in a
> sensible way.

I must be missing something. After the PTE is changed, but before the
secondary TLB notification/invalidation - What prevents another thread from
changing the mappings (e.g., using munmap/mmap), and setting a new page
at that PTE?

Wouldnât it end with the page being mapped without a secondary TLB flush in
between?

Nadav