On Mon, Feb 18, 2008 at 11:35:31PM +0100, Michiel Boland wrote:
quoted text
> >Hi. After recent upgrade (from 21 dec to today's src) the kernel crashes=
=20
quoted text
> >when starting X with
> >
> >panic: pmap_remove_all: page 0xc56e07f8 is fictitious
>=20
> FWIW below is a trivial program to re-create a similar crash. Needs root,=
=20
quoted text
> obviously. But still shouldn't cause a panic though. Note that the trick=
=20
quoted text
> in the program is that we mmap two pages, then munmap only half of them.
>=20
> #include <sys/types.h>
> #include <sys/mman.h>
> #include <fcntl.h>
> #include <stdio.h>
> #include <unistd.h>
>=20
> static const off_t map_address =3D 0xa0000;
> static const size_t map_size =3D 0x1000;
>=20
> static int testit(int fd)
> {
> 	void *p;
> 	int rv;
>=20
> 	p =3D mmap(NULL, 2 * map_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
> 		  map_address);
> 	if (p =3D=3D MAP_FAILED) {
> 		perror("mmap");
> 		return -1;
> 	}
> 	rv =3D *(char *) p;
> 	if (munmap(p, map_size) =3D=3D -1) {
> 		perror("munmap");
> 		return -1;
> 	}
> 	return rv;
> }
>=20
> int main(void)
> {
> 	int fd, rv;
>=20
> 	fd =3D open("/dev/mem", O_RDWR);
> 	if (fd =3D=3D -1) {
> 		perror("open");
> 		return 1;
> 	}
> 	rv =3D testit(fd);
> 	close(fd);
> 	return rv;
> }

What happen there is that munmap() do the split for the /dev/mem mapping.
This caused the OBJT_DEVICE ref_count to be bumped, and vm_map_entry_delete=
()
called vm_object_page_remove(). The later called pmap_remove_all()
unconditionally.

pmap_remove_all has the KASSERT that fails exactly when supplied
fictitious page. It becomes KASSERT in the rev. 1.106 of i386/pmap.c,
committed 2008/01/08, it was under the PMAP_DIAGNOSTIC before.

Since such page has md.pv_list empty anyway, this KASSERT seems to be
only the statement of intent. The change below would prevent the panic
by not calling pmap_remove_all from vm_object_page_remove for such pages.

Alan, do you have objections ? [Alternative seems to be a removal of the
assertions from all pmap implementations, that also weaken the invariants
for other callers that do skip fictitious pages].

diff --git a/sys/vm/vm_object.c b/sys/vm/vm_object.c
index 21c0ac6..21ee10d 100644
--- a/sys/vm/vm_object.c
+++ b/sys/vm/vm_object.c
@@ -1884,7 +1884,8 @@ again:
 		 */
 		if ((wirings =3D p->wire_count) !=3D 0 &&
 		    (wirings =3D pmap_page_wired_mappings(p)) !=3D p->wire_count) {
-			pmap_remove_all(p);
+			if ((p->flags & PG_FICTITIOUS) =3D=3D 0)
+				pmap_remove_all(p);
 			/* Account for removal of managed, wired mappings. */
 			p->wire_count -=3D wirings;
 			if (!clean_only)
@@ -1898,7 +1899,8 @@ again:
 			if (p->valid & p->dirty)
 				continue;
 		}
-		pmap_remove_all(p);
+		if ((p->flags & PG_FICTITIOUS) =3D=3D 0)
+			pmap_remove_all(p);
 		/* Account for removal of managed, wired mappings. */
 		if (wirings !=3D 0)
 			p->wire_count -=3D wirings;