Re: ci_want_resched bits vs. MD ci_data.cpu_softints bits

[Chuck Silvers <chuq%chuq.com@localhost>]

On Sat, Dec 03, 2022 at 03:40:13PM +0100, Martin Husemann wrote:
> On Thu, Jul 07, 2022 at 09:22:42PM +0200, Martin Husemann wrote:
> > I guess in older times ci->ci_want_resched was used as a boolean flag, so only
> > 0 or !0 did matter - but nowadays it is a flag word of various bits:
> > 
> > #define     RESCHED_REMOTE          0x01    /* request is for a remote CPU */
> > #define     RESCHED_IDLE            0x02    /* idle LWP observed */
> > #define     RESCHED_UPREEMPT        0x04    /* immediate user ctx switch */
> > #define     RESCHED_KPREEMPT        0x08    /* immediate kernel ctx switch */
> > 
> > 
> > The MD usage of ci_data.cpu_softints on powerpc is a bitmask of pending
> > softint IPLs, which could easily collide with above flags.
> 
> Even if this (currently) does not seem to cause issues, mixing bits from
> different bitsets is wrong, so I'd like to commit the originaly proposed
> fix (only ever clear all bits).

I agree this seems best.


> Additionaly I'd like to commit something like the second change below, fixing
> an issue when creating new lwps (that neither should have ASTs pending nor
> own the altivec psu). I am not sure if the PSL_VEC is the only bit that
> should be cleared here.

the only bits in md_flags that can be set are PSL_VEC and PSL_SE,
but nothing ever looks at the PSL_SE bit in this field.
so you skip copying the whole l_md structure and just set both
md_flags and md_astpending to zero.  the code in process_machdep.c
that sets and clears PSL_SE in md_flags could also be removed.

-Chuck


> Martin
> 
> 
> Index: kern/kern_synch.c
> ===================================================================
> RCS file: /cvsroot/src/sys/kern/kern_synch.c,v
> retrieving revision 1.352
> diff -u -p -r1.352 kern_synch.c
> --- kern/kern_synch.c	26 Oct 2022 23:23:28 -0000	1.352
> +++ kern/kern_synch.c	3 Dec 2022 14:14:38 -0000
> @@ -554,7 +554,8 @@ nextlwp(struct cpu_info *ci, struct sche
>  	 * the update to ci_want_resched will become globally visible before
>  	 * the release of spc_mutex becomes globally visible.
>  	 */
> -	ci->ci_want_resched = ci->ci_data.cpu_softints;
> +	if (ci->ci_data.cpu_softints == 0)
> +		ci->ci_want_resched = 0;
>  
>  	return newl;
>  }
> Index: arch/powerpc/powerpc/vm_machdep.c
> ===================================================================
> RCS file: /cvsroot/src/sys/arch/powerpc/powerpc/vm_machdep.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 vm_machdep.c
> --- arch/powerpc/powerpc/vm_machdep.c	6 Jul 2020 10:52:12 -0000	1.104
> +++ arch/powerpc/powerpc/vm_machdep.c	3 Dec 2022 14:14:38 -0000
> @@ -95,8 +95,13 @@ cpu_lwp_fork(struct lwp *l1, struct lwp 
>  	struct pcb * const pcb1 = lwp_getpcb(l1);
>  	struct pcb * const pcb2 = lwp_getpcb(l2);
>  
> -	/* Copy MD part of lwp and set up user trapframe pointer.  */
> +	/*
> +	 * Copy MD part of lwp, fix up for new lwp and set up user
> +	 * trapframe pointer.
> +	 */
>  	l2->l_md = l1->l_md;
> +	l2->l_md.md_flags &= ~PSL_VEC;
> +	l2->l_md.md_astpending = 0;
>  	l2->l_md.md_utf = trapframe(l2);
>  
>  	/* Copy PCB. */