allow read into untouched noreserve mappings

Corinna Vinschen
Tue Jul 18 20:12:00 GMT 2006

On Jul 18 11:59, Brian Ford wrote:
> On Tue, 18 Jul 2006, Corinna Vinschen wrote:
> > I applied your patch to the cv-branch with some changes.  The way you
> > are calling search_record (see there)
> >
> > > +      long record_idx = map_list->search_record ((caddr_t)addr, 1,
> > > +						 u_addr, u_len, -1);
> >
> > always returns a u_len of 1.  The result is that for each page in
> > memory, the loop runs 4096 times in the worst case.
> I see that now :-(.
> I confess to not understanding the purpose and inner workings of either
> search_record implementation.  I first tried the two parameter version,
> but then realized it was searching through file offsets rather than map
> addresses (what is that usefull for?).

The answer is in list::try_map.  It tries to find a suitable, unused
record which can be reused for another mapping.  The idea here is to
accomodate old, non-standard implementations which assume that two
consecutive mappings are using consecutive pages in memory.  An example
are old autoconf tests.  This was more of a problem when getpagesize()
was 4K.  Today it will only be used on 9x due to the alignment bug I'm
referring to in mmap64.

> What I wanted was a function that returned the mmap record for an
> arbitrary address [range].  That seems pretty basic.  Why is there not
> such a thing?
> > I added the necessary alignment stuff
> See, I don't understand why every caller should need to do "the necessary
> alignment stuff"?

It's simply working as designed.  Did I claim somewhere that the code is
perfect?  I don't think so.  If you have a better or more elegant
solution, provide a patch.  It's as easy as that.

> > and minimized the number of calls to VirtualAlloc.
> Yeah, that was the concept I was going for.  Find the map that this
> address belongs to, commit the smaller of the address range for the region
> or the map, repeat while there is still an uncommitted address range.

You don't have to apologize.  You did that, just the search_record call
was incorrect.  To find this requires some debugging, that's all.

> > Don't be surprised that I now used getpagesize() instead of
> > getsystempagesize ().  [...]
> I know this dichotomy has been discussed at length before and there
> doesn't seem to be any win-win compromise.  I'm not sure if I agree or not
> *shrug*, but my opinion doesn't matter much anyway.

You know the problem of page size vs. allocation granularity, right?
I was fighting for keeping the page size in Cygwin at 4K for a long time
but at one point it got just too awkward to support it any longer,
so I gave up.  There's really no reason to go through this once again.

> One minor nit though, this stuff could be moved after the check for an
> empty mmap region list.

Indeed.  But, hey, this is the cygwin-patches list.  Just provide a

> > Thanks for the patch.  It's available for further digestion and patches
> > in the cv-branch.
> I guess it'll be a while then before this hits a release then :-(.  Thanks
> for applying anyway.

Sure.  The branch will be folded back into the main line after 1.5.21
has been released which will be very soon.


Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Project Co-Leader          cygwin AT cygwin DOT com
Red Hat

More information about the Cygwin-patches mailing list