Date: Sat, 12 Apr 2003 08:04:55 +0800 From: "David Xu" <davidxu@freebsd.org> To: "Peter Wemm" <peter@wemm.org> Cc: freebsd-threads@freebsd.org Subject: Re: patch for %gs saving Message-ID: <002501c30087$26c203e0$0701a8c0@tiger> References: <20030411191005.68C322A7EA@canning.wemm.org>
next in thread | previous in thread | raw e-mail | index | archive | help
----- Original Message -----=20 From: "Peter Wemm" <peter@wemm.org> To: "David Xu" <davidxu@freebsd.org> Cc: <freebsd-threads@freebsd.org> Sent: Saturday, April 12, 2003 3:10 AM Subject: Re: patch for %gs saving=20 > "David Xu" wrote: > >=20 > > ----- Original Message -----=3D20 > > From: "Peter Wemm" <peter@wemm.org> > > To: "David Xu" <davidxu@freebsd.org> > > Cc: <freebsd-threads@freebsd.org> > > Sent: Friday, April 11, 2003 1:37 PM > > Subject: Re: patch for %gs saving=3D20 > >=20 > >=20 > > > "David Xu" wrote: > > > > Here is the patch for kernel to save %gs, > > > > it works well on my machine. > > > > http://people.freebsd.org/~davidxu/i386_gs.diff > > > > Daniel, is this the reason in your libpthread > > > > patch that doesn't use getcontext syscall ? > > >=3D20 > > > To put some background on the issue, there is a reason why we did = not > > > do this. %gs is not used by the kernel, so it does not normally = need =3D > > to > > > be saved and restored on every trap into the kernel. Setting a = =3D > > segment > > > register is Really Slow - measured in hundreds of clock cycles. > > >=3D20 > > > So, we normally only touch %gs when we context switch to a = different =3D > > process > > > that may have a different %gs. Or when one of the context = syscalls =3D > > wants > > > it changed. We cannot avoid touching %fs because we use it for = kernel > > > private data. But if it wasn't for that, we wouldn't be touching > > > %fs for regular traps/syscalls/etc either. > > >=3D20 > > > Bruce Evans understands this better than I do, I would suggest not = =3D > > making > > > this change without talking about it with him first. > > >=3D20 > >=20 > > Yes, I know loading a descriptor is slow, but in real world, such = =3D > > optimization > > will be lost in real work noise. And setcontext syscall is broken by = =3D > > this > > optimization, userland will fail to set his context in atomic = operation, > > set pcb->gs =3D3D userland_gs does not work, so the cost is = obviously, I =3D > > can optimize > > my patch to only save and restore %gs at trap and interrupt time, = when =3D > > entering > > kernel, I can always zero %gs because kernel does not use it, I = think =3D > > this can > > reduce clock cycles at context switch, this might be better than = current =3D > > code which > > loading %gs at every context switch. >=20 > Currently we only change %gs when switching one entire process to = another > seperate process. Your patch changes it so that it changes %gs on = every > single fault/trap/syscall/etc. I regularly see process traces where > the ratio of syscalls to context switches is over 1000 to 1. ie: your > patch would increase the cost of the %gs activity over a thousand = times. >=20 Yes, you are right, I wouldn't change kernel code to save and restore = %gs at every trap/interrupt, I have measured it, I won't do. however, these is couple of bugs: 1. set_mcontext: does not work for %gs if the thread calling set_mcontext is the curthread. 2. set_user_ldt_rv: it does not reload %gs when user LDT might be changed. 3. cpu_switch_load_gs: in trap.c, if cpu_switch_load_gs is failed, trap.c will = psignal(SIGSEGV), however when cpu_switch_load_gs is failed, it is under sched_lock = held, so PROC_LOCK(p) generates LOR and dead lock is possible. 4. other code need to reload %gs set_mcontext and set_user_ldt_rv etcs should be hacked to reloaded %gs, these are costs because of %gs not saved by kernel. > Cheers, > -Peter > -- > Peter Wemm - peter@wemm.org; peter@FreeBSD.org; peter@yahoo-inc.com > "All of this is for nothing if we don't go to the stars" - JMS/B5
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?002501c30087$26c203e0$0701a8c0>