A few quick mmap questions

[Corinna Vinschen]

On Dec 19 18:36, Ken Brown wrote:
> Hi Corinna,
> 
> I plan to keep working on mmap issues while you're on vacation.  A few
> questions have come up while I've been looking at the code.
> 
> 1. The function is_mmapped_region() doesn't seem to be used anywhere. Is it
> OK to remove it?

Yes.  Commit 29a126322783 ("Simplify stack allocation code in child
after fork") dropped its only usage.  I just forgot to remove
is_mmapped_region() as well.

> 2. I can't find any uses of filler pages, in spite of the comment preceding
> the definition of __PROT_FILLER.  Is it OK to remove all references to
> __PROT_FILLER and the associated methods?

Yes.  The filler pages were only used on 32 bit Windows.  This code should
have been removed when we we're going 64 bit-only.

> 3. I'm still puzzled about mmap_is_attached_or_noreserve().  See my email
> from yesterday on the cygwin ML.  I think it's broken but I may be wrong.
> If I'm right, I don't see an easy way to fix it.  As I said before, the only
> thing I can think of is to sort the mmap_list of anonymous mappings.  But
> I'm afraid that would be too expensive (in terms of time).

I copy/pasted from the cygwin ML:

===================
> I think I'm seeing a similar confusion in                                     
> mmap_is_attached_or_noreserve().  I'm tired now and am having trouble         
> sorting out exactly what that function is doing.

noreserve() pages are MAP_RESERVE pages.  Trying to read or write on them
raises a STATUS_ACCESS_VIOLATION.  The idea is basically to commit the
requested read/write region (in case of the signal: just 1 byte, which in
turn requires to commit a full page.

> But the definition of       
> commit_len looks suspicious to me.  We know from above that start_addr <=     
> u_addr. 

We do?  match() returns the intersection, independently of addr being
lower or higher than get_address().  Same goes for the length.

In case of a signal:

  start_addr is addr rounded down to the page address and len is rounded
  up to the pagesize.  So we have a single page.  rec->match() either
  returns false, or if a noreserve() record matches, it will return the
  single page in u_addr, u_len.

    commit_len = 4096 - (0x100000000 - 0x100000000) = 4096

In case we're called via fhandler_base::raw_read():

  Assuming the region to read is enclosing the noreserve() region...

  - addr = 0x100000000, len = 256K
  - rec->get_address() = 0x100010000, len = 64K

  --> start_addr = addr = 0x100000000
      len = 256K

      rec->match() returns

      u_addr = 0x100010000, u_len = 64K

      commit_len = 64K - (0x100000000 - 0x100010000)
		 = 64K - 0x0xFFFFFFFFFFFF0000
		 = 128K

      Note: commit_len is size_t, so it's unsigned!

      if (commit_len (128K) > len (256K))?  Nope!

  --> VirtualAlloc (0x100000000, 128K, MEM_COMMIT, rec->gen_protect ());

  If this VirtualAlloc fails, the process is screwed and gets a
  well-deserved SIGBUS.

I'm not saying that there's a chance I'm missing something, but
as of now, it looks ok to me.

> The only thing I can come up with immediately is that we should sort the list
> of mmap_records in order of their starting addresses.  Then if start_addr ==
> u_addr, we commit u_len bytes, otherwise we fail.

I don't understand this one.  The code loops over all records,
so where's the problem?


Thanks,
Corinna