From: mpi <mpi@openbsd.org>
Date: Sat, 13 May 2023 09:24:59 +0000 (+0000)
Subject: Put back in the simplification of the aiodone daemon.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=a375eb795cc698fba4b8d5cb4d5f6a02e53fc10c;p=openbsd

Put back in the simplification of the aiodone daemon.

Previous "breakage" of the swap on arm64 has been found to be an issue
on one machine the rockpro/arm64 related to a deadlock built into the
sdmmc(4) stack interacting with swapping code both running under
KERNEL_LOCK().

This issue is easily reproducible on -current and entering swap when
building LLVM on a rockpro crashes the machine by memory corruption.

Tested by mlarkin@ on octeon & i386, by myself on amd64 & arm64 and by
sthen@ on i386 port bulk.

ok beck@ some time ago.

Previous commit message:

Simplify the aiodone daemon which is only used for async writes.

- Remove unused support for asynchronous read, including error conditions

- Grab the proper lock for each page that has been written to swap.  This
allows to enable an assertion in uvm_page_unbusy().

- Move the uvm_anon_release() call outside of uvm_page_unbusy() and
assert for the different anon cases.

ok beck@, kettenis@
---

diff --git a/sys/uvm/uvm_aobj.c b/sys/uvm/uvm_aobj.c
index faea505fbea..490c94d6724 100644
--- a/sys/uvm/uvm_aobj.c
+++ b/sys/uvm/uvm_aobj.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_aobj.c,v 1.107 2022/08/29 02:58:13 jsg Exp $	*/
+/*	$OpenBSD: uvm_aobj.c,v 1.108 2023/05/13 09:24:59 mpi Exp $	*/
 /*	$NetBSD: uvm_aobj.c,v 1.39 2001/02/18 21:19:08 chs Exp $	*/
 
 /*
@@ -143,7 +143,6 @@ struct pool uvm_aobj_pool;
 
 static struct uao_swhash_elt	*uao_find_swhash_elt(struct uvm_aobj *, int,
 				     boolean_t);
-static int			 uao_find_swslot(struct uvm_object *, int);
 static boolean_t		 uao_flush(struct uvm_object *, voff_t,
 				     voff_t, int);
 static void			 uao_free(struct uvm_aobj *);
@@ -241,7 +240,7 @@ uao_find_swhash_elt(struct uvm_aobj *aobj, int pageidx, boolean_t create)
 /*
  * uao_find_swslot: find the swap slot number for an aobj/pageidx
  */
-static inline int
+int
 uao_find_swslot(struct uvm_object *uobj, int pageidx)
 {
 	struct uvm_aobj *aobj = (struct uvm_aobj *)uobj;
diff --git a/sys/uvm/uvm_aobj.h b/sys/uvm/uvm_aobj.h
index e9e2936d2d1..203609019fc 100644
--- a/sys/uvm/uvm_aobj.h
+++ b/sys/uvm/uvm_aobj.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_aobj.h,v 1.19 2022/07/24 11:00:22 mpi Exp $	*/
+/*	$OpenBSD: uvm_aobj.h,v 1.20 2023/05/13 09:24:59 mpi Exp $	*/
 /*	$NetBSD: uvm_aobj.h,v 1.10 2000/01/11 06:57:49 chs Exp $	*/
 
 /*
@@ -60,6 +60,7 @@
 
 void uao_init(void);
 int uao_set_swslot(struct uvm_object *, int, int);
+int uao_find_swslot(struct uvm_object *, int);
 int uao_dropswap(struct uvm_object *, int);
 int uao_swap_off(int, int);
 int uao_shrink(struct uvm_object *, int);
diff --git a/sys/uvm/uvm_page.c b/sys/uvm/uvm_page.c
index c4eb92d8641..bab50a86fcc 100644
--- a/sys/uvm/uvm_page.c
+++ b/sys/uvm/uvm_page.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_page.c,v 1.171 2023/04/11 00:45:09 jsg Exp $	*/
+/*	$OpenBSD: uvm_page.c,v 1.172 2023/05/13 09:24:59 mpi Exp $	*/
 /*	$NetBSD: uvm_page.c,v 1.44 2000/11/27 08:40:04 chs Exp $	*/
 
 /*
@@ -1036,13 +1036,14 @@ uvm_pagefree(struct vm_page *pg)
  * uvm_page_unbusy: unbusy an array of pages.
  *
  * => pages must either all belong to the same object, or all belong to anons.
+ * => if pages are object-owned, object must be locked.
  * => if pages are anon-owned, anons must have 0 refcount.
+ * => caller must make sure that anon-owned pages are not PG_RELEASED.
  */
 void
 uvm_page_unbusy(struct vm_page **pgs, int npgs)
 {
 	struct vm_page *pg;
-	struct uvm_object *uobj;
 	int i;
 
 	for (i = 0; i < npgs; i++) {
@@ -1052,35 +1053,20 @@ uvm_page_unbusy(struct vm_page **pgs, int npgs)
 			continue;
 		}
 
-#if notyet
-		/*
-                 * XXX swap case in uvm_aio_aiodone() is not holding the lock.
-		 *
-		 * This isn't compatible with the PG_RELEASED anon case below.
-		 */
 		KASSERT(uvm_page_owner_locked_p(pg));
-#endif
 		KASSERT(pg->pg_flags & PG_BUSY);
 
 		if (pg->pg_flags & PG_WANTED) {
 			wakeup(pg);
 		}
 		if (pg->pg_flags & PG_RELEASED) {
-			uobj = pg->uobject;
-			if (uobj != NULL) {
-				uvm_lock_pageq();
-				pmap_page_protect(pg, PROT_NONE);
-				/* XXX won't happen right now */
-				if (pg->pg_flags & PQ_AOBJ)
-					uao_dropswap(uobj,
-					    pg->offset >> PAGE_SHIFT);
-				uvm_pagefree(pg);
-				uvm_unlock_pageq();
-			} else {
-				rw_enter(pg->uanon->an_lock, RW_WRITE);
-				uvm_anon_release(pg->uanon);
-			}
+			KASSERT(pg->uobject != NULL ||
+			    (pg->uanon != NULL && pg->uanon->an_ref > 0));
+			atomic_clearbits_int(&pg->pg_flags, PG_RELEASED);
+			pmap_page_protect(pg, PROT_NONE);
+			uvm_pagefree(pg);
 		} else {
+			KASSERT((pg->pg_flags & PG_FAKE) == 0);
 			atomic_clearbits_int(&pg->pg_flags, PG_WANTED|PG_BUSY);
 			UVM_PAGE_OWN(pg, NULL);
 		}
diff --git a/sys/uvm/uvm_pager.c b/sys/uvm/uvm_pager.c
index adb3632045c..a348311f50a 100644
--- a/sys/uvm/uvm_pager.c
+++ b/sys/uvm/uvm_pager.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uvm_pager.c,v 1.89 2022/08/19 05:53:19 mpi Exp $	*/
+/*	$OpenBSD: uvm_pager.c,v 1.90 2023/05/13 09:24:59 mpi Exp $	*/
 /*	$NetBSD: uvm_pager.c,v 1.36 2000/11/27 18:26:41 chs Exp $	*/
 
 /*
@@ -755,50 +755,77 @@ void
 uvm_aio_aiodone_pages(struct vm_page **pgs, int npages, boolean_t write,
     int error)
 {
-	struct vm_page *pg;
 	struct uvm_object *uobj;
+	struct vm_page *pg;
+	struct rwlock *slock;
 	boolean_t swap;
-	int i;
+	int i, swslot;
 
+	slock = NULL;
 	uobj = NULL;
+	pg = pgs[0];
+	swap = (pg->uanon != NULL && pg->uobject == NULL) ||
+		(pg->pg_flags & PQ_AOBJ) != 0;
+
+	KASSERT(swap);
+	KASSERT(write);
+
+	if (error) {
+		if (pg->uobject != NULL) {
+			swslot = uao_find_swslot(pg->uobject,
+			    pg->offset >> PAGE_SHIFT);
+		} else {
+			swslot = pg->uanon->an_swslot;
+		}
+		KASSERT(swslot);
+	}
 
 	for (i = 0; i < npages; i++) {
+		int anon_disposed = 0;
+
 		pg = pgs[i];
+		KASSERT((pg->pg_flags & PG_FAKE) == 0);
 
-		if (i == 0) {
-			swap = (pg->pg_flags & PQ_SWAPBACKED) != 0;
-			if (!swap) {
-				uobj = pg->uobject;
-				rw_enter(uobj->vmobjlock, RW_WRITE);
-			}
+		/*
+		 * lock each page's object (or anon) individually since
+		 * each page may need a different lock.
+		 */
+		if (pg->uobject != NULL) {
+			slock = pg->uobject->vmobjlock;
+		} else {
+			slock = pg->uanon->an_lock;
 		}
-		KASSERT(swap || pg->uobject == uobj);
+		rw_enter(slock, RW_WRITE);
+		anon_disposed = (pg->pg_flags & PG_RELEASED) != 0;
+		KASSERT(!anon_disposed || pg->uobject != NULL ||
+		    pg->uanon->an_ref == 0);
+		uvm_lock_pageq();
 
 		/*
-		 * if this is a read and we got an error, mark the pages
-		 * PG_RELEASED so that uvm_page_unbusy() will free them.
+		 * if this was a successful write,
+		 * mark the page PG_CLEAN.
 		 */
-		if (!write && error) {
-			atomic_setbits_int(&pg->pg_flags, PG_RELEASED);
-			continue;
+		if (!error) {
+			pmap_clear_reference(pg);
+			pmap_clear_modify(pg);
+			atomic_setbits_int(&pg->pg_flags, PG_CLEAN);
 		}
-		KASSERT(!write || (pgs[i]->pg_flags & PG_FAKE) == 0);
 
 		/*
-		 * if this is a read and the page is PG_FAKE,
-		 * or this was a successful write,
-		 * mark the page PG_CLEAN and not PG_FAKE.
+		 * unlock everything for this page now.
 		 */
-		if ((pgs[i]->pg_flags & PG_FAKE) || (write && error != ENOMEM)) {
-			pmap_clear_reference(pgs[i]);
-			pmap_clear_modify(pgs[i]);
-			atomic_setbits_int(&pgs[i]->pg_flags, PG_CLEAN);
-			atomic_clearbits_int(&pgs[i]->pg_flags, PG_FAKE);
+		if (pg->uobject == NULL && anon_disposed) {
+			uvm_unlock_pageq();
+			uvm_anon_release(pg->uanon);
+		} else {
+			uvm_page_unbusy(&pg, 1);
+			uvm_unlock_pageq();
+			rw_exit(slock);
 		}
 	}
-	uvm_page_unbusy(pgs, npages);
-	if (!swap) {
-		rw_exit(uobj->vmobjlock);
+
+	if (error) {
+		uvm_swap_markbad(swslot, npages);
 	}
 }