Revert simplification of the aiodone daemon it breaks swap on arm64.

Found the hard way by mlarkin@ and deraadt@.

diff --git a/sys/uvm/uvm_aobj.c b/sys/uvm/uvm_aobj.c
index 1f7700d..1c85448 100644 (file)
--- a/sys/uvm/uvm_aobj.c
+++ b/sys/uvm/uvm_aobj.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uvm_aobj.c,v 1.104 2022/07/11 11:33:17 mpi Exp $      */
+/*     $OpenBSD: uvm_aobj.c,v 1.105 2022/07/24 11:00:22 mpi Exp $      */
 /*     $NetBSD: uvm_aobj.c,v 1.39 2001/02/18 21:19:08 chs Exp $        */
 
 /*
@@ -143,6 +143,7 @@ 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 *);
@@ -240,7 +241,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
  */
-int
+inline static 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 4a52851..e9e2936 100644 (file)
--- a/sys/uvm/uvm_aobj.h
+++ b/sys/uvm/uvm_aobj.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uvm_aobj.h,v 1.18 2022/07/11 11:33:17 mpi Exp $       */
+/*     $OpenBSD: uvm_aobj.h,v 1.19 2022/07/24 11:00:22 mpi Exp $       */
 /*     $NetBSD: uvm_aobj.h,v 1.10 2000/01/11 06:57:49 chs Exp $        */
 
 /*
@@ -60,7 +60,6 @@
 
 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 a21ae31..f4ad82d 100644 (file)
--- a/sys/uvm/uvm_page.c
+++ b/sys/uvm/uvm_page.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uvm_page.c,v 1.167 2022/07/11 11:33:17 mpi Exp $      */
+/*     $OpenBSD: uvm_page.c,v 1.168 2022/07/24 11:00:22 mpi Exp $      */
 /*     $NetBSD: uvm_page.c,v 1.44 2000/11/27 08:40:04 chs Exp $        */
 
 /*
@@ -1036,14 +1036,13 @@ 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++) {
@@ -1053,19 +1052,35 @@ 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) {
-                       KASSERT(pg->uobject != NULL ||
-                           (pg->uanon != NULL && pg->uanon->an_ref > 0));
-                       atomic_clearbits_int(&pg->pg_flags, PG_RELEASED);
-                       uvm_pagefree(pg);
+                       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);
+                       }
                } 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 6e0ad75..2f886c0 100644 (file)
--- a/sys/uvm/uvm_pager.c
+++ b/sys/uvm/uvm_pager.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uvm_pager.c,v 1.84 2022/07/17 17:59:35 kettenis Exp $ */
+/*     $OpenBSD: uvm_pager.c,v 1.85 2022/07/24 11:00:22 mpi Exp $      */
 /*     $NetBSD: uvm_pager.c,v 1.36 2000/11/27 18:26:41 chs Exp $       */
 
 /*
@@ -735,77 +735,50 @@ void
 uvm_aio_aiodone_pages(struct vm_page **pgs, int npages, boolean_t write,
     int error)
 {
-       struct uvm_object *uobj;
        struct vm_page *pg;
-       struct rwlock *slock;
+       struct uvm_object *uobj;
        boolean_t swap;
-       int i, swslot;
+       int i;
 
-       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);
 
-               /*
-                * 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;
+               if (i == 0) {
+                       swap = (pg->pg_flags & PQ_SWAPBACKED) != 0;
+                       if (!swap) {
+                               uobj = pg->uobject;
+                               rw_enter(uobj->vmobjlock, RW_WRITE);
+                       }
                }
-               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();
+               KASSERT(swap || pg->uobject == uobj);
 
                /*
-                * if this was a successful write,
-                * mark the page PG_CLEAN.
+                * if this is a read and we got an error, mark the pages
+                * PG_RELEASED so that uvm_page_unbusy() will free them.
                 */
-               if (!error) {
-                       pmap_clear_reference(pg);
-                       pmap_clear_modify(pg);
-                       atomic_setbits_int(&pg->pg_flags, PG_CLEAN);
+               if (!write && error) {
+                       atomic_setbits_int(&pg->pg_flags, PG_RELEASED);
+                       continue;
                }
+               KASSERT(!write || (pgs[i]->pg_flags & PG_FAKE) == 0);
 
                /*
-                * unlock everything for this page now.
+                * 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.
                 */
-               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);
+               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 (error) {
-               uvm_swap_markbad(swslot, npages);
+       uvm_page_unbusy(pgs, npages);
+       if (!swap) {
+               rw_exit(uobj->vmobjlock);
        }
 }