From 305e6808b99fa339aae1581a35952340d7e45c78 Mon Sep 17 00:00:00 2001 From: mpi Date: Sun, 24 Jul 2022 11:00:22 +0000 Subject: [PATCH] Revert simplification of the aiodone daemon it breaks swap on arm64. Found the hard way by mlarkin@ and deraadt@. --- sys/uvm/uvm_aobj.c | 5 +-- sys/uvm/uvm_aobj.h | 3 +- sys/uvm/uvm_page.c | 31 ++++++++++++----- sys/uvm/uvm_pager.c | 81 +++++++++++++++------------------------------ 4 files changed, 54 insertions(+), 66 deletions(-) diff --git a/sys/uvm/uvm_aobj.c b/sys/uvm/uvm_aobj.c index 1f7700d2324..1c85448f0e8 100644 --- 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 4a528517e27..e9e2936d2d1 100644 --- 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 a21ae310d68..f4ad82de86b 100644 --- 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 6e0ad7552ac..2f886c03054 100644 --- 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); } } -- 2.20.1