Re: VM wiring fixed

From: Alan Cox (alc_at_cs.rice.edu)
Date: 04/29/04

  • Next message: Brian Fundakowski Feldman: "Re: VM wiring fixed" 
    Date: Wed, 28 Apr 2004 21:24:01 -0500
To: Brian Fundakowski Feldman <green@FreeBSD.org>

    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 <wirehog>"
    > to take the machine out of an all-wired deadlock. See patch at URL:
    > <http://green.homeunix.org/~green/vm-wiring.patch>
    >

    In summary, consider this a positive review for the MAP_FUTUREWIRE and
    the vm_fault_unwire() fixes.

    Regards,
    Alan
    _______________________________________________
    freebsd-current@freebsd.org mailing list
    http://lists.freebsd.org/mailman/listinfo/freebsd-current
    To unsubscribe, send any mail to "freebsd-current-unsubscribe@freebsd.org"

  • Next message: Brian Fundakowski Feldman: "Re: VM wiring fixed"