Date: Tue, 6 May 2003 14:40:10 +0800 From: "David Xu" <davidxu@freebsd.org> To: "Julian Elischer" <julian@elischer.org>, "Daniel Eischen" <eischen@pcnet1.pcnet.com> Cc: threads@freebsd.org Subject: Re: kern_threads.c.. upcall question.. Message-ID: <04ec01c3139a$579093d0$f001a8c0@davidw2k> References: <Pine.BSF.4.21.0305051713510.27399-100000@InterJet.elischer.org>
next in thread | previous in thread | raw e-mail | index | archive | help
----- Original Message -----=20
From: "Julian Elischer" <julian@elischer.org>
To: "Daniel Eischen" <eischen@pcnet1.pcnet.com>
Cc: <threads@freebsd.org>
Sent: Tuesday, May 06, 2003 8:39 AM
Subject: Re: kern_threads.c.. upcall question..
>=20
>=20
> On Mon, 5 May 2003, Daniel Eischen wrote:
>=20
> > On Mon, 5 May 2003, Julian Elischer wrote:
> >=20
> > >=20
> > > In kern_threads.c, in function thread_export_context()
> > > it first does a copyin() of the
> > > context storage area. I think this is un-needed (and a complete =
waste of
> > > cycles) but I am not sure if there is some strange condition
> > > regarding floating point registers or something that may want =
this..
> > >=20
> > > Dan, Jon, David?
> > > do any of you have a good reason why I shouldn't remove the =
copyin().??
> >=20
> > Yeah, the threads library keeps the thread's active signal
> > mask in the context area. The stack and flags may also
> > be used in the context.
> >=20
> > You should be able to safely copy out the mcontext without
> > a copyin(). Is that what you're currently doing? Or is
> > the entire context (ucontext) being exported?
>=20
> it does:
>=20
> *) copyin the ucontext_t
>=20
> *) load context into it using thread_getcontext() which uses
> get_mcontext() and adds the kernel thread sigmask (this must have
> been added by jeff in some way) (note it doesn't OR, just writes)
>=20
> *) copyout of the ucontext_t
>=20
> The ucontext is:
>=20
> sigset_t uc_sigmask; <-- gets over-written (!)
> mcontext_t uc_mcontext; <-- gets over-written
> struct __ucontext *uc_link; <-- one in userland MAY be
> valuable
> stack_t uc_stack; <-- probably worth saving..
> int uc_flags; <-- probably worth saving
> int __spare__[4];
>=20
>=20
>=20
>=20
> It seems to me that just copying out the sigset_t and mcontext_t
> may be all that is needed. (and I'm not so sure about the sigset_t
> at that..
>=20
>=20
> >=20
> > --=20
> > Dan Eischen
> >=20
> >=20
I think the following patch is enough:
Index: kern_thread.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /home/ncvs/src/sys/kern/kern_thread.c,v
retrieving revision 1.129
diff -u -r1.129 kern_thread.c
--- kern_thread.c 1 May 2003 12:16:06 -0000 1.129
+++ kern_thread.c 6 May 2003 06:32:10 -0000
@@ -721,41 +721,6 @@
}
=20
/*
- * Fill a ucontext_t with a thread's context information.
- *
- * This is an analogue to getcontext(3).
- */
-void
-thread_getcontext(struct thread *td, ucontext_t *uc)
-{
-
- get_mcontext(td, &uc->uc_mcontext, 0);
- PROC_LOCK(td->td_proc);
- uc->uc_sigmask =3D td->td_sigmask;
- PROC_UNLOCK(td->td_proc);
-}
-
-/*
- * Set a thread's context from a ucontext_t.
- *
- * This is an analogue to setcontext(3).
- */
-int
-thread_setcontext(struct thread *td, ucontext_t *uc)
-{
- int ret;
-
- ret =3D set_mcontext(td, &uc->uc_mcontext);
- if (ret =3D=3D 0) {
- SIG_CANTMASK(uc->uc_sigmask);
- PROC_LOCK(td->td_proc);
- td->td_sigmask =3D uc->uc_sigmask;
- PROC_UNLOCK(td->td_proc);
- }
- return (ret);
-}
-
-/*
* Initialize global thread allocation resources.
*/
void
@@ -962,27 +927,25 @@
uintptr_t mbx;
void *addr;
int error,temp;
- ucontext_t uc;
+ mcontext_t mc;
=20
p =3D td->td_proc;
kg =3D td->td_ksegrp;
=20
/* Export the user/machine context. */
- addr =3D (void *)(&td->td_mailbox->tm_context);
- error =3D copyin(addr, &uc, sizeof(ucontext_t));
- if (error)=20
- goto bad;
-
- thread_getcontext(td, &uc);
- error =3D copyout(&uc, addr, sizeof(ucontext_t));
- if (error)=20
+ get_mcontext(td, &mc, 0);
+ addr =3D (void *)(&td->td_mailbox->tm_context.uc_mcontext);
+ error =3D copyout(&mc, addr, sizeof(mcontext_t));
+ if (error)
goto bad;
=20
/* Exports clock ticks in kernel mode */
addr =3D (caddr_t)(&td->td_mailbox->tm_sticks);
temp =3D fuword(addr) + td->td_usticks;
- if (suword(addr, temp))
+ if (suword(addr, temp)) {
+ error =3D EFAULT;
goto bad;
+ }
=20
/* Get address in latest mbox of list pointer */
addr =3D (void *)(&td->td_mailbox->tm_next);
And should we disable single threading testing or do
double checking in thread_user_enter()? I think per-syscall
PROC_LOCK is too expensive for us.
David Xu
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?04ec01c3139a$579093d0$f001a8c0>