> Date: Fri, 1 Sep 2023 23:02:59 +0200
> From: Martin Pieuchot <m...@openbsd.org>
>
> On 12/08/23(Sat) 10:43, Martin Pieuchot wrote:
> > Since UVM has been imported, we zap mappings associated to anon pages
> > before deactivating or freeing them. Sadly, with the introduction of
> > locking for amaps & anons, I added new code paths that do not respect
> > this behavior.
> > The diff below restores it by moving the call to pmap_page_protect()
> > inside uvm_anon_release(). With it the 3 code paths using the function
> > are now coherent with the rest of UVM.
> >
> > I remember a discussion we had questioning the need for zapping such
> > mappings. I'm interested in hearing more arguments for or against this
> > change. However, right now, I'm more concerned about coherency, so I'd
> > like to commit the change below before we try something different.
>
> Unless there's an objection, I'll commit this tomorrow in order to
> unblock my upcoming UVM changes.
I was planning to take a closer look at this, but the change should be
fine in terms of correctness.
> > Index: uvm/uvm_anon.c
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_anon.c,v
> > retrieving revision 1.55
> > diff -u -p -u -7 -r1.55 uvm_anon.c
> > --- uvm/uvm_anon.c 11 Apr 2023 00:45:09 -0000 1.55
> > +++ uvm/uvm_anon.c 15 May 2023 13:55:28 -0000
> > @@ -251,14 +251,15 @@ uvm_anon_release(struct vm_anon *anon)
> > KASSERT((pg->pg_flags & PG_RELEASED) != 0);
> > KASSERT((pg->pg_flags & PG_BUSY) != 0);
> > KASSERT(pg->uobject == NULL);
> > KASSERT(pg->uanon == anon);
> > KASSERT(anon->an_ref == 0);
> >
> > uvm_lock_pageq();
> > + pmap_page_protect(pg, PROT_NONE);
> > uvm_pagefree(pg);
> > uvm_unlock_pageq();
> > KASSERT(anon->an_page == NULL);
> > lock = anon->an_lock;
> > uvm_anfree(anon);
> > rw_exit(lock);
> > /* Note: extra reference is held for PG_RELEASED case. */
> > Index: uvm/uvm_fault.c
> > ===================================================================
> > RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
> > retrieving revision 1.133
> > diff -u -p -u -7 -r1.133 uvm_fault.c
> > --- uvm/uvm_fault.c 4 Nov 2022 09:36:44 -0000 1.133
> > +++ uvm/uvm_fault.c 15 May 2023 13:55:28 -0000
> > @@ -392,15 +392,14 @@ uvmfault_anonget(struct uvm_faultinfo *u
> >
> > /*
> > * if we were RELEASED during I/O, then our anon is
> > * no longer part of an amap. we need to free the
> > * anon and try again.
> > */
> > if (pg->pg_flags & PG_RELEASED) {
> > - pmap_page_protect(pg, PROT_NONE);
> > KASSERT(anon->an_ref == 0);
> > /*
> > * Released while we had unlocked amap.
> > */
> > if (locked)
> > uvmfault_unlockall(ufi, NULL, NULL);
> > uvm_anon_release(anon); /* frees page for us */
> >
>
>
