> Date: Tue, 1 Dec 2020 13:18:59 -0300
> From: Martin Pieuchot <[email protected]>
>
> On 01/12/20(Tue) 15:18, Mark Kettenis wrote:
> > > Date: Tue, 1 Dec 2020 11:08:50 -0300
> > > From: Martin Pieuchot <[email protected]>
> > >
> > > As recently pointed out by jmatthew@ these need the lock as well, ok?
> >
> > No. These pages are "unmanaged" and therefore they can not be on any
> > of the page queues and locking those page queues would be pointless.
> >
> > We probably should adjust the documentation to reflect this.
>
> Something like that?
Yes, because...
> Btw did you look at the uses of uvm_pagefree(), uvm_pagelookup() & co in
> the various pmap? Do you have any suggestion about how to handle them?
...the pages in the various pmaps aren't managed either. However the
use of uvm_pagelookup() in some of the pmaps suggests that we need to
think a bit about PG_TABLED. It doesn't really make sense to use the
page queue lock for that though.
For the amd64 pmap there are separate objects for each individual
userland pmap. So the per-pmap lock may already provide the necessary
locking.
I also need to go back to your previous diff. In hindsight I think
locking the page queues should not be necessary for buffer map pages
either.
> Index: uvm/uvm_page.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_page.c,v
> retrieving revision 1.153
> diff -u -p -r1.153 uvm_page.c
> --- uvm/uvm_page.c 1 Dec 2020 13:56:22 -0000 1.153
> +++ uvm/uvm_page.c 1 Dec 2020 16:16:29 -0000
> @@ -928,7 +928,7 @@ uvm_pagerealloc(struct vm_page *pg, stru
> * uvm_pageclean: clean page
> *
> * => erase page's identity (i.e. remove from object)
> - * => caller must lock page queues
> + * => caller must lock page queues if `pg' is managed
> * => assumes all valid mappings of pg are gone
> */
> void
> @@ -937,7 +937,8 @@ uvm_pageclean(struct vm_page *pg)
> u_int flags_to_clear = 0;
>
> #if all_pmap_are_fixed
> - MUTEX_ASSERT_LOCKED(&uvm.pageqlock);
> + if (pg->pg_flags & (PG_TABLED|PQ_ACTIVE|PQ_INACTIVE))
> + MUTEX_ASSERT_LOCKED(&uvm.pageqlock);
> #endif
>
> #ifdef DEBUG
> @@ -998,14 +999,15 @@ uvm_pageclean(struct vm_page *pg)
> *
> * => erase page's identity (i.e. remove from object)
> * => put page on free list
> - * => caller must lock page queues
> + * => caller must lock page queues if `pg' is managed
> * => assumes all valid mappings of pg are gone
> */
> void
> uvm_pagefree(struct vm_page *pg)
> {
> #if all_pmap_are_fixed
> - MUTEX_ASSERT_LOCKED(&uvm.pageqlock);
> + if (pg->pg_flags & (PG_TABLED|PQ_ACTIVE|PQ_INACTIVE))
> + MUTEX_ASSERT_LOCKED(&uvm.pageqlock);
> #endif
>
> uvm_pageclean(pg);
>
