From: Jonathan Matthew <jonathan@d14n.org>
Subject: Re: cpu_exit() diet
To: Martin Pieuchot <mpi@grenadille.net>
Cc: tech@openbsd.org
Date: Wed, 21 May 2025 18:54:33 +1000

On Fri, May 16, 2025 at 04:31:23PM +0200, Martin Pieuchot wrote:
> Diff below moves pmap_deactivate() and sched_exit() outside of
> cpu_exit(). 
> 
> The order of operations change for i386 and mips64:
> 
>  - For i386 pmap_deactivate() was missing but since it's a no-op it's fine.
> 
>  - For mips64 pmap_deactivate() was called before restoring `p_addr'.  It's
>    also harmless because it only clears the curpmap pointer.
> 
> I tested this on arm64, amd64 and i386.  I'd appreciate tests on other
> architectures to ensure there's no mistake.

This survives booting and building a kernel on octeon/mips64.

> 
> ok?
> 
> Index: kern/kern_exit.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/kern_exit.c,v
> diff -u -p -r1.246 kern_exit.c
> --- kern/kern_exit.c	16 May 2025 13:40:30 -0000	1.246
> +++ kern/kern_exit.c	16 May 2025 14:13:41 -0000
> @@ -382,16 +382,21 @@ exit1(struct proc *p, int xexit, int xsi
>  	 */
>  
>  	/*
> -	 * Finally, call machine-dependent code to switch to a new
> -	 * context (possibly the idle context).  Once we are no longer
> -	 * using the dead process's vmspace and stack, exit2() will be
> -	 * called to schedule those resources to be released by the
> -	 * reaper thread.
> -	 *
> -	 * Note that cpu_exit() will end with a call equivalent to
> -	 * cpu_switch(), finishing our execution (pun intended).
> +	 * Finally, call machine-dependent code.
>  	 */
>  	cpu_exit(p);
> +
> +	/*
> +	 * Deactivate the exiting address space before the vmspace
> +	 * is freed.  Note that we will continue to run on this
> +	 * vmspace's context until the switch to idle in sched_exit().
> +	 *
> +	 * Once we are no longer using the dead process's vmspace and
> +	 * stack, exit2() will be called to schedule those resources
> +	 * to be released by the reaper thread.
> +	 */
> +	pmap_deactivate(p);
> +	sched_exit(p);
>  	panic("cpu_exit returned");
>  }
>  
> Index: arch/alpha/alpha/locore.s
> ===================================================================
> RCS file: /cvs/src/sys/arch/alpha/alpha/locore.s,v
> diff -u -p -r1.57 locore.s
> --- arch/alpha/alpha/locore.s	28 Apr 2025 13:27:20 -0000	1.57
> +++ arch/alpha/alpha/locore.s	16 May 2025 14:13:41 -0000
> @@ -653,7 +653,7 @@ LEAF(cpu_switchto, 2)
>  	 * We don't deactivate if we came here from sched_exit
>  	 * (old pmap no longer exists; vmspace has been freed).
>  	 * oldproc will be NULL in this case.  We have actually
> -	 * taken care of calling pmap_deactivate() in cpu_exit(),
> +	 * taken care of calling pmap_deactivate() in exit1(),
>  	 * before the vmspace went away.
>  	 */
>  	beq	s0, 2f
> Index: arch/alpha/alpha/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/alpha/alpha/vm_machdep.c,v
> diff -u -p -r1.53 vm_machdep.c
> --- arch/alpha/alpha/vm_machdep.c	21 May 2024 23:16:06 -0000	1.53
> +++ arch/alpha/alpha/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -45,25 +45,11 @@
>  #include <machine/reg.h>
>  
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - */
>  void
> -cpu_exit(p)
> -	struct proc *p;
> +cpu_exit(struct proc *p)
>  {
> -
>  	if (p->p_addr->u_pcb.pcb_fpcpu != NULL)
>  		fpusave_proc(p, 0);
> -
> -	/*
> -	 * Deactivate the exiting address space before the vmspace
> -	 * is freed.  Note that we will continue to run on this
> -	 * vmspace's context until the switch to idle in sched_exit().
> -	 */
> -	pmap_deactivate(p);
> -	sched_exit(p);
> -	/* NOTREACHED */
>  }
>  
>  /*
> Index: arch/amd64/amd64/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/vm_machdep.c,v
> diff -u -p -r1.49 vm_machdep.c
> --- arch/amd64/amd64/vm_machdep.c	24 Oct 2024 17:37:03 -0000	1.49
> +++ arch/amd64/amd64/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -116,17 +116,9 @@ cpu_fork(struct proc *p1, struct proc *p
>  	pcb->pcb_rbp = 0;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - *
> - * We clean up a little and then call sched_exit() with the old proc as an
> - * argument.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  /*
> Index: arch/arm/arm/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm/arm/vm_machdep.c,v
> diff -u -p -r1.29 vm_machdep.c
> --- arch/arm/arm/vm_machdep.c	11 Apr 2023 00:45:07 -0000	1.29
> +++ arch/arm/arm/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -130,9 +130,6 @@ cpu_exit(struct proc *p)
>  	/* If we were using the FPU, forget about it. */
>  	if (p->p_addr->u_pcb.pcb_fpcpu != NULL)
>  		vfp_discard(p);
> -
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/arm64/arm64/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/arm64/arm64/vm_machdep.c,v
> diff -u -p -r1.13 vm_machdep.c
> --- arch/arm64/arm64/vm_machdep.c	31 Jan 2025 20:49:25 -0000	1.13
> +++ arch/arm64/arm64/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -112,17 +112,9 @@ cpu_fork(struct proc *p1, struct proc *p
>  	pcb->pcb_sp = (uint64_t)sf;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - *
> - * We clean up a little and then call sched_exit() with the old proc as an
> - * argument.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/hppa/hppa/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/hppa/hppa/vm_machdep.c,v
> diff -u -p -r1.85 vm_machdep.c
> --- arch/hppa/hppa/vm_machdep.c	25 Oct 2022 15:15:38 -0000	1.85
> +++ arch/hppa/hppa/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -131,9 +131,6 @@ cpu_exit(struct proc *p)
>  	fpu_proc_flush(p);
>  
>  	pool_put(&hppa_fppl, pcb->pcb_fpstate);
> -
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/i386/i386/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/i386/i386/vm_machdep.c,v
> diff -u -p -r1.74 vm_machdep.c
> --- arch/i386/i386/vm_machdep.c	11 Apr 2023 00:45:07 -0000	1.74
> +++ arch/i386/i386/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -103,9 +103,6 @@ cpu_fork(struct proc *p1, struct proc *p
>  	pcb->pcb_ebp = 0;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> @@ -114,7 +111,6 @@ cpu_exit(struct proc *p)
>  	if (p->p_addr->u_pcb.pcb_fpcpu != NULL)
>  		npxsave_proc(p, 0);
>  #endif
> -	sched_exit(p);
>  }
>  
>  /*
> Index: arch/m88k/m88k/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/m88k/m88k/vm_machdep.c,v
> diff -u -p -r1.30 vm_machdep.c
> --- arch/m88k/m88k/vm_machdep.c	5 Jun 2024 19:22:04 -0000	1.30
> +++ arch/m88k/m88k/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -108,14 +108,9 @@ cpu_fork(struct proc *p1, struct proc *p
>  	mdpcb->pcb_pc = (u_int)proc_trampoline;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/mips64/mips64/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/mips64/mips64/vm_machdep.c,v
> diff -u -p -r1.42 vm_machdep.c
> --- arch/mips64/mips64/vm_machdep.c	22 May 2022 16:54:17 -0000	1.42
> +++ arch/mips64/mips64/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -125,9 +125,6 @@ cpu_fork(struct proc *p1, struct proc *p
>  	pcb->pcb_context.val[0] = (register_t)func;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> @@ -136,12 +133,10 @@ cpu_exit(struct proc *p)
>  	if (ci->ci_fpuproc == p)
>  		ci->ci_fpuproc = NULL;
>  
> -	pmap_deactivate(p);
>  #if UPAGES == 1
>  	/* restore p_addr for proper deallocation */
>  	p->p_addr = (void *)p->p_md.md_uarea;
>  #endif
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/powerpc/powerpc/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/powerpc/powerpc/vm_machdep.c,v
> diff -u -p -r1.55 vm_machdep.c
> --- arch/powerpc/powerpc/vm_machdep.c	11 Apr 2023 00:45:07 -0000	1.55
> +++ arch/powerpc/powerpc/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -130,10 +130,6 @@ cpu_fork(struct proc *p1, struct proc *p
>  }
>  
>  /*
> - * cpu_exit is called as the last action during exit.
> - * We release the address space and machine-dependent resources,
> - * including the memory for the user structure and kernel stack.
> - *
>   * Since we don't have curproc anymore, we cannot sleep, and therefore
>   * this is at least incorrect for the multiprocessor version.
>   * Not sure whether we can get away with this in the single proc version.		XXX
> @@ -157,9 +153,6 @@ cpu_exit(struct proc *p)
>  	if (pcb->pcb_vr != NULL)
>  		pool_put(&ppc_vecpl, pcb->pcb_vr);
>  #endif /* ALTIVEC */
> -	
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/powerpc64/powerpc64/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/powerpc64/powerpc64/vm_machdep.c,v
> diff -u -p -r1.9 vm_machdep.c
> --- arch/powerpc64/powerpc64/vm_machdep.c	11 Apr 2023 00:45:08 -0000	1.9
> +++ arch/powerpc64/powerpc64/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -113,17 +113,9 @@ cpu_fork(struct proc *p1, struct proc *p
>  	pcb->pcb_sp = (register_t)sf;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - *
> - * We clean up a little and then call sched_exit() with the old proc as an
> - * argument.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/riscv64/riscv64/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/riscv64/riscv64/vm_machdep.c,v
> diff -u -p -r1.11 vm_machdep.c
> --- arch/riscv64/riscv64/vm_machdep.c	11 Apr 2023 00:45:08 -0000	1.11
> +++ arch/riscv64/riscv64/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -104,17 +104,9 @@ cpu_fork(struct proc *p1, struct proc *p
>  	pcb->pcb_sp = (uint64_t)sf;
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - *
> - * We clean up a little and then call sched_exit() with the old proc as an
> - * argument.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  struct kmem_va_mode kv_physwait = {
> Index: arch/sh/sh/locore_c.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sh/sh/locore_c.c,v
> diff -u -p -r1.14 locore_c.c
> --- arch/sh/sh/locore_c.c	6 Jan 2023 19:10:18 -0000	1.14
> +++ arch/sh/sh/locore_c.c	16 May 2025 14:13:41 -0000
> @@ -156,9 +156,6 @@ cpu_exit(struct proc *p)
>  {
>  	if (p->p_md.md_flags & MDP_STEP)
>  		_reg_write_2(SH_(BBRB), 0);
> -
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  #ifndef P1_STACK
> Index: arch/sparc64/sparc64/vm_machdep.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/sparc64/sparc64/vm_machdep.c,v
> diff -u -p -r1.45 vm_machdep.c
> --- arch/sparc64/sparc64/vm_machdep.c	29 Mar 2024 21:27:53 -0000	1.45
> +++ arch/sparc64/sparc64/vm_machdep.c	16 May 2025 14:13:41 -0000
> @@ -269,13 +269,6 @@ fpusave_proc(struct proc *p, int save)
>  #endif
>  }
>  
> -/*
> - * cpu_exit is called as the last action during exit.
> - *
> - * We clean up a little and then call sched_exit() with the old proc
> - * as an argument.  sched_exit() schedules the old vmspace and stack
> - * to be freed, then selects a new process to run.
> - */
>  void
>  cpu_exit(struct proc *p)
>  {
> @@ -284,9 +277,6 @@ cpu_exit(struct proc *p)
>  		free(p->p_md.md_fpstate, M_SUBPROC, sizeof(struct fpstate));
>  		p->p_md.md_fpstate = NULL;
>  	}
> -
> -	pmap_deactivate(p);
> -	sched_exit(p);
>  }
>  
>  
> 
>