Unnecessary limitation on munmap?

[Ken Brown]

Hi Mark,

On 12/21/2024 5:15 PM, Mark Geisert wrote:
> Hi Ken,
> 
> On 12/21/2024 1:42 PM, Ken Brown wrote:
>> I'm wondering why munmap insists on operating with chunks of size 64k 
>> instead of pages of size 4k.  In other words, what would go wrong if 
>> we did the following:
>>
>> --- a/winsup/cygwin/mm/mmap.cc
>> +++ b/winsup/cygwin/mm/mmap.cc
>> @@ -1143,7 +1143,7 @@ munmap (void *addr, size_t len)
>>         set_errno (EINVAL);
>>         return -1;
>>       }
>> -  const size_t pagesize = wincap.allocation_granularity ();
>> +  const size_t pagesize = wincap.page_size ();
>>     if (((uintptr_t) addr % pagesize) || !len)
>>       {
>>         set_errno (EINVAL);
>>
>> I'm currently testing a build with this patch, and so far I haven't 
>> seen any problems.  But maybe I don't know what to test.
> 
> I'm afraid I don't remember the circumstances, but the change from 4K 
> pages to 64K was done somewhat recently.  In the last 2-3 years IIRC.
> 
> Maybe check the git history of mmap.cc for clues?  Some corner case of 
> how Windows manages page protection wasn't allowing the desired POSIX 
> and/or Linux behavior?
Good suggestion.  The following commit changed getpagesize() to return 
64k instead of 4k [and it makes the opposite change for 
getsystempagesize()], but the commit message doesn't say why:

commit 1656b946379942d5a7d78afdaf9786b78b7b618b
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Tue Jan 9 15:46:41 2007 +0000

             * syscalls.cc (getpagesize): Change condition for clarity.
             (getsystempagesize): Ditto.

And munmap used getpagesize() to determine its page size for a long 
time, which means that the munmap page size changed from 4k to 64k after 
the above commit.  I don't know the rationale for that change.  Maybe 
the point is that a well-behaved application should call getpagesize() 
or sysconf() before calling munmap, since POSIX says the following about 
munmap: "The implementation may require that addr be a multiple of the 
page size as returned by sysconf()."  Notice that it says "may", so 
Cygwin could still be POSIX compliant if munmap went back to using 4k 
chunks, as in the old days.

Ken