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 faea505..490c94d 100644 (file)
--- 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 e9e2936..2036090 100644 (file)
--- 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 c4eb92d..bab50a8 100644 (file)
--- 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 adb3632..a348311 100644 (file)
--- 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);
        }
 }