Re: Simplify locking code in pdaemon
On Thu, Aug 18, 2022 at 12:53:39PM +0200, Martin Pieuchot wrote:
> Use a "slock" variable as done in multiple places to simplify the code.
> The locking stay the same. This is just a first step to simplify this
> mess.
>
> Also get rid of the return value of the function, it is never checked.
>
> ok?
ok jsg@
>
> Index: uvm/uvm_pdaemon.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 uvm_pdaemon.c
> --- uvm/uvm_pdaemon.c 28 Jun 2022 19:31:30 - 1.101
> +++ uvm/uvm_pdaemon.c 18 Aug 2022 10:44:52 -
> @@ -102,7 +102,7 @@ extern void drmbackoff(long);
> */
>
> void uvmpd_scan(struct uvm_pmalloc *);
> -boolean_tuvmpd_scan_inactive(struct uvm_pmalloc *, struct pglist *);
> +void uvmpd_scan_inactive(struct uvm_pmalloc *, struct pglist *);
> void uvmpd_tune(void);
> void uvmpd_drop(struct pglist *);
>
> @@ -377,17 +377,16 @@ uvm_aiodone_daemon(void *arg)
> * => we handle the building of swap-backed clusters
> * => we return TRUE if we are exiting because we met our target
> */
> -
> -boolean_t
> +void
> uvmpd_scan_inactive(struct uvm_pmalloc *pma, struct pglist *pglst)
> {
> - boolean_t retval = FALSE; /* assume we haven't hit target */
> int free, result;
> struct vm_page *p, *nextpg;
> struct uvm_object *uobj;
> struct vm_page *pps[SWCLUSTPAGES], **ppsp;
> int npages;
> struct vm_page *swpps[SWCLUSTPAGES];/* XXX: see below */
> + struct rwlock *slock;
> int swnpages, swcpages; /* XXX: see below */
> int swslot;
> struct vm_anon *anon;
> @@ -402,7 +401,6 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
>*/
> swslot = 0;
> swnpages = swcpages = 0;
> - free = 0;
> dirtyreacts = 0;
> p = NULL;
>
> @@ -431,18 +429,14 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
>*/
> uobj = NULL;
> anon = NULL;
> -
> if (p) {
> /*
> - * update our copy of "free" and see if we've met
> - * our target
> + * see if we've met our target
>*/
> free = uvmexp.free - BUFPAGES_DEFICIT;
> if (((pma == NULL || (pma->pm_flags & UVM_PMA_FREED)) &&
> (free + uvmexp.paging >= uvmexp.freetarg << 2)) ||
> dirtyreacts == UVMPD_NUMDIRTYREACTS) {
> - retval = TRUE;
> -
> if (swslot == 0) {
> /* exit now if no swap-i/o pending */
> break;
> @@ -450,9 +444,9 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
>
> /* set p to null to signal final swap i/o */
> p = NULL;
> + nextpg = NULL;
> }
> }
> -
> if (p) {/* if (we have a new page to consider) */
> /*
>* we are below target and have a new page to consider.
> @@ -460,11 +454,12 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
> uvmexp.pdscans++;
> nextpg = TAILQ_NEXT(p, pageq);
>
> + anon = p->uanon;
> + uobj = p->uobject;
> if (p->pg_flags & PQ_ANON) {
> - anon = p->uanon;
> KASSERT(anon != NULL);
> - if (rw_enter(anon->an_lock,
> - RW_WRITE|RW_NOSLEEP)) {
> + slock = anon->an_lock;
> + if (rw_enter(slock, RW_WRITE|RW_NOSLEEP)) {
> /* lock failed, skip this page */
> continue;
> }
> @@ -474,23 +469,20 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
>*/
> if (pmap_is_referenced(p)) {
> uvm_pageactivate(p);
> - rw_exit(anon->an_lock);
> + rw_exit(slock);
> uvmexp.pdreact++;
> continue;
> }
> if (p->pg_flags & PG_BUSY) {
> - rw_exit(anon->an_lock);
> + rw_exit(slock);
> uvmexp.pdbusy++;
> - /* someone else owns page, skip it */
> continue;
>
Simplify locking code in pdaemon
Use a "slock" variable as done in multiple places to simplify the code.
The locking stay the same. This is just a first step to simplify this
mess.
Also get rid of the return value of the function, it is never checked.
ok?
Index: uvm/uvm_pdaemon.c
===
RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
retrieving revision 1.101
diff -u -p -r1.101 uvm_pdaemon.c
--- uvm/uvm_pdaemon.c 28 Jun 2022 19:31:30 - 1.101
+++ uvm/uvm_pdaemon.c 18 Aug 2022 10:44:52 -
@@ -102,7 +102,7 @@ extern void drmbackoff(long);
*/
void uvmpd_scan(struct uvm_pmalloc *);
-boolean_t uvmpd_scan_inactive(struct uvm_pmalloc *, struct pglist *);
+void uvmpd_scan_inactive(struct uvm_pmalloc *, struct pglist *);
void uvmpd_tune(void);
void uvmpd_drop(struct pglist *);
@@ -377,17 +377,16 @@ uvm_aiodone_daemon(void *arg)
* => we handle the building of swap-backed clusters
* => we return TRUE if we are exiting because we met our target
*/
-
-boolean_t
+void
uvmpd_scan_inactive(struct uvm_pmalloc *pma, struct pglist *pglst)
{
- boolean_t retval = FALSE; /* assume we haven't hit target */
int free, result;
struct vm_page *p, *nextpg;
struct uvm_object *uobj;
struct vm_page *pps[SWCLUSTPAGES], **ppsp;
int npages;
struct vm_page *swpps[SWCLUSTPAGES];/* XXX: see below */
+ struct rwlock *slock;
int swnpages, swcpages; /* XXX: see below */
int swslot;
struct vm_anon *anon;
@@ -402,7 +401,6 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
*/
swslot = 0;
swnpages = swcpages = 0;
- free = 0;
dirtyreacts = 0;
p = NULL;
@@ -431,18 +429,14 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
*/
uobj = NULL;
anon = NULL;
-
if (p) {
/*
-* update our copy of "free" and see if we've met
-* our target
+* see if we've met our target
*/
free = uvmexp.free - BUFPAGES_DEFICIT;
if (((pma == NULL || (pma->pm_flags & UVM_PMA_FREED)) &&
(free + uvmexp.paging >= uvmexp.freetarg << 2)) ||
dirtyreacts == UVMPD_NUMDIRTYREACTS) {
- retval = TRUE;
-
if (swslot == 0) {
/* exit now if no swap-i/o pending */
break;
@@ -450,9 +444,9 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
/* set p to null to signal final swap i/o */
p = NULL;
+ nextpg = NULL;
}
}
-
if (p) {/* if (we have a new page to consider) */
/*
* we are below target and have a new page to consider.
@@ -460,11 +454,12 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
uvmexp.pdscans++;
nextpg = TAILQ_NEXT(p, pageq);
+ anon = p->uanon;
+ uobj = p->uobject;
if (p->pg_flags & PQ_ANON) {
- anon = p->uanon;
KASSERT(anon != NULL);
- if (rw_enter(anon->an_lock,
- RW_WRITE|RW_NOSLEEP)) {
+ slock = anon->an_lock;
+ if (rw_enter(slock, RW_WRITE|RW_NOSLEEP)) {
/* lock failed, skip this page */
continue;
}
@@ -474,23 +469,20 @@ uvmpd_scan_inactive(struct uvm_pmalloc *
*/
if (pmap_is_referenced(p)) {
uvm_pageactivate(p);
- rw_exit(anon->an_lock);
+ rw_exit(slock);
uvmexp.pdreact++;
continue;
}
if (p->pg_flags & PG_BUSY) {
- rw_exit(anon->an_lock);
+ rw_exit(slock);
uvmexp.pdbusy++;
- /* someone else owns page, skip it */
continue;
}
uvmexp.pdanscan++;
} else {
- uobj = p->uobject;
