PR/52706 CVS commit: [netbsd-8] src/sys

["Martin Husemann" <martin%netbsd.org@localhost>]

The following reply was made to PR kern/52706; it has been noted by GNATS.

From: "Martin Husemann" <martin%netbsd.org@localhost>
To: gnats-bugs%gnats.NetBSD.org@localhost
Cc: 
Subject: PR/52706 CVS commit: [netbsd-8] src/sys
Date: Tue, 27 Feb 2018 09:07:34 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Tue Feb 27 09:07:33 UTC 2018
 
 Modified Files:
 	src/sys/arch/alpha/alpha [netbsd-8]: pmap.c
 	src/sys/arch/m68k/m68k [netbsd-8]: pmap_motorola.c
 	src/sys/arch/powerpc/oea [netbsd-8]: pmap.c
 	src/sys/arch/sparc64/sparc64 [netbsd-8]: pmap.c
 	src/sys/arch/x86/x86 [netbsd-8]: pmap.c
 	src/sys/dev/dtv [netbsd-8]: dtv_scatter.c
 	src/sys/dev/marvell [netbsd-8]: mvxpsec.c
 	src/sys/kern [netbsd-8]: subr_extent.c subr_pool.c uipc_mbuf.c
 	src/sys/opencrypto [netbsd-8]: crypto.c
 	src/sys/sys [netbsd-8]: mbuf.h pool.h
 	src/sys/ufs/chfs [netbsd-8]: chfs_malloc.c
 	src/sys/uvm [netbsd-8]: uvm_fault.c
 
 Log Message:
 Pull up following revision(s) (requested by mrg in ticket #593):
 	sys/dev/marvell/mvxpsec.c: revision 1.2
 	sys/arch/m68k/m68k/pmap_motorola.c: revision 1.70
 	sys/opencrypto/crypto.c: revision 1.102
 	sys/arch/sparc64/sparc64/pmap.c: revision 1.308
 	sys/ufs/chfs/chfs_malloc.c: revision 1.5
 	sys/arch/powerpc/oea/pmap.c: revision 1.95
 	sys/sys/pool.h: revision 1.80,1.82
 	sys/kern/subr_pool.c: revision 1.209-1.216,1.219-1.220
 	sys/arch/alpha/alpha/pmap.c: revision 1.262
 	sys/kern/uipc_mbuf.c: revision 1.173
 	sys/uvm/uvm_fault.c: revision 1.202
 	sys/sys/mbuf.h: revision 1.172
 	sys/kern/subr_extent.c: revision 1.86
 	sys/arch/x86/x86/pmap.c: revision 1.266 (via patch)
 	sys/dev/dtv/dtv_scatter.c: revision 1.4
 
 Allow only one pending call to a pool's backing allocator at a time.
 Candidate fix for problems with hanging after kva fragmentation related
 to PR kern/45718.
 
 Proposed on tech-kern:
 https://mail-index.NetBSD.org/tech-kern/2017/10/23/msg022472.html
 Tested by bouyer@ on i386.
 
 This makes one small change to the semantics of pool_prime and
 pool_setlowat: they may fail with EWOULDBLOCK instead of ENOMEM, if
 there is a pending call to the backing allocator in another thread but
 we are not actually out of memory.  That is unlikely because nearly
 always these are used during initialization, when the pool is not in
 use.
 
 Define the new flag too for previous commit.
 
 pool_grow can now fail even when sleeping is ok. Catch this case in pool_get
 and retry.
 
 Assert that pool_get failure happens only with PR_NOWAIT.
 This would have caught the mistake I made last week leading to null
 pointer dereferences all over the place, a mistake which I evidently
 poorly scheduled alongside maxv's change to the panic message on x86
 for null pointer dereferences.
 
 Since pr_lock is now used to wait for two things now (PR_GROWING and
 PR_WANTED) we need to loop for the condition we wanted.
 make the KASSERTMSG/panic strings consistent as '%s: [%s], __func__, wchan'
 Handle the ERESTART case from pool_grow()
 
 don't pass 0 to the pool flags
 Guess pool_cache_get(pc, 0) means PR_WAITOK here.
 Earlier on in the same context we use kmem_alloc(sz, KM_SLEEP).
 
 use PR_WAITOK everywhere.
 use PR_NOWAIT.
 
 Don't use 0 for PR_NOWAIT
 
 use PR_NOWAIT instead of 0
 
 panic ex nihilo -- PR_NOWAITing for zerot
 
 Add assertions that either PR_WAITOK or PR_NOWAIT are set.
 - fix an assert; we can reach there if we are nowait or limitfail.
 - when priming the pool and failing with ERESTART, don't decrement the number
   of pages; this avoids the issue of returning an ERESTART when we get to 0,
   and is more correct.
 - simplify the pool_grow code, and don't wakeup things if we ENOMEM.
 
 In pmap_enter_ma(), only try to allocate pves if we might need them,
 and even if that fails, only fail the operation if we later discover
 that we really do need them.  This implements the requirement that
 pmap_enter(PMAP_CANFAIL) must not fail when replacing an existing
 mapping with the first mapping of a new page, which is an unintended
 consequence of the changes from the rmind-uvmplock branch in 2011.
 
 The problem arises when pmap_enter(PMAP_CANFAIL) is used to replace an existing
 pmap mapping with a mapping of a different page (eg. to resolve a copy-on-write).
 If that fails and leaves the old pmap entry in place, then UVM won't hold
 the right locks when it eventually retries.  This entanglement of the UVM and
 pmap locking was done in rmind-uvmplock in order to improve performance,
 but it also means that the UVM state and pmap state need to be kept in sync
 more than they did before.  It would be possible to handle this in the UVM code
 instead of in the pmap code, but these pmap changes improve the handling of
 low memory situations in general, and handling this in UVM would be clunky,
 so this seemed like the better way to go.
 
 This somewhat indirectly fixes PR 52706, as well as the failing assertion
 about "uvm_page_locked_p(old_pg)".  (but only on x86, various other platforms
 will need their own changes to handle this issue.)
 In uvm_fault_upper_enter(), if pmap_enter(PMAP_CANFAIL) fails, assert that
 the pmap did not leave around a now-stale pmap mapping for an old page.
 If such a pmap mapping still existed after we unlocked the vm_map,
 the UVM code would not know later that it would need to lock the
 lower layer object while calling the pmap to remove or replace that
 stale pmap mapping.  See PR 52706 for further details.
 hopefully workaround the irregularly "fork fails in init" problem.
 if a pool is growing, and the grower is PR_NOWAIT, mark this.
 if another caller wants to grow the pool and is also PR_NOWAIT,
 busy-wait for the original caller, which should either succeed
 or hard-fail fairly quickly.
 
 implement the busy-wait by unlocking and relocking this pools
 mutex and returning ERESTART.  other methods (such as having
 the caller do this) were significantly more code and this hack
 is fairly localised.
 ok chs@ riastradh@
 
 Don't release the lock in the PR_NOWAIT allocation. Move flags setting
 after the acquiring the mutex. (from Tobias Nygren)
 apply the change from arch/x86/x86/pmap.c rev. 1.266 commitid vZRjvmxG7YTHLOfA:
 
 In pmap_enter_ma(), only try to allocate pves if we might need them,
 and even if that fails, only fail the operation if we later discover
 that we really do need them.  If we are replacing an existing mapping,
 reuse the pv structure where possible.
 
 This implements the requirement that pmap_enter(PMAP_CANFAIL) must not fail
 when replacing an existing mapping with the first mapping of a new page,
 which is an unintended consequence of the changes from the rmind-uvmplock
 branch in 2011.
 
 The problem arises when pmap_enter(PMAP_CANFAIL) is used to replace an existing
 pmap mapping with a mapping of a different page (eg. to resolve a copy-on-write).
 If that fails and leaves the old pmap entry in place, then UVM won't hold
 the right locks when it eventually retries.  This entanglement of the UVM and
 pmap locking was done in rmind-uvmplock in order to improve performance,
 but it also means that the UVM state and pmap state need to be kept in sync
 more than they did before.  It would be possible to handle this in the UVM code
 instead of in the pmap code, but these pmap changes improve the handling of
 low memory situations in general, and handling this in UVM would be clunky,
 so this seemed like the better way to go.
 
 This somewhat indirectly fixes PR 52706 on the remaining platforms where
 this problem existed.
 
 
 To generate a diff of this commit:
 cvs rdiff -u -r1.261 -r1.261.8.1 src/sys/arch/alpha/alpha/pmap.c
 cvs rdiff -u -r1.69 -r1.69.8.1 src/sys/arch/m68k/m68k/pmap_motorola.c
 cvs rdiff -u -r1.94 -r1.94.8.1 src/sys/arch/powerpc/oea/pmap.c
 cvs rdiff -u -r1.307 -r1.307.6.1 src/sys/arch/sparc64/sparc64/pmap.c
 cvs rdiff -u -r1.245.6.1 -r1.245.6.2 src/sys/arch/x86/x86/pmap.c
 cvs rdiff -u -r1.3 -r1.3.2.1 src/sys/dev/dtv/dtv_scatter.c
 cvs rdiff -u -r1.1 -r1.1.12.1 src/sys/dev/marvell/mvxpsec.c
 cvs rdiff -u -r1.80 -r1.80.8.1 src/sys/kern/subr_extent.c
 cvs rdiff -u -r1.207 -r1.207.6.1 src/sys/kern/subr_pool.c
 cvs rdiff -u -r1.172 -r1.172.6.1 src/sys/kern/uipc_mbuf.c
 cvs rdiff -u -r1.78.2.4 -r1.78.2.5 src/sys/opencrypto/crypto.c
 cvs rdiff -u -r1.170.2.1 -r1.170.2.2 src/sys/sys/mbuf.h
 cvs rdiff -u -r1.79 -r1.79.10.1 src/sys/sys/pool.h
 cvs rdiff -u -r1.4 -r1.4.30.1 src/sys/ufs/chfs/chfs_malloc.c
 cvs rdiff -u -r1.199.6.2 -r1.199.6.3 src/sys/uvm/uvm_fault.c
 
 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.