Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 10 May 2002 23:50:17 -0400 (EDT)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Alan Cox <alc@cs.rice.edu>
Cc:        Jeff Roberson <jroberson@chesapeake.net>, alpha@FreeBSD.ORG, obrien@FreeBSD.ORG, Andrew Gallatin <gallatin@cs.duke.edu>
Subject:   Re: gcc3 & alpha kernels
Message-ID:  <XFMail.20020510235017.jhb@FreeBSD.org>
In-Reply-To: <20020511034040.GA2412@cs.rice.edu>

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

On 11-May-2002 Alan Cox wrote:
> On Fri, May 10, 2002 at 08:48:37PM -0400, John Baldwin wrote:
>> 
>> ... 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.
>> 
> 
> In the MI parts of the vm, outside of _vm_object_allocate(),
> there is only one other use of atomic ops and that simply
> adds 1 to an u_int.  The rest were removed when Giant was
> introduced.

Hmm.  Ok, after talking with jeff@ it is getting weirder.  I was wrong
at first as we are hanging on the second atomic_cmpset, not hte first (printf's
help).  It seems that gcc has a bug (*sigh*) in that instead of usign 'zapnot'
to zero extned the unsigned int it passes in, it is now using addl s0,zero,s0,
but addl does sign extension on its result, so now we get a sign extended
version to compare against.  Thus, one way of fixing this is to revert Jeff's
earlier fix.  However, it is arguably wrong for gcc to be sign-extending the
unsigned int it is passing in.

Here's the asm of the atomic_cmpset_int() for hash_rand:

     1f8:       09 00 3f 41     addl    s0,zero,s0
     1fc:       58 00 2b a0     ldl     t0,88(s2)
     200:       00 00 4a a8     ldl_l   t1,0(s1)
     204:       22 f6 41 48     zapnot  t1,0xf,t1
     208:       a2 05 49 40     cmpeq   t1,s0,t1
     20c:       04 00 40 e4     beq     t1,220 <_vm_object_allocate+0x1ac>
     210:       02 04 e1 47     mov     t0,t1
     214:       00 00 4a b8     stl_c   t1,0(s1)
     218:       00 00 40 e4     beq     t1,21c <_vm_object_allocate+0x1a8>
     21c:       00 40 00 60     mb

And printf output:

Copyright (c) 1992-2002 The FreeBSD Project.
Copyright (c) 1979, 1980, 1983, 1986, 1988, 1989, 1991, 1992, 1993, 1994
        The Regents of the University of California. All rights reserved.
FreeBSD 5.0-CURRENT #3: Fri May 10 23:25:50 EDT 2002
    john@sema.baldwin.cx:/usr/obj/usr/src/sys/SEMA
_vm_object_allocate: cmpset(0, 0, 13)
_vm_object_allocate: cmpset(0, 0, ffffff7f)
_vm_object_allocate: cmpset(13, 13, 6)
_vm_object_allocate: cmpset(ffffff7f, ffffff7f, fffffefe)
_vm_object_allocate: cmpset(ffffff7f, ffffff7f, fffffefe)
_vm_object_allocate: cmpset(ffffff7f, ffffff7f, fffffefe)
_vm_object_allocate: cmpset(ffffff7f, ffffff7f, fffffefe)
_vm_object_allocate: cmpset(ffffff7f, ffffff7f, fffffefe)
_vm_object_allocate: cmpset(ffffff7f, ffffff7f, fffffefe)

(I did a printf for each interation, as you can see it hung the second
time we tried to do the second atomic_cmpset()).

Given this, it might be interesting to see if just backing out Jeff's change
fixes things for now w/o needing to change any types around.  OTOH, I do
think it's cleaner to have hash_rand and next_index be unsigned at least, but
pg_color might be able to to back to a u_short if it works.  *shrug*

Sorry for the confusion earlier.

-- 

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.20020510235017.jhb>