amd64/169927: siginfo, si_code for fpe errors when error occurs using the SSE math processor

Bruce Evans brde at optusnet.com.au
Wed Jul 18 06:59:49 UTC 2012


On Wed, 18 Jul 2012, Konstantin Belousov wrote:

> On Wed, Jul 18, 2012 at 02:36:16PM +1000, Bruce Evans wrote:
>> On Wed, 18 Jul 2012, Konstantin Belousov wrote:
> ...
>>> The curpcb access shall be spelled as PCPU_GET(curpcb). Please note that
>>> compiler is not allowed to cache the accesses, since there is __asm
>>> __volatile expression to indirect through %gs-prefixed read.
>>
>> Fix curpcb then.  curthread was de-pessimized to use __pure2 and to not
>> use __volatile.  I could never this to work to cache curthread when I
>> ried it, but it apparently works in -current.
> ..
> curpcb() could be implemented like this for amd64 only:

Similarly for i386?

> diff --git a/sys/amd64/include/pcb.h b/sys/amd64/include/pcb.h
> index 22cbbe2..3417c52 100644
> --- a/sys/amd64/include/pcb.h
> +++ b/sys/amd64/include/pcb.h
> @@ -144,6 +144,17 @@ void	makectx(struct trapframe *, struct pcb *);
> int	savectx(struct pcb *) __returns_twice;
> void	resumectx(struct pcb *);
>
> +static __inline __pure2 struct pcb *
> +__curpcb(void)
> +{
> +	struct pcb *pcb;
> +
> +	__asm("movq %%gs:%1,%0" : "=r" (pcb)
> +	    : "i" (offsetof(struct pcpu, pc_curpcb)));

pc_curpcb is only a pointer, so there is no need for pcb.h to be used
(any more than proc.h is neaded for __curthread()).

I think this can go in machine/pcpu.h next to __curthread().  Or
machine/cpuinfo can provide the an alternative to PCPU_GET() without
__volatile and sys/pcpu.h can do this.

__curthread() avoids a dependency on sys/pcpu.h by hard-coding the
offset of pc_curthread as 0.  This hack seems to be unnecessary,
Including machine/pcpu.h without going through sys/pcpu.h should be an
error.  This error is only in a few low tier files:
- i386/xen/pcpu.h: this bogusly includes machine/pcpu.h after already
   including sys/cpu.h
- mips/cavium/octeon_machdep.c
- mips/cavium/uart_dev_oct16550.c
- powerpc/include/platform.h
- xen/evtchn.h.  xen code is horrible.  In i386/include/xen/xen_os.h,
   as part of this file's namespace pollution, it claims to include
   sys/time.h for pcpu.h.  But actually sys/time.h only provides moderate
   namespace pollution that doesn't include pcpu.h.  It is another style
   bug to include sys/time.h directly.  sys/time.h is standard namespace
   pollution in sys/param.h in the _KERNEL case.

So amd64 can apparently safely put this function in machine/pcpu.h.
i386 might have to untangle the xen pollution.  Other arches aren't
directly affected, since they don't have npx.  I wondered if they
even had curpcb (if not then pc_curpcb should be in the MD fields).
Grep shows that they have considerable variation and messes.  At
lease ia64 doesn't use the "MI" pc_curpcb.  There is similar variation
and messes for pcpup.

> +	return (pcb);
> +}
> +#define	curpcb		(__curpcb())
> +
> #endif
>
> #endif /* _AMD64_PCB_H_ */
> diff --git a/sys/sys/user.h b/sys/sys/user.h
> index accb7c3..ab1c7a7 100644
> --- a/sys/sys/user.h
> +++ b/sys/sys/user.h
> @@ -35,6 +35,7 @@
> #ifndef _SYS_USER_H_
> #define _SYS_USER_H_
>
> +#include <sys/pcpu.h>

Too much namespace pollution for me.

> #include <machine/pcb.h>

Putting the definiton in machine/pcpu.h should avoid changing the
prerequistes for machine/pcb.h.

> #ifndef _KERNEL
> /* stuff that *used* to be included by user.h, or is now needed */
>
> Please note the location in pcb.h an not in machine/pcpu.h, where it
> cannot work for technical reasons (struct pcpu is not defined yet).

Not applicable -- see above.

> This should be committed separetely, together with the pass over amd64/
> to convert PCPU_GET(curpcb) into curpcb().
>
> Do you agree with committing the PR fix as is and adding the curpcb() later ?
> And removing fnclex() later (it seems I convinced enough for its removal).

OK.

Bruce


More information about the freebsd-amd64 mailing list