Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: Read uvmexp.free only once in pdaemon
To:
Martin Pieuchot <mpi@grenadille.net>
Cc:
tech@openbsd.org
Date:
Wed, 06 Nov 2024 11:15:16 +0100

Download raw body.

Thread
> Date: Wed, 6 Nov 2024 10:01:32 +0100
> From: Martin Pieuchot <mpi@grenadille.net>
> 
> Now that all "shrinkers" freeing non-managed memory return the amount of
> freed pages we can use them.
> 
> Diff below update `shortage' based on return value and remove one more
> access to `uvmexp.free'.
> 
> While here compute `inactive_shortage' early in the loop where
> `uvmexp.inactarg' is calculated.  They are both part of the same logic
> and since the "shrinkers" do not deal with managed memory it is fine.
> Also do not trash the per-CPU cache if we already freed enough memory.
> This is generally the case when the pagedaemon is awaken to free "low"
> memory and the buffer cache satisfies the request.
> 
> Ok?

We might want to consider not calling drmbackoff() if the buffer cache
already satisfied the request as well.  But let's think about that
later.

ok kettenis@

> Index: uvm/uvm_pdaemon.c
> ===================================================================
> RCS file: /cvs/src/sys/uvm/uvm_pdaemon.c,v
> diff -u -p -r1.121 uvm_pdaemon.c
> --- uvm/uvm_pdaemon.c	5 Nov 2024 17:28:32 -0000	1.121
> +++ uvm/uvm_pdaemon.c	6 Nov 2024 08:33:03 -0000
> @@ -237,6 +237,7 @@ uvm_pageout(void *arg)
>   			} else
>   				constraint = no_constraint;
>   		}
> +		/* How many pages do we need to free during this round? */
>   		shortage = uvmexp.freetarg - uvmexp.free + BUFPAGES_DEFICIT;
>   		uvm_unlock_fpageq();
>   @@ -248,6 +249,8 @@ uvm_pageout(void *arg)
>   		if (uvmexp.inactarg <= uvmexp.freetarg) {
>   			uvmexp.inactarg = uvmexp.freetarg + 1;
>   		}
> +		inactive_shortage =
> +			uvmexp.inactarg - uvmexp.inactive - BUFPAGES_INACT;
>   		uvm_unlock_pageq();
>    		/* Reclaim pages from the buffer cache if possible. */
> @@ -259,19 +262,17 @@ uvm_pageout(void *arg)
>   		if (size == 0)
>   			size = 16; /* XXX */
>   -		(void) bufbackoff(&constraint, size * 2);
> +		shortage -= bufbackoff(&constraint, size * 2);
>   #if NDRM > 0
> -		drmbackoff(size * 2);
> +		shortage -= drmbackoff(size * 2);
>   #endif
> -		uvm_pmr_cache_drain();
> +		if (shortage > 0)
> +			shortage -= uvm_pmr_cache_drain();
>    		/*
>   		 * scan if needed
>   		 */
>   		uvm_lock_pageq();
> -		shortage = uvmexp.freetarg - uvmexp.free + BUFPAGES_DEFICIT;
> -		inactive_shortage =
> -			uvmexp.inactarg - uvmexp.inactive - BUFPAGES_INACT;
>   		if (pma != NULL || (shortage > 0) || (inactive_shortage > 0)) {
>   			uvmpd_scan(pma, shortage, inactive_shortage,
>   			    &constraint);
> 
>