Index | Thread | Search

From:
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject:
Re: pdaemon, interrupts and ci_idepth
To:
Martin Pieuchot <mpi@grenadille.net>
Cc:
tech@openbsd.org
Date:
Tue, 22 Oct 2024 21:01:02 +0200

Download raw body.

Thread
> Date: Sat, 19 Oct 2024 15:23:32 +0200
> From: Martin Pieuchot <mpi@grenadille.net>
> 
> When a system is OOM and has only its reserved pages available, it is
> not helpful to let interrupt handlers steal such memory from the page
> daemon.  
> 
> I've seen iwm(4)'s interrupt handler allocate memory reserved for the
> pagedaemon because checking against `curproc' is not good enough.  So
> the diff below use the MD `ci_idepth' variable to prevent such scenario.
> If this is acceptable, I'd be happy to rename `ci_intrdepth' used by
> some architectures to `ci_idepth'.
> 
> Ok?

Yes, this makes sense.  But this does require some MD work first.

Also I wonder if it makes sense to abstract the

  (curproc == uvm.pagedaemon_proc && curcpu()->ci_idepth == 0)

and

  (curproc == syncerproc && curcpu()->ci_idepth == 0)

conditions into inline functions, such that that first multi-line
condition becomes a bit easier to understand...

> diff --git sys/uvm/uvm_pmemrange.c sys/uvm/uvm_pmemrange.c
> index 0dff15f6cee..611d6d2785d 100644
> --- sys/uvm/uvm_pmemrange.c
> +++ sys/uvm/uvm_pmemrange.c
> @@ -967,7 +967,8 @@ again:
>  	}
>  
>  	if ((uvmexp.free <= (uvmexp.reserve_pagedaemon + count)) &&
> -	    (curproc != uvm.pagedaemon_proc) && (curproc != syncerproc)) {
> +	    (((curproc != uvm.pagedaemon_proc) && (curproc != syncerproc)) ||
> +	     (curcpu()->ci_idepth > 0))) {
>  		uvm_unlock_fpageq();
>  		if (flags & UVM_PLA_WAITOK) {
>  			uvm_wait("uvm_pmr_getpages");
> @@ -2095,7 +2096,7 @@ uvm_wait_pla(paddr_t low, paddr_t high, paddr_t size, int failok)
>  	struct uvm_pmalloc pma;
>  	const char *wmsg = "pmrwait";
>  
> -	if (curproc == uvm.pagedaemon_proc) {
> +	if (curproc == uvm.pagedaemon_proc && curcpu()->ci_idepth == 0) {
>  		/*
>  		 * This is not that uncommon when the pagedaemon is trying
>  		 * to flush out a large mmapped file. VOP_WRITE will circle
> 
>