This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: allow read into untouched noreserve mappings
Appologies for the previous message being un-timely. We had a network
outage, and it got queued before your response.
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?).
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"?
> 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.
> Don't be surprised that I now used getpagesize() instead of
> getsystempagesize (). I mulled over this a while. The idea is that the
> application expects a page size of 64K, not 4K. So the functionality
> makes most sense if it assumes 64K pages, too. This also minimizes the
> number of necessary calls to mmap_is_attached_or_noreserve_page, which
> is a good thing, IMO.
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.
One minor nit though, this stuff could be moved after the check for an
empty mmap region list.
> 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.
--
Brian Ford
Lead Realtime Software Engineer
VITAL - Visual Simulation Systems
FlightSafety International
the best safety device in any aircraft is a well-trained crew...