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.
> 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 */
>
