Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 13 May 2011 21:59:45 +1000 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Warner Losh <imp@bsdimp.com>
Cc:        mdf@freebsd.org, src-committers@freebsd.org, Artem Belevich <art@freebsd.org>, Attilio Rao <attilio@freebsd.org>, Oleksandr Tymoshenko <gonzo@freebsd.org>, Bruce Evans <brde@optusnet.com.au>, svn-src-projects@freebsd.org, Warner Losh <imp@freebsd.org>
Subject:   Re: svn commit: r221614 - projects/largeSMP/sys/powerpc/include
Message-ID:  <20110513213047.P1035@besplex.bde.org>
In-Reply-To: <31ABDF1E-1D1E-4DDB-B89D-D36E9B7DDC63@bsdimp.com>
References:  <201105080039.p480doiZ021493@svn.freebsd.org> <BANLkTi=e7GtBM-PTq9yJHSLRoaOWh62AeA@mail.gmail.com> <BANLkTiktwEvRktZrGOqKKB2iSB99a3Jw=g@mail.gmail.com> <BANLkTik17r-XampEdO%2BsQ7aMOL_SDyhG=g@mail.gmail.com> <BANLkTinaWDcaiZiB3G5Szoaho1jVSeniMA@mail.gmail.com> <BANLkTimj3ohmvACmvcPa3yrdsUj=4D2V3Q@mail.gmail.com> <BANLkTikSgEXZz8vjj7kuyeWQE_oKqzB8ug@mail.gmail.com> <BANLkTinHGpL5tC3-5jOPUq6bJ2Ks7j_Dww@mail.gmail.com> <BANLkTi=DOD9p-YUMm33D5ZShTjS_Q1hEvg@mail.gmail.com> <BANLkTikj%2Bszgd%2BptzD6y%2BofPs%2B8bR7Z8ew@mail.gmail.com> <BANLkTi=hB=ytsGFD8NbG7q56qTQJjroPHg@mail.gmail.com> <BANLkTimkW-S%2BmGrwPMYdHTqZ%2BhoYA=xxeA@mail.gmail.com> <31ABDF1E-1D1E-4DDB-B89D-D36E9B7DDC63@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 13 May 2011, Warner Losh wrote:

> On May 12, 2011, at 6:06 PM, Attilio Rao wrote:
>
>> 2011/5/12  <mdf@freebsd.org>:
>>> On Thu, May 12, 2011 at 2:12 PM, Attilio Rao <attilio@freebsd.org> wrote:
>>>> 2011/5/12 Artem Belevich <art@freebsd.org>:
>>>>> On Thu, May 12, 2011 at 10:05 PM, Attilio Rao <attilio@freebsd.org> wrote:
>>>>>> I spoke in person with Artem and in the end I just decided to make the
>>>>>> smallest possible subset of changes to fix the _long on 32 bits and
>>>>>> then "completed" (as some of them already exist today) the macro
>>>>>> converting the arguments to u_int stuff:
>>>>>> http://www.freebsd.org/~attilio/largeSMP/mips-atomic2.diff

Another reply said that it has too many casts.  1 cast would be too many
for me.

>>>>> Let's get back for a second to the original issue you had that propted
>>>>> you to do atomic ops changes.
>>>>> If I understand you correctly, your code was passing cpuset_t* as an
>>>>> argument to atomic_something_long and that caused compiler to complain
>>>>> that cpuset_t* is not uint32_t*.
>>>>>
>>>>> Could you post definition of cpuset_t ?
>>>>>
>>>>> It's possible that compiler was actually correct. For instance,
>>>>> compiler would be right to complain if cpuset_t is a packed structure,
>>>>> even if that structure is made of a single uint32_t field.
>>>>
>>>> It doesn't do the atomic of  cpuset_t, it does atomic of members of
>>>> cpuset_t which are actually long.

So it has the same design bug as the fd_set API.  longs are very
unportable, and should be inherently slow, since long should be similar
to register_t[2], so an array of longs for a bitmap should be the same
speed as an array of register_t's with twice as many elements.

This bug is missing for the sigset_t API.  sigset_t is even closer to
cpuset_t fd_set.  It uses an array of uint32_t.

>>> Isn't this an argument for making it an array of u_int, even though
>>> it's marginally pessimal on amd64 and other 64-bit arches?  There is
>>> guaranteed support for a int-sized (or perhaps 32-bit sized) atomic
>>> op.

Yes.  register_t is unfortunately not as well supported as u_int by
various levels.  uint32_t for sigset_t only works as well as it does since
it is the same as u_int on all supported arches.  OTOH, uint32_t is better
than u_int (or even register_t) since it is fixed widths.  You probably
don't want the bitmap subtype length to be MD like it is when it is long
(or register_t).  This depends on how well the implementation details
are hidden by the API.

>> There is also guaratees for long-sized atomic operations. I'm not sure
>> what atomic(9) says, but the truth is that long on FreeBSD always

It says that u_long and uint64_t are supported on all arches.  The former
is a bug, and the latter is wrong (uint64_t is not supported on most or
all 32-bit arches).

>> matches word boundry, so there is no problem (Bruce thinks that long
>> actually had to be double the size of words, hence the theoretically
>> possible difficulties for atomic operation, but actually that never
>> was the case on FreeBSD).

I agree :-).

> If we can't pass a long-sized operand to the atomic_long operations, then I'd say that's a bug.

No, the bug is promising to support this.

>
> I think that the current ABI zoo on MIPS may mean that it makes sense to define atomic_foo_32, atomic_foo_64, etc and then define atomic_long in terms of them, such that type-safety is ensured.  Since the _32/_64 functions are defined in .S files, adding aliases based on ABI is easy and we can just have the right prototypes in the mips atomic.h.  While a little gross in some sense, we know that we can audit things such that it will be correct, and also optimal.  This is, after all, low level code and that code often calls for tricks to get optimal performance.

Please use a Unix editor, or at least the Unix newline character.

Supporting uint64_t would be good iff uint64_t == register_t (or maybe
bus_width_t), but there are some complications in getting clients to
use this.  Too many clients now use u_long or long.  Perhaps a global
substition of u_long by register_t would be a practical solutlion for
these.  But register_t is bogusly (signed) int32_t on i386.  There is
also u_register_t on at least i386.  The clients are mostly using
u_long or long for historical reasons.  E.g., as I pointed out earlier,
all the clients of atomic_fetchadd_long() only used it since it used
to be the largest integral type.  BSD4.4 or Net/2 or earlier has far
too many longs.  Someone apparently changed lots of ints to longs to
support 16 bit ints, or the types were always long because ints actually
were 16 bits.  Using longs causes various ABI problems, and FreeBSD via
mostly Lite2 via mostly NetBSD changed many of them to int32_t.  But
many of the old longs remain.  Requiring clients to only use longs when
they actually need to do so forces them to justify this use.  I don't
know of a single case where it is justified (after 1999, when C broke
the guarante that long is the largest integral types).  Using a fixed
width type gives an unchanging API but breaks the automatic future
and arch-dependent expansion that is given by using long.  Using long
used to maximize the expansion possibilities.  Now it doesn't even do
that.

Bruce



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