Date: Tue, 6 May 2003 15:42:31 +0800 From: "David Xu" <davidxu@freebsd.org> To: "Julian Elischer" <julian@elischer.org> Cc: Daniel Eischen <eischen@pcnet1.pcnet.com> Subject: Re: kern_threads.c.. upcall question.. Message-ID: <003001c313a3$0de7d500$f001a8c0@davidw2k> References: <Pine.BSF.4.21.0305060026570.4662-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: "David Xu" <davidxu@freebsd.org>
Cc: <threads@freebsd.org>; "Daniel Eischen" <eischen@pcnet1.pcnet.com>
Sent: Tuesday, May 06, 2003 3:30 PM
Subject: Re: kern_threads.c.. upcall question..
>=20
>=20
> On Tue, 6 May 2003, David Xu wrote:
>=20
> >=20
> > ----- Original Message -----=20
> > From: "Julian Elischer" <julian@elischer.org>
> > To: "David Xu" <davidxu@freebsd.org>
> > Cc: "Daniel Eischen" <eischen@pcnet1.pcnet.com>; =
<threads@freebsd.org>
> > Sent: Tuesday, May 06, 2003 3:10 PM
> > Subject: Re: kern_threads.c.. upcall question..
> >=20
> >=20
> > >=20
> > >=20
> > > On Tue, 6 May 2003, David Xu wrote:
> > >=20
> > > > I think the following patch is enough:
> > >=20
> > > I agree that this seems enough from what I was reading.
> > > (I like patches that have most lines starting with '-' :-)
> > >=20
> > >=20
> > > >=20
> > > > 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 @@
> > > [...]
> > > > 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);
> > >=20
> > > I think this could be optimised even more.
> > > (why copy the FP regs if they are not valid) (etc).
> > > but it is an improvement..
> > >=20
> >=20
> > Why need we an intermediate mcontext_t, why not
> > direct copy the context in trap frame to userland space?
> > This should be fastest. :-)
>=20
> this is what I was thinking..
> get_mcontext_user(td, addr)
> [...]
>=20
>=20
> >=20
> >=20
> > > > + 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);
> > > >=20
> > > >=20
> > > > 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.
> > >=20
> > > I am not sure which one you refer too.. Which single_threading
> > > test?
> > >=20
> > Single threading testing in thread_user_enter(), someone put=20
> > a PROC_LOCK, quoted here:
> > /* =20
> > * First check that we shouldn't just abort.
> > * But check if we are the single thread first!
> > */
> > PROC_LOCK(p);
> > if ((p->p_flag & P_SINGLE_EXIT) && (p->p_singlethread !=3D =
td)) {
> > mtx_lock_spin(&sched_lock);
> > thread_stopped(p);
> > thread_exit();
> > /* NOTREACHED */
> > }
> > PROC_UNLOCK(p);
> >=20
> > > BTW, I am a little unsure about the calling of thread_user_enter()
> > > from thread_userret().
> > >=20
> >=20
> > This is quantum preemptive for userland, it is used when statclock
> > interrupt hit in userland, if thread quantum is exhausted,
> > it must unbind upcall from current thread, and schedule an upcall,
> > you can think it is an implicit syscall triggered by CPU =
automatically
> > which just means to swap out current thread.
>=20
>=20
> ah..
> I was thinking that we could delay most of thread_user_enter()
> until we need it..
> i.e why read the mailbox until we need it..
>=20
I think fuword is very fast, at least in i386, there is
few instructions in fuword, please read its code in
/sys/i386/i386/support.s, optimizing it may be not worth to do.
Beside this, you can not fetch mailbox pointer on damand,=20
thread_schedule_upcall is protected under sched lock, it=20
can not handle page fault at that time, and kse_thr_interrupt
need a mailbox pointer to indentify thread in kernel, delaying
it will cause problem.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?003001c313a3$0de7d500$f001a8c0>