This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: munmap slowness; IsBadReadPtr considered harmful


On Wed, 25 Feb 2004, Corinna Vinschen wrote:
> On Feb 24 17:23, Brian Ford wrote:
> > I guess it all depends on your interpretation of the following lines from:
> >
> > http://www.opengroup.org/onlinepubs/007904975/functions/munmap.html
> >
> > ERRORS:
> >
> > [EINVAL]
> >     Addresses in the range [addr,addr+len) are outside the valid range for
> > the address space of a process.
> >
> > What does that *really* mean, especially in terms of
> > COMMIT/RESERVE/FREE and NOACCESS/GUARD?
> >
> That's true.  On Linux, calling munmap on an already munmap'd memory
> is no problem.  Even more interesting, regardless of the state of the
> page allocation, all addresses valid as virtual memory addresses of
> a process do not fail with munmap.
>
> Since NOACCESS is used to mark munmapped pages in Cygwin's mmap
> implementation, this would mean that munmapping an already munmapped
> page would fail on Cygwin when checking for NOACCESS.  Testing reveals
> that this is already the case right now so using IsBadReadPtr in this
> context is actually wrong.  Oh boy.
>
> I tested your first implementation of that function.  To say it in
> simple words, the function only checks if the addresses are in the
> valid virtual address space of the process.  I used it in the munmap
> context and munmap behaves like on Linux now, only failing for addresses
> outside the valid virtual address space of the process.
>
> What I did is this:  I stripped the function to the bare minimum and
> put it into miscfuncs.cc, called "check_invalid_virtual_addr".  Also
> munmap now calls check_invalid_virtual_addr instead of IsBadReadPtr
>
Great, thanks!  That should give a *big* performance boost to any app
that mmaps a large, or network based file but doesn't touch all of it.

One minor nit, though.  There is no problem with the current usage in
munmap because the address is garanteed to be page aligned.  However,
since this is supposed to be a generic function, that assumption could
cause you to not test the last page in a range on the rare occassion
where its attributes are different since:

lpAddress: This value is rounded down to the next page boundary.

2004-02-25  Brian Ford  <ford@vss.fsi.com>

        * miscfuncs.cc (check_invalid_virtual_addr): Assure the last page
	in the range is always tested.  Add appropriate const.

	* mmap.cc (mmap_record::aloc_fh): Remove unused static path_conf
	object.

I'm not as sure about the mmap.cc change, but it looks unused to me and
I've been running that way for a week or so without any problems.

-- 
Brian Ford
Senior Realtime Software Engineer
VITAL - Visual Simulation Systems
FlightSafety International
Phone: 314-551-8460
Fax:   314-551-8444

Attachment: munmap.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]