Re: Simplify aiodone daemon
> Op 29-06-2022 16:17 schreef Martin Pieuchot : > > > The aiodone daemon accounts for and frees/releases pages they were > written to swap. It is only used for asynchronous write. The diff > below uses this knowledge to: > > - Stop suggesting that uvm_swap_get() can be asynchronous. There's an > assert for PGO_SYNCIO 3 lines above. > > - Remove unused support for asynchronous read, including error > conditions, from uvm_aio_aiodone_pages(). > > - Grab the proper lock for each page that has been written to swap. > This allows to enable an assert in uvm_page_unbusy(). > > - Move the uvm_anon_release() call outside of uvm_page_unbusy() and > assert for the different anon cases. This will allows us to unify > code paths waiting for busy pages. > > This is adapted/simplified from what is in NetBSD. > > ok? I don't fully understand the old code, but the diff makes sense combined with what you state above. Two small nits below. ok kettenis@ > Index: uvm/uvm_aobj.c > === > RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v > retrieving revision 1.103 > diff -u -p -r1.103 uvm_aobj.c > --- uvm/uvm_aobj.c29 Dec 2021 20:22:06 - 1.103 > +++ uvm/uvm_aobj.c29 Jun 2022 11:16:35 - > @@ -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 intuao_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 *aob > /* > * uao_find_swslot: find the swap slot number for an aobj/pageidx > */ > -inline static int > +int > uao_find_swslot(struct uvm_object *uobj, int pageidx) > { > struct uvm_aobj *aobj = (struct uvm_aobj *)uobj; > Index: uvm/uvm_aobj.h > === > RCS file: /cvs/src/sys/uvm/uvm_aobj.h,v > retrieving revision 1.17 > diff -u -p -r1.17 uvm_aobj.h > --- uvm/uvm_aobj.h21 Oct 2020 09:08:14 - 1.17 > +++ uvm/uvm_aobj.h29 Jun 2022 11:16:35 - > @@ -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); Spurious space > int uao_dropswap(struct uvm_object *, int); > int uao_swap_off(int, int); > int uao_shrink(struct uvm_object *, int); > Index: uvm/uvm_page.c > === > RCS file: /cvs/src/sys/uvm/uvm_page.c,v > retrieving revision 1.166 > diff -u -p -r1.166 uvm_page.c > --- uvm/uvm_page.c12 May 2022 12:48:36 - 1.166 > +++ uvm/uvm_page.c29 Jun 2022 11:47:55 - > @@ -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,19 @@ uvm_page_unbusy(struct vm_page **pgs, in > 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-
Simplify aiodone daemon
The aiodone daemon accounts for and frees/releases pages they were written to swap. It is only used for asynchronous write. The diff below uses this knowledge to: - Stop suggesting that uvm_swap_get() can be asynchronous. There's an assert for PGO_SYNCIO 3 lines above. - Remove unused support for asynchronous read, including error conditions, from uvm_aio_aiodone_pages(). - Grab the proper lock for each page that has been written to swap. This allows to enable an assert in uvm_page_unbusy(). - Move the uvm_anon_release() call outside of uvm_page_unbusy() and assert for the different anon cases. This will allows us to unify code paths waiting for busy pages. This is adapted/simplified from what is in NetBSD. ok? Index: uvm/uvm_aobj.c === RCS file: /cvs/src/sys/uvm/uvm_aobj.c,v retrieving revision 1.103 diff -u -p -r1.103 uvm_aobj.c --- uvm/uvm_aobj.c 29 Dec 2021 20:22:06 - 1.103 +++ uvm/uvm_aobj.c 29 Jun 2022 11:16:35 - @@ -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_tuao_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 *aob /* * uao_find_swslot: find the swap slot number for an aobj/pageidx */ -inline static int +int uao_find_swslot(struct uvm_object *uobj, int pageidx) { struct uvm_aobj *aobj = (struct uvm_aobj *)uobj; Index: uvm/uvm_aobj.h === RCS file: /cvs/src/sys/uvm/uvm_aobj.h,v retrieving revision 1.17 diff -u -p -r1.17 uvm_aobj.h --- uvm/uvm_aobj.h 21 Oct 2020 09:08:14 - 1.17 +++ uvm/uvm_aobj.h 29 Jun 2022 11:16:35 - @@ -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); Index: uvm/uvm_page.c === RCS file: /cvs/src/sys/uvm/uvm_page.c,v retrieving revision 1.166 diff -u -p -r1.166 uvm_page.c --- uvm/uvm_page.c 12 May 2022 12:48:36 - 1.166 +++ uvm/uvm_page.c 29 Jun 2022 11:47:55 - @@ -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,19 @@ uvm_page_unbusy(struct vm_page **pgs, in 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); + 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);