Re: pmap & buffer cache dummy pagers

2021-09-03 Thread Mark Kettenis
> Date: Thu, 2 Sep 2021 22:16:52 +0200
> From: Martin Pieuchot 
> 
> Diff below introduces two dummy pagers for subsystem that manipulate UVM
> objects that are 'special'.  Those pagers will be used to enforce checks
> in functions that expect a lock to be held, like:
> 
>   KASSERT(obj == NULL || UVM_OBJ_IS_PMAP(obj) ||
> rw_write_held(obj->vmobjlock));
> 
> They are also used, in the diff below, to document which routines expect
> such objects and a serialization offered by the KERNEL_LOCK().  More
> examples can be seen in my WIP unlocking diff.
> 
> The idea is taken from NetBSD which also use such dummy pager for some
> of their pmaps.  I don't believe there's a need to change anything with
> these usages of the uvm_obj_* API for the moment but at the same time it
> helps me to have such implicit documentation.
> 
> ok?

And it follows a pattern that we already have.

I'm still not sure why the hell we need uvm objects in those pmaps,
but I'm also not too eager to delve into them to see if we can rid of
those bits right now.

ok kettenis@

> Index: arch/amd64/amd64/pmap.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/pmap.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 pmap.c
> --- arch/amd64/amd64/pmap.c   18 Jun 2021 06:17:28 -  1.145
> +++ arch/amd64/amd64/pmap.c   2 Sep 2021 19:55:57 -
> @@ -671,7 +671,7 @@ pmap_bootstrap(paddr_t first_avail, padd
>  
>   kpm = pmap_kernel();
>   for (i = 0; i < PTP_LEVELS - 1; i++) {
> - uvm_obj_init(>pm_obj[i], NULL, 1);
> + uvm_obj_init(>pm_obj[i], _pager, 1);
>   kpm->pm_ptphint[i] = NULL;
>   }
>   memset(>pm_list, 0, sizeof(kpm->pm_list));  /* pm_list not used */
> @@ -1307,7 +1307,7 @@ pmap_create(void)
>  
>   /* init uvm_object */
>   for (i = 0; i < PTP_LEVELS - 1; i++) {
> - uvm_obj_init(>pm_obj[i], NULL, 1);
> + uvm_obj_init(>pm_obj[i], _pager, 1);
>   pmap->pm_ptphint[i] = NULL;
>   }
>   pmap->pm_stats.wired_count = 0;
> Index: arch/hppa/hppa/pmap.c
> ===
> RCS file: /cvs/src/sys/arch/hppa/hppa/pmap.c,v
> retrieving revision 1.175
> diff -u -p -r1.175 pmap.c
> --- arch/hppa/hppa/pmap.c 16 Jun 2021 09:02:21 -  1.175
> +++ arch/hppa/hppa/pmap.c 2 Sep 2021 19:54:23 -
> @@ -496,7 +496,7 @@ pmap_bootstrap(vaddr_t vstart)
>*/
>   kpm = _pmap_store;
>   bzero(kpm, sizeof(*kpm));
> - uvm_obj_init(>pm_obj, NULL, 1);
> + uvm_obj_init(>pm_obj, _pager, 1);
>   kpm->pm_space = HPPA_SID_KERNEL;
>   kpm->pm_pid = HPPA_PID_KERNEL;
>   kpm->pm_pdir_pg = NULL;
> @@ -678,7 +678,7 @@ pmap_create(void)
>  
>   mtx_init(>pm_mtx, IPL_VM);
>  
> - uvm_obj_init(>pm_obj, NULL, 1);
> + uvm_obj_init(>pm_obj, _pager, 1);
>  
>   for (space = 1 + arc4random_uniform(hppa_sid_max);
>   pmap_sdir_get(space); space = (space + 1) % hppa_sid_max);
> Index: arch/i386/i386/pmap.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/pmap.c,v
> retrieving revision 1.214
> diff -u -p -r1.214 pmap.c
> --- arch/i386/i386/pmap.c 16 Jun 2021 09:02:21 -  1.214
> +++ arch/i386/i386/pmap.c 2 Sep 2021 19:55:57 -
> @@ -963,7 +963,7 @@ pmap_bootstrap(vaddr_t kva_start)
>   kpm = pmap_kernel();
>   mtx_init(>pm_mtx, -1); /* must not be used */
>   mtx_init(>pm_apte_mtx, IPL_VM);
> - uvm_obj_init(>pm_obj, NULL, 1);
> + uvm_obj_init(>pm_obj, _pager, 1);
>   bzero(>pm_list, sizeof(kpm->pm_list));  /* pm_list not used */
>   kpm->pm_pdir = (vaddr_t)(proc0.p_addr->u_pcb.pcb_cr3 + KERNBASE);
>   kpm->pm_pdirpa = proc0.p_addr->u_pcb.pcb_cr3;
> @@ -1348,7 +1348,7 @@ pmap_create(void)
>   mtx_init(>pm_apte_mtx, IPL_VM);
>  
>   /* init uvm_object */
> - uvm_obj_init(>pm_obj, NULL, 1);
> + uvm_obj_init(>pm_obj, _pager, 1);
>   pmap->pm_stats.wired_count = 0;
>   pmap->pm_stats.resident_count = 1;  /* count the PDP allocd below */
>   pmap->pm_ptphint = NULL;
> Index: uvm/uvm_object.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_object.c,v
> retrieving revision 1.19
> diff -u -p -r1.19 uvm_object.c
> --- uvm/uvm_object.c  16 Jun 2021 09:02:21 -  1.19
> +++ uvm/uvm_object.c  2 Sep 2021 20:00:03 -
> @@ -41,6 +41,16 @@
>  
>  #include 
>  
> +/* Dummy object used by some pmaps for sanity checks. */
> +const struct uvm_pagerops pmap_pager = {
> + /* nothing */
> +};
> +
> +/* Dummy object used by the buffer cache for sanity checks. */
> +const struct uvm_pagerops bufcache_pager = {
> + /* nothing */
> +};
> +
>  /* We will fetch this page count per step */
>  #define  FETCH_PAGECOUNT 16
>  
> @@ -159,6 +169,9 @@ uvm_obj_free(struct uvm_object *uobj)
>  {
> 

pmap & buffer cache dummy pagers

2021-09-02 Thread Martin Pieuchot
Diff below introduces two dummy pagers for subsystem that manipulate UVM
objects that are 'special'.  Those pagers will be used to enforce checks
in functions that expect a lock to be held, like:

KASSERT(obj == NULL || UVM_OBJ_IS_PMAP(obj) ||
rw_write_held(obj->vmobjlock));

They are also used, in the diff below, to document which routines expect
such objects and a serialization offered by the KERNEL_LOCK().  More
examples can be seen in my WIP unlocking diff.

The idea is taken from NetBSD which also use such dummy pager for some
of their pmaps.  I don't believe there's a need to change anything with
these usages of the uvm_obj_* API for the moment but at the same time it
helps me to have such implicit documentation.

ok?

Index: arch/amd64/amd64/pmap.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/pmap.c,v
retrieving revision 1.145
diff -u -p -r1.145 pmap.c
--- arch/amd64/amd64/pmap.c 18 Jun 2021 06:17:28 -  1.145
+++ arch/amd64/amd64/pmap.c 2 Sep 2021 19:55:57 -
@@ -671,7 +671,7 @@ pmap_bootstrap(paddr_t first_avail, padd
 
kpm = pmap_kernel();
for (i = 0; i < PTP_LEVELS - 1; i++) {
-   uvm_obj_init(>pm_obj[i], NULL, 1);
+   uvm_obj_init(>pm_obj[i], _pager, 1);
kpm->pm_ptphint[i] = NULL;
}
memset(>pm_list, 0, sizeof(kpm->pm_list));  /* pm_list not used */
@@ -1307,7 +1307,7 @@ pmap_create(void)
 
/* init uvm_object */
for (i = 0; i < PTP_LEVELS - 1; i++) {
-   uvm_obj_init(>pm_obj[i], NULL, 1);
+   uvm_obj_init(>pm_obj[i], _pager, 1);
pmap->pm_ptphint[i] = NULL;
}
pmap->pm_stats.wired_count = 0;
Index: arch/hppa/hppa/pmap.c
===
RCS file: /cvs/src/sys/arch/hppa/hppa/pmap.c,v
retrieving revision 1.175
diff -u -p -r1.175 pmap.c
--- arch/hppa/hppa/pmap.c   16 Jun 2021 09:02:21 -  1.175
+++ arch/hppa/hppa/pmap.c   2 Sep 2021 19:54:23 -
@@ -496,7 +496,7 @@ pmap_bootstrap(vaddr_t vstart)
 */
kpm = _pmap_store;
bzero(kpm, sizeof(*kpm));
-   uvm_obj_init(>pm_obj, NULL, 1);
+   uvm_obj_init(>pm_obj, _pager, 1);
kpm->pm_space = HPPA_SID_KERNEL;
kpm->pm_pid = HPPA_PID_KERNEL;
kpm->pm_pdir_pg = NULL;
@@ -678,7 +678,7 @@ pmap_create(void)
 
mtx_init(>pm_mtx, IPL_VM);
 
-   uvm_obj_init(>pm_obj, NULL, 1);
+   uvm_obj_init(>pm_obj, _pager, 1);
 
for (space = 1 + arc4random_uniform(hppa_sid_max);
pmap_sdir_get(space); space = (space + 1) % hppa_sid_max);
Index: arch/i386/i386/pmap.c
===
RCS file: /cvs/src/sys/arch/i386/i386/pmap.c,v
retrieving revision 1.214
diff -u -p -r1.214 pmap.c
--- arch/i386/i386/pmap.c   16 Jun 2021 09:02:21 -  1.214
+++ arch/i386/i386/pmap.c   2 Sep 2021 19:55:57 -
@@ -963,7 +963,7 @@ pmap_bootstrap(vaddr_t kva_start)
kpm = pmap_kernel();
mtx_init(>pm_mtx, -1); /* must not be used */
mtx_init(>pm_apte_mtx, IPL_VM);
-   uvm_obj_init(>pm_obj, NULL, 1);
+   uvm_obj_init(>pm_obj, _pager, 1);
bzero(>pm_list, sizeof(kpm->pm_list));  /* pm_list not used */
kpm->pm_pdir = (vaddr_t)(proc0.p_addr->u_pcb.pcb_cr3 + KERNBASE);
kpm->pm_pdirpa = proc0.p_addr->u_pcb.pcb_cr3;
@@ -1348,7 +1348,7 @@ pmap_create(void)
mtx_init(>pm_apte_mtx, IPL_VM);
 
/* init uvm_object */
-   uvm_obj_init(>pm_obj, NULL, 1);
+   uvm_obj_init(>pm_obj, _pager, 1);
pmap->pm_stats.wired_count = 0;
pmap->pm_stats.resident_count = 1;  /* count the PDP allocd below */
pmap->pm_ptphint = NULL;
Index: uvm/uvm_object.c
===
RCS file: /cvs/src/sys/uvm/uvm_object.c,v
retrieving revision 1.19
diff -u -p -r1.19 uvm_object.c
--- uvm/uvm_object.c16 Jun 2021 09:02:21 -  1.19
+++ uvm/uvm_object.c2 Sep 2021 20:00:03 -
@@ -41,6 +41,16 @@
 
 #include 
 
+/* Dummy object used by some pmaps for sanity checks. */
+const struct uvm_pagerops pmap_pager = {
+   /* nothing */
+};
+
+/* Dummy object used by the buffer cache for sanity checks. */
+const struct uvm_pagerops bufcache_pager = {
+   /* nothing */
+};
+
 /* We will fetch this page count per step */
 #defineFETCH_PAGECOUNT 16
 
@@ -159,6 +169,9 @@ uvm_obj_free(struct uvm_object *uobj)
 {
struct vm_page *pg;
struct pglist pgl;
+
+   KASSERT(UVM_OBJ_IS_BUFCACHE(uobj));
+   KERNEL_ASSERT_LOCKED();
 
TAILQ_INIT();
/*
Index: uvm/uvm_object.h
===
RCS file: /cvs/src/sys/uvm/uvm_object.h,v
retrieving revision 1.26
diff -u -p -r1.26 uvm_object.h
--- uvm/uvm_object.h16 Jun 2021 09:02:21 -  1.26