From owner-freebsd-current@FreeBSD.ORG Wed Apr 28 19:24:03 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EE89D16A4CE; Wed, 28 Apr 2004 19:24:02 -0700 (PDT) Received: from cs.rice.edu (cs.rice.edu [128.42.1.30]) by mx1.FreeBSD.org (Postfix) with ESMTP id B0A0643D31; Wed, 28 Apr 2004 19:24:02 -0700 (PDT) (envelope-from alc@cs.rice.edu) Received: from localhost (calypso.cs.rice.edu [128.42.1.127]) by cs.rice.edu (Postfix) with ESMTP id 2B19F4ACBA; Wed, 28 Apr 2004 21:24:02 -0500 (CDT) Received: from cs.rice.edu ([128.42.1.30]) by localhost (calypso.cs.rice.edu [128.42.1.127]) (amavisd-new, port 10024) with LMTP id 07981-01-55; Wed, 28 Apr 2004 21:24:01 -0500 (CDT) Received: by cs.rice.edu (Postfix, from userid 19572) id AF53B4ACBD; Wed, 28 Apr 2004 21:24:01 -0500 (CDT) Date: Wed, 28 Apr 2004 21:24:01 -0500 From: Alan Cox To: Brian Fundakowski Feldman Message-ID: <20040429022401.GJ5199@cs.rice.edu> References: <200404290008.i3T08HaS004242@green.homeunix.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <200404290008.i3T08HaS004242@green.homeunix.org> User-Agent: Mutt/1.3.28i X-Virus-Scanned: by amavis-20030616-p7 at cs.rice.edu cc: alc@FreeBSD.org cc: bms@FreeBSD.org cc: current@FreeBSD.org Subject: Re: VM wiring fixed X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Apr 2004 02:24:03 -0000 On Wed, Apr 28, 2004 at 08:08:17PM -0400, Brian Fundakowski Feldman wrote: > There are several severe wiring bugs in -CURRENT that I believe I have fixed. > Please test/review as appropriate if you're affected by any of them. This > is a superset of the previous patch which just mostly-fixed mlockall(2). > > * MAP_FUTUREWIRE was not unset in vmspace_dofree(), causing the next process > to use that vmspace to wire all of its memory. Would setting flags to 0 in vmspace_alloc() accomplish the same? > * kmem_*() calls either called vm_map_wire() or set MAP_ENTRY_NOFAULT, but > kmem_free() did not undo the vm_map_wire() calls. I don't understand this comment. kmem_free() calls vm_map_remove(), which calls vm_map_delete(), which calls vm_map_entry_unwire(), etc. However, I don't see any change corresponding to this statement in your patch. > * vm_fault_unlock() could not unwire pages that were not in the pmap already, > leaking them permanently. By definition a wired mapping is in the pmap. A wired page can, however, be mapped by a non-wired mapping. Thus, a non-wired mapping of a wired page could be absent from the pmap. Problems such as PR/29915 are a result of vm_fault_unwire() not understanding fake pages created by the device pager. Pmap_extract() followed by PHYS_TO_VM_PAGE() does not result in a pointer to the fake page that was wired. Thus, the panic on unexpected wiring count. Your patch to vm_fault_unwire() does look like a correct fix to this issue. > * vm_map_{un,}wire() did not keep track of wirings as they should. User > wirings are separate from system wirings, and there can be exactly one. > There can be unlimited system wirings, but wired_count will remain at > zero for map entries that are allocated as MAP_ENTRY_NOFAULT. MAP_ENTRY_NOFAULT means that the vm_map_entry has no backing vm_object and that you should never fault on it, which would create a backing object. vm_map_{un,}wire(), however, deal with map entries that do have backing objects. I'm not quite sure why you chose to introduce MAP_ENTRY_NOFAULT into the implementation of vm_map_{un,}wire(). I'll have to look at this more carefully. > * vslock()/vsunlock() did not both use VM_MAP_WIRE_SYSTEM as they must; > I believe this resulted in more wiring leaks. > * vm_map_delete() did not wait for all wirings (except one user wiring) to > drain, so vslock() guaranteed nothing. It is not vm_map_delete()'s responsibility to wait for all wirings to be drained. It is only responsible for a single wired mapping. > * The condition in vm_fault() where all pages have been exhausted is easy to > deadlock, but was impossible to recover from. The OOM killer works only > when all memory has been used, not all wired memory. However, now it is > possible to kill offending processes with SIGKILL instead of the > vm_fault() in trap_pfault() looping forever. > * The init(8) program should really be using mlockall(2) so that it can kill > off processes hogging all the wired memory. However, I have not fixed this > because in such case, e.g. while I would like for Ctrl+Alt+Del to work, > init(8) may actually need to allocate and wire new pages itself to keep > running. I think a way to fix this is to conditionalize the > vm_page_count_severe() condition on p->p_pid != 1 so that just like the > REAL system processes, it can bring the page count lower than "severe". > > I haven't tested it out on SMP yet, but on UP the latest changes don't seem > to have any negative effects. All wired memory leaks appear to be gone and > although init(8) probably can't do it, I can enter DDB and "kill 9 " > to take the machine out of an all-wired deadlock. See patch at URL: > > In summary, consider this a positive review for the MAP_FUTUREWIRE and the vm_fault_unwire() fixes. Regards, Alan