From owner-freebsd-current  Mon Mar  1  6:56:47 1999
Delivered-To: freebsd-current@freebsd.org
Received: from godzilla.zeta.org.au (godzilla.zeta.org.au [203.26.10.9])
	by hub.freebsd.org (Postfix) with ESMTP id D6B5F15006
	for <current@FreeBSD.ORG>; Mon,  1 Mar 1999 06:56:42 -0800 (PST)
	(envelope-from bde@godzilla.zeta.org.au)
Received: (from bde@localhost)
	by godzilla.zeta.org.au (8.8.7/8.8.7) id BAA18860;
	Tue, 2 Mar 1999 01:56:19 +1100
Date: Tue, 2 Mar 1999 01:56:19 +1100
From: Bruce Evans <bde@zeta.org.au>
Message-Id: <199903011456.BAA18860@godzilla.zeta.org.au>
To: bde@zeta.org.au, dick@tar.com
Subject: Re: lockmgr panic with mmap()
Cc: current@FreeBSD.ORG, julian@whistle.com, pangolin@home.com
Sender: owner-freebsd-current@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

>> mincore() locks the vmspace map, and initialises vec[] a byte at a time
>> using subyte().  When vec[] is sufficiently large, it is not all in core
>> initially and a page fault occurs in subyte().  The new stack growing
>> code locks the vmspace map early and panics.
>
>It appears to me the potential for deadlock exists in the old grow code
>too, but is less likely.  
>
>I've attached a patch that addresses the issue, though there is a
>question if this is the best solution.
>
>With the old grow code, if the map lock is already held, one of the
>following will be true
>a) the grow code doesn't get executed because the va is not
>   in the stack region, which is equivalent to grow returning
>   success
>b) the grow code returns success because there's no need to
>   grow the region because the va is aready in the mapped region.
>c) the grow code returns false if the va is within the stack
>   region and there's no room left, which normally will result
>   in a SIGSEGV
>d) the grow code attempts to grow the stack via vm_map_find and
>   will then deadlock at the vm_map_lock call
>
>With this patch, if the map lock is already held, by always returning
>success, cases a) and b) will be the same as before.  For case c)
>the code will attempt to continue to execute, generally calling
>vm_fault, which should, I think, generate a SIGSEGV, essentially
>the same result as the old code, but via a different path.  Case d)
>will now continue executing in a manner equivalent to the new case c).
>
>The question is whether there is a way to do the autogrow function if
>the map lock is already held.

I think it shouldn't work.  Copying to and from user space must not
generate SIGSEGV if the application could access its memory directly.
The easiest way to implement this is to never lock maps before
accessing user memory.  Atomic copies won't get any easier to do
after you've backed out of a deadlock.

I think mincore() should just unlock the map while calling subyte() and
restart if the map changes significantly underneath it.  Some critical
examples:

1. mincore(addr, 0xfffffff0, vec) at rtprio 0 (in only process with this
   priority).  vec[] only needs to have size 1MB (for 4K pages).
   It's barely reasonable for the application to expect vec[] to be all
   mapped and to perfectly represent the current state.  To implement
   this, mincore() would have to prefault vec[] (or backtrack to give
   the equivalent) so that it doesn't block.

2. mincore(vec, len, vec).  This is quite likely to change the incore'ness
   of vec[].

Bruce


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message