On Thu, Jul 25, 2013 at 09:56:59AM +0200, Dominic Fandrey wrote:
> On 22/07/2013 12:07, Konstantin Belousov wrote:
> > On Mon, Jul 22, 2013 at 11:50:24AM +0200, Dominic Fandrey wrote:
> >> ...
> >>
> >> I run amd through sysutils/automounter, which is a scripting solution
> >> that generates an amd.map file based on encountered devices and devd
> >> events. The SIGHUP it sends to amd to tell it the map file was updated
> >> does not cause problems, only a -SIGKILL- SIGTERM may cause the freeze.
> >>
> >> Nothing was mounted (by amd) during the last freeze.
> >>
> >> ...
> > 
> > Are you sure that the machine did not paniced ?  Do you have serial console 
> > ?
> > 
> > The amd(8) locks itself into memory, most likely due to the fear of
> > deadlock. There are some known issues with user wirings in stable/9.
> > If the problem you see is indeed due to wiring, you might try to apply
> > r253187-r253191.
> 
> I tried that. Applying the diff was straightforward enough. But the
> resulting kernel paniced as soon as it tried to mount the root fs.
You did provided a useful info to diagnose the issue.

Patch should keep KBI compatible, but, just in case, if you have any
third-party module, rebuild it.

> 
> So I'll wait for the MFC from someone who knows what he/she is doing.

Patch below booted for me, and I run some sanity check tests for the
mlockall(2), which also did not resulted in misbehaviour.

Index: kern/vfs_bio.c
===================================================================
--- kern/vfs_bio.c      (revision 253643)
+++ kern/vfs_bio.c      (working copy)
@@ -1614,7 +1614,8 @@ brelse(struct buf *bp)
                                        (PAGE_SIZE - poffset) : resid;
 
                                KASSERT(presid >= 0, ("brelse: extra page"));
-                               vm_page_set_invalid(m, poffset, presid);
+                               if (pmap_page_wired_mappings(m) == 0)
+                                       vm_page_set_invalid(m, poffset, presid);
                                if (had_bogus)
                                        printf("avoided corruption bug in 
bogus_page/brelse code\n");
                        }
Index: vm/vm_fault.c
===================================================================
--- vm/vm_fault.c       (revision 253643)
+++ vm/vm_fault.c       (working copy)
@@ -286,6 +286,19 @@ RetryFault:;
                    (u_long)vaddr);
        }
 
+       if (fs.entry->eflags & MAP_ENTRY_IN_TRANSITION &&
+           fs.entry->wiring_thread != curthread) {
+               vm_map_unlock_read(fs.map);
+               vm_map_lock(fs.map);
+               if (vm_map_lookup_entry(fs.map, vaddr, &fs.entry) &&
+                   (fs.entry->eflags & MAP_ENTRY_IN_TRANSITION)) {
+                       fs.entry->eflags |= MAP_ENTRY_NEEDS_WAKEUP;
+                       vm_map_unlock_and_wait(fs.map, 0);
+               } else
+                       vm_map_unlock(fs.map);
+               goto RetryFault;
+       }
+
        /*
         * Make a reference to this object to prevent its disposal while we
         * are messing with it.  Once we have the reference, the map is free
Index: vm/vm_map.c
===================================================================
--- vm/vm_map.c (revision 253643)
+++ vm/vm_map.c (working copy)
@@ -2272,6 +2272,7 @@ vm_map_unwire(vm_map_t map, vm_offset_t start, vm_
                 * above.)
                 */
                entry->eflags |= MAP_ENTRY_IN_TRANSITION;
+               entry->wiring_thread = curthread;
                /*
                 * Check the map for holes in the specified region.
                 * If VM_MAP_WIRE_HOLESOK was specified, skip this check.
@@ -2304,8 +2305,24 @@ done:
                else
                        KASSERT(result, ("vm_map_unwire: lookup failed"));
        }
-       entry = first_entry;
-       while (entry != &map->header && entry->start < end) {
+       for (entry = first_entry; entry != &map->header && entry->start < end;
+           entry = entry->next) {
+               /*
+                * If VM_MAP_WIRE_HOLESOK was specified, an empty
+                * space in the unwired region could have been mapped
+                * while the map lock was dropped for draining
+                * MAP_ENTRY_IN_TRANSITION.  Moreover, another thread
+                * could be simultaneously wiring this new mapping
+                * entry.  Detect these cases and skip any entries
+                * marked as in transition by us.
+                */
+               if ((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 ||
+                   entry->wiring_thread != curthread) {
+                       KASSERT((flags & VM_MAP_WIRE_HOLESOK) != 0,
+                           ("vm_map_unwire: !HOLESOK and new/changed entry"));
+                       continue;
+               }
+
                if (rv == KERN_SUCCESS && (!user_unwire ||
                    (entry->eflags & MAP_ENTRY_USER_WIRED))) {
                        if (user_unwire)
@@ -2321,15 +2338,15 @@ done:
                                    entry->object.vm_object->type == OBJT_SG));
                        }
                }
-               KASSERT(entry->eflags & MAP_ENTRY_IN_TRANSITION,
-                       ("vm_map_unwire: in-transition flag missing"));
+               KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) != 0,
+                   ("vm_map_unwire: in-transition flag missing"));
                entry->eflags &= ~MAP_ENTRY_IN_TRANSITION;
+               entry->wiring_thread = NULL;
                if (entry->eflags & MAP_ENTRY_NEEDS_WAKEUP) {
                        entry->eflags &= ~MAP_ENTRY_NEEDS_WAKEUP;
                        need_wakeup = TRUE;
                }
                vm_map_simplify_entry(map, entry);
-               entry = entry->next;
        }
        vm_map_unlock(map);
        if (need_wakeup)
@@ -2423,6 +2440,7 @@ vm_map_wire(vm_map_t map, vm_offset_t start, vm_of
                 * above.)
                 */
                entry->eflags |= MAP_ENTRY_IN_TRANSITION;
+               entry->wiring_thread = curthread;
                if ((entry->protection & (VM_PROT_READ | VM_PROT_EXECUTE)) == 0
                    || (entry->protection & prot) != prot) {
                        entry->eflags |= MAP_ENTRY_WIRE_SKIPPED;
@@ -2514,10 +2532,27 @@ done:
                else
                        KASSERT(result, ("vm_map_wire: lookup failed"));
        }
-       entry = first_entry;
-       while (entry != &map->header && entry->start < end) {
+       for (entry = first_entry; entry != &map->header && entry->start < end;
+           entry = entry->next) {
                if ((entry->eflags & MAP_ENTRY_WIRE_SKIPPED) != 0)
                        goto next_entry_done;
+
+               /*
+                * If VM_MAP_WIRE_HOLESOK was specified, an empty
+                * space in the unwired region could have been mapped
+                * while the map lock was dropped for faulting in the
+                * pages or draining MAP_ENTRY_IN_TRANSITION.
+                * Moreover, another thread could be simultaneously
+                * wiring this new mapping entry.  Detect these cases
+                * and skip any entries marked as in transition by us.
+                */
+               if ((entry->eflags & MAP_ENTRY_IN_TRANSITION) == 0 ||
+                   entry->wiring_thread != curthread) {
+                       KASSERT((flags & VM_MAP_WIRE_HOLESOK) != 0,
+                           ("vm_map_wire: !HOLESOK and new/changed entry"));
+                       continue;
+               }
+
                if (rv == KERN_SUCCESS) {
                        if (user_wire)
                                entry->eflags |= MAP_ENTRY_USER_WIRED;
@@ -2542,15 +2577,18 @@ done:
                        }
                }
        next_entry_done:
-               KASSERT(entry->eflags & MAP_ENTRY_IN_TRANSITION,
-                       ("vm_map_wire: in-transition flag missing"));
-               entry->eflags &= 
~(MAP_ENTRY_IN_TRANSITION|MAP_ENTRY_WIRE_SKIPPED);
+               KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) != 0,
+                   ("vm_map_wire: in-transition flag missing %p", entry));
+               KASSERT(entry->wiring_thread == curthread,
+                   ("vm_map_wire: alien wire %p", entry));
+               entry->eflags &= ~(MAP_ENTRY_IN_TRANSITION |
+                   MAP_ENTRY_WIRE_SKIPPED);
+               entry->wiring_thread = NULL;
                if (entry->eflags & MAP_ENTRY_NEEDS_WAKEUP) {
                        entry->eflags &= ~MAP_ENTRY_NEEDS_WAKEUP;
                        need_wakeup = TRUE;
                }
                vm_map_simplify_entry(map, entry);
-               entry = entry->next;
        }
        vm_map_unlock(map);
        if (need_wakeup)
@@ -3185,6 +3223,7 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fo
                        *new_entry = *old_entry;
                        new_entry->eflags &= ~(MAP_ENTRY_USER_WIRED |
                            MAP_ENTRY_IN_TRANSITION);
+                       new_entry->wiring_thread = NULL;
                        new_entry->wired_count = 0;
                        if (new_entry->eflags & MAP_ENTRY_VN_WRITECNT) {
                                vnode_pager_update_writecount(object,
@@ -3219,6 +3258,7 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fo
                         */
                        new_entry->eflags &= ~(MAP_ENTRY_USER_WIRED |
                            MAP_ENTRY_IN_TRANSITION | MAP_ENTRY_VN_WRITECNT);
+                       new_entry->wiring_thread = NULL;
                        new_entry->wired_count = 0;
                        new_entry->object.vm_object = NULL;
                        new_entry->cred = NULL;
Index: vm/vm_map.h
===================================================================
--- vm/vm_map.h (revision 253643)
+++ vm/vm_map.h (working copy)
@@ -116,6 +116,7 @@ struct vm_map_entry {
        int wired_count;                /* can be paged if = 0 */
        vm_pindex_t next_read;          /* index of the next sequential read */
        struct ucred *cred;             /* tmp storage for creator ref */
+       struct thread *wiring_thread;
 };
 
 #define MAP_ENTRY_NOSYNC               0x0001
Index: vm/vm_object.c
===================================================================
--- vm/vm_object.c      (revision 253643)
+++ vm/vm_object.c      (working copy)
@@ -1033,9 +1033,9 @@ vm_object_sync(vm_object_t object, vm_ooffset_t of
                         */
                        flags = OBJPR_NOTMAPPED;
                else if (old_msync)
-                       flags = 0;
+                       flags = OBJPR_NOTWIRED;
                else
-                       flags = OBJPR_CLEANONLY;
+                       flags = OBJPR_CLEANONLY | OBJPR_NOTWIRED;
                vm_object_page_remove(object, OFF_TO_IDX(offset),
                    OFF_TO_IDX(offset + size + PAGE_MASK), flags);
        }
@@ -1866,7 +1866,8 @@ again:
                vm_page_lock(p);
                if ((wirings = p->wire_count) != 0 &&
                    (wirings = pmap_page_wired_mappings(p)) != p->wire_count) {
-                       if ((options & OBJPR_NOTMAPPED) == 0) {
+                       if ((options & (OBJPR_NOTWIRED | OBJPR_NOTMAPPED)) ==
+                           0) {
                                pmap_remove_all(p);
                                /* Account for removal of wired mappings. */
                                if (wirings != 0)
@@ -1876,8 +1877,7 @@ again:
                                p->valid = 0;
                                vm_page_undirty(p);
                        }
-                       vm_page_unlock(p);
-                       continue;
+                       goto next;
                }
                if (vm_page_sleep_if_busy(p, TRUE, "vmopar"))
                        goto again;
@@ -1886,12 +1886,12 @@ again:
                if ((options & OBJPR_CLEANONLY) != 0 && p->valid != 0) {
                        if ((options & OBJPR_NOTMAPPED) == 0)
                                pmap_remove_write(p);
-                       if (p->dirty) {
-                               vm_page_unlock(p);
-                               continue;
-                       }
+                       if (p->dirty)
+                               goto next;
                }
                if ((options & OBJPR_NOTMAPPED) == 0) {
+                       if ((options & OBJPR_NOTWIRED) != 0 && wirings != 0)
+                               goto next;
                        pmap_remove_all(p);
                        /* Account for removal of wired mappings. */
                        if (wirings != 0) {
@@ -1903,6 +1903,7 @@ again:
                        }
                }
                vm_page_free(p);
+next:
                vm_page_unlock(p);
        }
        vm_object_pip_wakeup(object);
Index: vm/vm_object.h
===================================================================
--- vm/vm_object.h      (revision 253643)
+++ vm/vm_object.h      (working copy)
@@ -176,6 +176,7 @@ struct vm_object {
  */
 #define        OBJPR_CLEANONLY 0x1             /* Don't remove dirty pages. */
 #define        OBJPR_NOTMAPPED 0x2             /* Don't unmap pages. */
+#define        OBJPR_NOTWIRED  0x4             /* Don't remove wired pages. */
 
 TAILQ_HEAD(object_q, vm_object);
 
Index: vm/vm_page.c
===================================================================
--- vm/vm_page.c        (revision 253643)
+++ vm/vm_page.c        (working copy)
@@ -2639,8 +2639,6 @@ vm_page_set_invalid(vm_page_t m, int base, int siz
        vm_page_bits_t bits;
 
        VM_OBJECT_LOCK_ASSERT(m->object, MA_OWNED);
-       KASSERT((m->oflags & VPO_BUSY) == 0,
-           ("vm_page_set_invalid: page %p is busy", m));
        bits = vm_page_bits(base, size);
        if (m->valid == VM_PAGE_BITS_ALL && bits != 0)
                pmap_remove_all(m);
Index: .
===================================================================
--- .   (revision 253643)
+++ .   (working copy)

Property changes on: .
___________________________________________________________________
Modified: svn:mergeinfo
   Merged /head/sys:r253187-253191

Attachment: pgpqsSEjIW170.pgp
Description: PGP signature

Reply via email to