Subject: Re: Wine & NetBSD?
To: Todd Vierling <tv@wasabisystems.com>
From: Bang Jun-Young <bjy@mogua.org>
List: tech-kern
Date: 11/21/2001 03:35:53
On Tue, Nov 20, 2001 at 11:23:00AM -0500, Todd Vierling wrote:
>    The format of the call is as follows:
> 
>    pa=mmap(addr, len, prot, flags, fildes, off);
>    [...]
> 
>    When MAP_FIXED is set in the flags argument, the implementation is
>    informed that the value of pa must be addr, exactly. If MAP_FIXED is set,
>    mmap() may return MAP_FAILED and set errno to [EINVAL]. If a MAP_FIXED
>    request is successful, the mapping established by mmap() replaces any
>    previous mappings for the process' pages in the range [pa, pa + len).
> 
>    When MAP_FIXED is not set, the implementation uses addr in an unspecified
>    manner to arrive at pa. The pa so chosen will be an area of the address

'unspecified' - we should be free to choose any address which is valid. 

>    space that the implementation deems suitable for a mapping of len bytes
>    to the file. All implementations interpret an addr value of 0 as granting
>    the implementation complete freedom in selecting pa, subject to
>    constraints described below. A non-zero value of addr is taken to be a
>    suggestion of a process address near which the mapping should be placed.
> 
>    When the implementation selects a value for pa, it never places a mapping
>    at address 0, nor does it replace any extant mapping.

The patch 100% satisfies the above citation, and _does_ better
than the current implementation. Take a look at it again:

+	else if (addr == 0 ||
+	    (addr >= round_page((vaddr_t)p->p_vmspace->vm_taddr) &&
+	     addr < round_page((vaddr_t)p->p_vmspace->vm_daddr + MAXDSIZ)))
+		addr = round_page((vaddr_t)p->p_vmspace->vm_daddr + MAXDSIZ);

NetBSD mmap doesn't check if the first argument is 0, nor does it
allow mapping to be established at lower (valid) address:

-		addr = MAX(addr, round_page((vaddr_t)p->p_vmspace->vm_daddr +
-					    MAXDSIZ));

This can be rewritten as follows:

	else if (addr < round_page((vaddr_t)p->p_vmspace->vm_daddr + MAXDSIZ)
		addr = round_page((vaddr_t)p->p_vmspace->vm_daddr + MAXDSIZE);

Now it's clearer, isn't it?

> 
> aCCOrding to this citation, NetBSD is 100% correct in its implementation of
> MAP_FIXED vs. non-MAP_FIXED.

Yes, but the patch is also correct. Which one would we choose?

Jun-Young

-- 
Bang Jun-Young <bjy@mogua.org>