Re: Splitting the mmap_sem

[Jerome Glisse <jglisse@redhat.com>]

On Tue, Dec 10, 2019 at 04:26:40PM +0100, Vlastimil Babka wrote:
> On 12/5/19 6:21 PM, Jerome Glisse wrote:
> >> 
> >> So calling mmap() looks like this:
> >> 
> >>       1 allocate a new VMA
> >>       2 update pointer(s) in maple tree
> >>       3 sleep until old VMAs have a zero refcount
> >>       4 synchronize_rcu()
> >>       5 free old VMAs
> >>       6 flush caches for affected range
> >>       7 return to userspace
> >> 
> >> While one thread is calling mmap(MAP_FIXED), two other threads which are
> >> accessing the same address may see different data from each other and
> >> have different page translations in their respective CPU caches until
> >> the thread calling mmap() returns.  I believe this is OK, but would
> >> greatly appreciate hearing from people who know better.
> > 
> > I do not believe this is OK, i believe this is wrong (not even considering
> > possible hardware issues that can arise from such aliasing).
> 
> But is it true that the races can happen in the above such that multiple CPU's
> have different translations? I think it's impossible to tell from above - there
> are no details about when and which pte modifications happen, where ptl lock is
> taken... perhaps after filling those details, we could be able to see that
> there's no race.
> 

My assumption reading Matthew was that as step 6 is making progress
(flushing caches and i assume TLB too) then you can have a CPU which
is already flushed and that do take a fault against the new VMA and
thus get a new TLB entry that do not match a CPU which is not yet
flushed.

Today this can not happens because page fault will serialize on the
mmap_sem (ie until the write mode is release when returning to user-
space).

I advocate that for MAP_FIXED we should keep that behavior ie have
page fault wait until all the CPUs caches and TLB are flush before
servicing new fault. The rational are:
    - Application with racing thread accessing an area which is under
      going a mmap MAP_FIXED are doing something weird (which might
      be totaly waranted) and thus it should not matter if we penalize
      those by not offering the benefit of fully concurrent page fault
      for that area. We are not making thing any worse then they are
      today.

    - Waiting on new MAP_FIXED vma is easy to implement and does not
      affect other kind of page fault (ie you still have concurrent
      fault to other area).

Note i stress again this is only for MAP_FIXED replacing an existing
vma. For any other mmap (well i would need to go over all the flags
to make sure there is nothing else similar to MAP_FIXED) we can have
full concurrency ie:
    - Access to an area that is under going munmap will SEGFAULT
      as soon as the vma is no longer valid (ie no longer in maple
      tree i guess)

    - Access to an area that is just being mmap will fault using
      the new vma (assuming we publish the vma only once it is
      ready to take fault)

    - A mmap can make progress as another munmap is making progress
      and reuse the the munmaped area as soon as it is fully flushed
      (caches and TLB).

Cheers,
Jérôme