Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 25 Jul 2013 13:00:37 +0300
From:      Konstantin Belousov <kostikbel@gmail.com>
To:        Dominic Fandrey <kamikaze@bsdforen.de>
Cc:        freebsd-stable@freebsd.org
Subject:   Re: stopping amd causes a freeze
Message-ID:  <20130725100037.GM5991@kib.kiev.ua>
In-Reply-To: <51F0DA4B.3000809@bsdforen.de>
References:  <51ED0060.2050502@bsdforen.de> <20130722100720.GI5991@kib.kiev.ua> <51F0DA4B.3000809@bsdforen.de>

next in thread | previous in thread | raw e-mail | index | archive | help

--o0dA3wbhdegQN8Av
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

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.
> >>
> >> ...
> >=20
> > Are you sure that the machine did not paniced ?  Do you have serial con=
sole ?
> >=20
> > 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.
>=20
> 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.

>=20
> 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- kern/vfs_bio.c	(revision 253643)
+++ kern/vfs_bio.c	(working copy)
@@ -1614,7 +1614,8 @@ brelse(struct buf *bp)
 					(PAGE_SIZE - poffset) : resid;
=20
 				KASSERT(presid >=3D 0, ("brelse: extra page"));
-				vm_page_set_invalid(m, poffset, presid);
+				if (pmap_page_wired_mappings(m) =3D=3D 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- vm/vm_fault.c	(revision 253643)
+++ vm/vm_fault.c	(working copy)
@@ -286,6 +286,19 @@ RetryFault:;
 		    (u_long)vaddr);
 	}
=20
+	if (fs.entry->eflags & MAP_ENTRY_IN_TRANSITION &&
+	    fs.entry->wiring_thread !=3D 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 |=3D 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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 |=3D MAP_ENTRY_IN_TRANSITION;
+		entry->wiring_thread =3D 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 =3D first_entry;
-	while (entry !=3D &map->header && entry->start < end) {
+	for (entry =3D first_entry; entry !=3D &map->header && entry->start < end;
+	    entry =3D 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) =3D=3D 0 ||
+		    entry->wiring_thread !=3D curthread) {
+			KASSERT((flags & VM_MAP_WIRE_HOLESOK) !=3D 0,
+			    ("vm_map_unwire: !HOLESOK and new/changed entry"));
+			continue;
+		}
+
 		if (rv =3D=3D KERN_SUCCESS && (!user_unwire ||
 		    (entry->eflags & MAP_ENTRY_USER_WIRED))) {
 			if (user_unwire)
@@ -2321,15 +2338,15 @@ done:
 				    entry->object.vm_object->type =3D=3D OBJT_SG));
 			}
 		}
-		KASSERT(entry->eflags & MAP_ENTRY_IN_TRANSITION,
-			("vm_map_unwire: in-transition flag missing"));
+		KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) !=3D 0,
+		    ("vm_map_unwire: in-transition flag missing"));
 		entry->eflags &=3D ~MAP_ENTRY_IN_TRANSITION;
+		entry->wiring_thread =3D NULL;
 		if (entry->eflags & MAP_ENTRY_NEEDS_WAKEUP) {
 			entry->eflags &=3D ~MAP_ENTRY_NEEDS_WAKEUP;
 			need_wakeup =3D TRUE;
 		}
 		vm_map_simplify_entry(map, entry);
-		entry =3D 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 |=3D MAP_ENTRY_IN_TRANSITION;
+		entry->wiring_thread =3D curthread;
 		if ((entry->protection & (VM_PROT_READ | VM_PROT_EXECUTE)) =3D=3D 0
 		    || (entry->protection & prot) !=3D prot) {
 			entry->eflags |=3D MAP_ENTRY_WIRE_SKIPPED;
@@ -2514,10 +2532,27 @@ done:
 		else
 			KASSERT(result, ("vm_map_wire: lookup failed"));
 	}
-	entry =3D first_entry;
-	while (entry !=3D &map->header && entry->start < end) {
+	for (entry =3D first_entry; entry !=3D &map->header && entry->start < end;
+	    entry =3D entry->next) {
 		if ((entry->eflags & MAP_ENTRY_WIRE_SKIPPED) !=3D 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) =3D=3D 0 ||
+		    entry->wiring_thread !=3D curthread) {
+			KASSERT((flags & VM_MAP_WIRE_HOLESOK) !=3D 0,
+			    ("vm_map_wire: !HOLESOK and new/changed entry"));
+			continue;
+		}
+
 		if (rv =3D=3D KERN_SUCCESS) {
 			if (user_wire)
 				entry->eflags |=3D 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 &=3D ~(MAP_ENTRY_IN_TRANSITION|MAP_ENTRY_WIRE_SKIPPED);
+		KASSERT((entry->eflags & MAP_ENTRY_IN_TRANSITION) !=3D 0,
+		    ("vm_map_wire: in-transition flag missing %p", entry));
+		KASSERT(entry->wiring_thread =3D=3D curthread,
+		    ("vm_map_wire: alien wire %p", entry));
+		entry->eflags &=3D ~(MAP_ENTRY_IN_TRANSITION |
+		    MAP_ENTRY_WIRE_SKIPPED);
+		entry->wiring_thread =3D NULL;
 		if (entry->eflags & MAP_ENTRY_NEEDS_WAKEUP) {
 			entry->eflags &=3D ~MAP_ENTRY_NEEDS_WAKEUP;
 			need_wakeup =3D TRUE;
 		}
 		vm_map_simplify_entry(map, entry);
-		entry =3D entry->next;
 	}
 	vm_map_unlock(map);
 	if (need_wakeup)
@@ -3185,6 +3223,7 @@ vmspace_fork(struct vmspace *vm1, vm_ooffset_t *fo
 			*new_entry =3D *old_entry;
 			new_entry->eflags &=3D ~(MAP_ENTRY_USER_WIRED |
 			    MAP_ENTRY_IN_TRANSITION);
+			new_entry->wiring_thread =3D NULL;
 			new_entry->wired_count =3D 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 &=3D ~(MAP_ENTRY_USER_WIRED |
 			    MAP_ENTRY_IN_TRANSITION | MAP_ENTRY_VN_WRITECNT);
+			new_entry->wiring_thread =3D NULL;
 			new_entry->wired_count =3D 0;
 			new_entry->object.vm_object =3D NULL;
 			new_entry->cred =3D NULL;
Index: vm/vm_map.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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 =3D 0 */
 	vm_pindex_t next_read;		/* index of the next sequential read */
 	struct ucred *cred;		/* tmp storage for creator ref */
+	struct thread *wiring_thread;
 };
=20
 #define MAP_ENTRY_NOSYNC		0x0001
Index: vm/vm_object.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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 =3D OBJPR_NOTMAPPED;
 		else if (old_msync)
-			flags =3D 0;
+			flags =3D OBJPR_NOTWIRED;
 		else
-			flags =3D OBJPR_CLEANONLY;
+			flags =3D 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 =3D p->wire_count) !=3D 0 &&
 		    (wirings =3D pmap_page_wired_mappings(p)) !=3D p->wire_count) {
-			if ((options & OBJPR_NOTMAPPED) =3D=3D 0) {
+			if ((options & (OBJPR_NOTWIRED | OBJPR_NOTMAPPED)) =3D=3D
+			    0) {
 				pmap_remove_all(p);
 				/* Account for removal of wired mappings. */
 				if (wirings !=3D 0)
@@ -1876,8 +1877,7 @@ again:
 				p->valid =3D 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) !=3D 0 && p->valid !=3D 0) {
 			if ((options & OBJPR_NOTMAPPED) =3D=3D 0)
 				pmap_remove_write(p);
-			if (p->dirty) {
-				vm_page_unlock(p);
-				continue;
-			}
+			if (p->dirty)
+				goto next;
 		}
 		if ((options & OBJPR_NOTMAPPED) =3D=3D 0) {
+			if ((options & OBJPR_NOTWIRED) !=3D 0 && wirings !=3D 0)
+				goto next;
 			pmap_remove_all(p);
 			/* Account for removal of wired mappings. */
 			if (wirings !=3D 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
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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. */
=20
 TAILQ_HEAD(object_q, vm_object);
=20
Index: vm/vm_page.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- 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;
=20
 	VM_OBJECT_LOCK_ASSERT(m->object, MA_OWNED);
-	KASSERT((m->oflags & VPO_BUSY) =3D=3D 0,
-	    ("vm_page_set_invalid: page %p is busy", m));
 	bits =3D vm_page_bits(base, size);
 	if (m->valid =3D=3D VM_PAGE_BITS_ALL && bits !=3D 0)
 		pmap_remove_all(m);
Index: .
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
--- .	(revision 253643)
+++ .	(working copy)

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

--o0dA3wbhdegQN8Av
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (FreeBSD)

iQIcBAEBAgAGBQJR8PdEAAoJEJDCuSvBvK1B1HkP/00UJGRIcAGjHQNJehkq9bSQ
DI0KClTl+1Jz2OkoObz2DeVIKlX0v3wKZYIhvKdHIxjUx3YbttqXr5a7MACQbPWZ
NFN0OoiNB9rSNxQogZzxWOKVeepegyTOaXmT5k1B+Swv8zdxVwKnbb4AHoD9+7B1
5uOqpCTruYvLGTHqab4/xIMkvvw4bzcWBiXfX3+dXFQIgtf+yVs9iCmhkKyPyotf
l/hmXuFWvCe8Mq+7RiCWbbeXjYlGN7wt9Vl5thTVQDYrX04/8deae5jeOuRWWRtF
j6oYSqI9o4xojFclZLGSJG/kbEDTBOLEAXjjNSn5OLm3/PSUu/uRpdWqsEZzPkCt
wJRI5nlylCKLbnQN8gUqYojnr8IOu/PcHtWYtvB0n7/aeGxeZNkSL93+148CS5YB
fBVRZ86goo95r9XcafN3wRmh60555sisshtniLExtE6b39mfNY3ckf7FCOCn/KKb
sThwGT/svinjO2HED4aGQZ0XYT6aTucV6TR+TLFgAetIXgcNa/DVWiDjEFWs+8BJ
R+aq3zgAVdyEwPhzew9+Zn8yh1n7D7sZ6uihq4DbjwokwJiVxRPAeVKqJkeECvFj
Y2EUPjfF+IAHjZyENMC8R3bD8RwJoKxnG1dEflQ3OsgBieglEaYOFbW4uBpPsyQi
z6qywP+JCtVyAiYsxQnM
=sxJQ
-----END PGP SIGNATURE-----

--o0dA3wbhdegQN8Av--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130725100037.GM5991>