On Sun, Sep 26, 2021 at 1:07 PM Guy Yur <guyyur@gmail.com> wrote:
On 26/9/21 20:08, Warner Losh wrote:
>
>
> On Fri, Sep 24, 2021 at 6:00 AM Richard Henderson
> <richard.henderson@linaro.org> wrote:
>
>     On 9/21/21 9:56 PM, Warner Losh wrote:
>     >           /* no page was there, so we allocate one */
>     >           void *p = mmap(host_start, qemu_host_page_size, prot,
>     > -                       flags | MAP_ANON, -1, 0);
>     > +                       flags | ((fd != -1) ? MAP_ANON : 0), -1, 0);
>
>     I don't understand this change, given that the actual fd passed is
>     always -1.
>
>
> That's a very good question. I'll have to trace down why that was made
> because
> I'm having trouble with it as well now that I'm trying to defend it.
>
mmap_frag can be called with a valid fd, if flags doesn't contain one of
MAP_ANON, MAP_STACK, MAP_GUARD.
The passed fd to mmap is -1 but if flags contains MAP_GUARD then
MAP_ANON cannot be added.

* If fd is valid (not -1) we want to map the pages with MAP_ANON.
* If flags contains MAP_GUARD we don't want to add MAP_ANON because it
will be rejected.
https://github.com/freebsd/freebsd-src/blob/master/sys/vm/vm_mmap.c#L302
* If flags contains MAP_ANON it doesn't matter if we add it or not.
* If flags contains MAP_STACK, mmap adds MAP_ANON when called so doesn't
matter if we add it or not either.
https://github.com/freebsd/freebsd-src/blob/master/sys/vm/vm_mmap.c#L284

The intention was to not pass MAP_ANON for the flags that use fd == -1
without specifying the flags directly,
with the assumption that future flags that don't require fd will also
not require MAP_ANON.
Changing to !(flags & MAP_GUARD) will also work.

Thanks Guy. that fills in the missing pieces for me of the range of possibilities
for calling. I've added it as a comment since it's tricky enough I've had to ask
twice and Richard asked as well :). It will be in the next spin of the mmap series.
 
Guy Yur

> Warner
>
>
>     r~
>