Subject: Re: proposal for PR 27023
To: Andrey Petrov <petrov@netbsd.org>
From: Chuck Silvers <chuq@chuq.com>
List: port-sparc
Date: 01/26/2005 17:18:11
--0eh6TmSyL6TZE2Uz
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Jan 25, 2005 at 01:51:33PM -0800, Andrey Petrov wrote:
> 
> On Tue, Jan 25, 2005 at 09:38:01AM -0800, Chuck Silvers wrote:
> > hi,
> > 
> > here are my thoughts on PR 27023.  the problem is that SA context switches
> > on both sparc and sparc64 need to write the contents of the register windows
> > out to the user stack.  on a normal, non-SA context switch, if the stack
> > isn't valid (perhaps because it was paged out), we'd just write the
> > register windows to the PCB and go on.  this isn't good enough for SAs
> > though, since the PCB is tied to a particular kernel LWP and the user
> > thread may be run on a different LWP next.  so we really do need to get
> > the register window contents back to the real user stack.
> > 
> 
> I thought that while LWP is blocked, corresponding user-level pthread is bound
> to it, and user-level scheduler can only wait till it became unblocked.
> Form this point user-level scheduling is irrelevant as it handled
> differently, I'd say.

yea, that's what I was trying to say in my second message yesterday.

I've attached a diff that eliminates the crash from the reproduction case
given in the PR, and it passes all the libpthread conformance tests on sparc.
I'm not sure that it's correct to skip the rwindow_save() in all the contexts
that sa_upcall_getstate() is called, but I suspect it is.

-Chuck

--0eh6TmSyL6TZE2Uz
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="diff.spt.4"

Index: arch/sparc/sparc/machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc/sparc/machdep.c,v
retrieving revision 1.251
diff -u -p -r1.251 machdep.c
--- arch/sparc/sparc/machdep.c	30 Jun 2004 21:16:39 -0000	1.251
+++ arch/sparc/sparc/machdep.c	27 Jan 2005 01:14:17 -0000
@@ -871,7 +871,7 @@ cpu_getmcontext(l, mcp, flags)
 	 * registers into the pcb; we need them in the process's memory.
 	 */
 	write_user_windows();
-	if (rwindow_save(l))
+	if ((l->l_flag & L_SA_SWITCHING) == 0 && rwindow_save(l))
 		sigexit(l, SIGILL);
 
 	/*
Index: arch/sparc64/sparc64/machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/sparc64/sparc64/machdep.c,v
retrieving revision 1.176
diff -u -p -r1.176 machdep.c
--- arch/sparc64/sparc64/machdep.c	17 Jan 2005 07:55:18 -0000	1.176
+++ arch/sparc64/sparc64/machdep.c	27 Jan 2005 01:14:19 -0000
@@ -1896,7 +1896,7 @@ cpu_getmcontext(l, mcp, flags)
 
 	/* First ensure consistent stack state (see sendsig). */ /* XXX? */
 	write_user_windows();
-	if (rwindow_save(l))
+	if ((l->l_flag & L_SA_SWITCHING) == 0 && rwindow_save(l))
 		sigexit(l, SIGILL);
 
 	/* For now: Erase any random indicators for optional state. */
Index: kern/kern_sa.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sa.c,v
retrieving revision 1.56
diff -u -p -r1.56 kern_sa.c
--- kern/kern_sa.c	6 Jan 2005 19:26:41 -0000	1.56
+++ kern/kern_sa.c	27 Jan 2005 01:14:20 -0000
@@ -801,7 +801,9 @@ sa_upcall_getstate(union sau_state *ss, 
 	size_t ucsize;
 
 	if (l) {
+		l->l_flag |= L_SA_SWITCHING;
 		getucontext(l, &ss->ss_captured.ss_ctx);
+		l->l_flag &= ~L_SA_SWITCHING;
 		sp = (void *)
 			((intptr_t)_UC_MACHINE_SP(&ss->ss_captured.ss_ctx));
 		sp = STACK_ALIGN(sp, ~_UC_UCONTEXT_ALIGN);
Index: sys/lwp.h
===================================================================
RCS file: /cvsroot/src/sys/sys/lwp.h,v
retrieving revision 1.24
diff -u -p -r1.24 lwp.h
--- sys/lwp.h	18 Jul 2004 21:26:52 -0000	1.24
+++ sys/lwp.h	27 Jan 2005 01:14:20 -0000
@@ -117,6 +117,7 @@ extern struct lwp lwp0;			/* LWP for pro
 #define	L_SA_YIELD	0x10000000 /* LWP on VP is yielding */
 #define	L_SA_IDLE	0x20000000 /* VP is idle */
 #define	L_COWINPROGRESS	0x40000000 /* UFS: doing copy on write */
+#define	L_SA_SWITCHING	0x80000000 /* SA LWP in context switch */
 
 /*
  * Status values.

--0eh6TmSyL6TZE2Uz--