Re: mmap failure [was: cygrunsrv + sshd + rsync = 20 times too slow -- throttled?]

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

On Sep  6 21:34, Brian Inglis wrote:
> On 2021-09-06 15:24, Ken Brown via Cygwin wrote:
> > On 9/6/2021 4:54 PM, Peter Dons Tychsen wrote:
> > > Hi there,
> > > 
> > > On Mon, 2021-09-06 at 14:40 -0400, Ken Brown via Cygwin wrote:
> > > > > No, wait.  I get what you say.  The optimzation settings of the test
> > > > > case should have no influence on the code inside the DLL.  That
> > > > > doesn't
> > > > > make sense for sure.  However, I ran the testcase under GDB, I could
> > > > > reproduce the issue, and I could fix it by setting mmap_ext.Reserved
> > > > > = 0;
> > > > > Go figure!
> > > > 
> > > > I don't get it, but I can confirm that the problem is fixed.
> > > 
> > > That sounds a bit like a voodoo fix, that could quickly regress again.
> > > 
> > > Here is my 2 cents:
> > > 
> > > Currently the mmap_ext structure is setup like this:
> > > 
> > >   215       MEM_EXTENDED_PARAMETER mmap_ext = {
> > >   216         .Type = MemExtendedParameterAddressRequirements,
> > >   217         .Pointer = (PVOID) &mmap_req
> > >   218       };
> > > 
> > > This means that all other entries in the struct are zero at
> > > initialization as described here:
> > > https://en.cppreference.com/w/c/language/struct_initialization
> > > 
> > > So if you set "mmap_ext.Reserved = 0" again after that its a double
> > > failure.
> > 
> > You're looking at the wrong source code.  The bug didn't occur until the
> > code was changed to do the following:
> > 
> >        /* g++ 11.2 workaround: don't use initializer */
> >        MEM_EXTENDED_PARAMETER mmap_ext;
> >        mmap_ext.Type = MemExtendedParameterAddressRequirements;
> >        mmap_ext.Pointer = (PVOID) &mmap_req;
> > 
> > This left mmap_ext.Reserved uninitialized, which Corinna has now fixed.
> 
> With undocumented structure member initialization an issue, maybe better to
> future proof using e.g.
> 
> 	MEM_EXTENDED_PARAMETER mmap_ext = { 0 }; // or memset or bzero
> 	...

You're right.  The bits in Reserved are just that, reserved.  So MSFT
may decide to use some of the  bits for other purposes, should the need
arise.  I've fixed that in git master.


Thanks,
Corinna