Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 May 2002 20:48:37 -0400 (EDT)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Andrew Gallatin <gallatin@cs.duke.edu>
Cc:        obrien@FreeBSD.ORG, alpha@FreeBSD.ORG, Jeff Roberson <jroberson@chesapeake.net>, alc@FreeBSD.org
Subject:   Re: gcc3 & alpha kernels
Message-ID:  <XFMail.20020510204837.jhb@FreeBSD.org>
In-Reply-To: <15580.13914.162169.930227@grasshopper.cs.duke.edu>

next in thread | previous in thread | raw e-mail | index | archive | help

On 10-May-2002 Andrew Gallatin wrote:
> 
> Jeff Roberson writes:
>  > On Fri, 10 May 2002, Andrew Gallatin wrote:
>  > 
>  > >
>  > > Alan Cox writes:
>  > >  > >
>  > >  > > Did Jeff see a lockup at boot?  Or was this on a running system?
>  > >  >
>  > >  > I believe it was at boot time.  I can't swear to that, however.
>  > >  >
>  > >
>  > > Thanks..  that's the same as me.  It would seem that the new compiler
>  > > is botching the atomic inlines then.
>  > >
>  > > Hmm..  According to the disassembly, it looks like the correct
>  > > sequences are there, though..
>  > >
>  > > Drew
>  > >
>  > 
>  > It was at boot time.  I believe that this was the first time we ever did
>  > negative atomic ints on alpha.  This was with the old compiler as well.  I
>  > haven't looked at the gcc3 output.
>  > 
>  > When I looked at the assembly it was pretty clear that the inline wasn't
>  > written to support non sign extended values.  If you change the prototype
>  > the signed int everything works as expected though.
> 

Bah, I sent another reply already with more detail.  It turns out to be a bug
in the C code and people shooting their feet off trying to be too fancy with
atomic ops.

In the diff I posted earlier there was this questionable section:

-        :      22 76 20 48     zapnot  t0,0x3,t1
         :      21 76 20 48     zapnot  t0,0x3,t0
-        :      01 04 21 42     addq    a1,t0,t0
+        :      22 f6 21 48     zapnot  t0,0xf,t1
+        :      01 04 31 40     addq    t0,a1,t0
         :      01 f0 23 44     and     t0,0x1f,t0
         :      00 00 83 a8     ldl_l   t3,0(t2)
         :      24 f6 81 48     zapnot  t3,0xf,t3
         :      a4 05 82 40     cmpeq   t3,t1,t3
-        :      04 00 80 e4     beq     t3,124 <_vm_object_allocate+0xbc>
+        :      04 00 80 e4     beq     t3,168 <_vm_object_allocate+0xc8>

This is the first atomic_cmpset_int() loop.  The problem is that when
we copy t0 to t1, we zapnot more bits.  The reason is that t1 is a short,
not an int.  The reason for that is that for some weird reason,
object->pg_color is a u_short even though next_index and incr are ints.  So
gcc currectly converts the unsigned short value in object->pg_color to an
int for atomic_cmpset_int() and if next_index has wrapped we are screwed.

One fix would be to cast object->pg_color to an int for this call but that
still has wrapping issues.  A better fix is probably to stop microptimizing
struct vm_object and use a int for pg_color.  I suggest that all the atomic
ops buried in the vm code be checked very carefully for these types of
short/int mismatches as well as any int/long mismatches and the like.

-- 

John Baldwin <jhb@FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-alpha" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.20020510204837.jhb>