stopping amd causes a freeze
Konstantin Belousov
kostikbel at gmail.com
Thu Jul 25 10:00:47 UTC 2013
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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-stable/attachments/20130725/b4b4343f/attachment.sig>
More information about the freebsd-stable
mailing list