Date: Thu, 12 May 2011 16:33:23 -0400 From: Warner Losh <imp@bsdimp.com> To: Attilio Rao <attilio@FreeBSD.org> Cc: src-committers@FreeBSD.org, Artem Belevich <art@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: <E25F78F9-1208-4F9F-960F-C9871A8105E3@bsdimp.com> In-Reply-To: <BANLkTinHGpL5tC3-5jOPUq6bJ2Ks7j_Dww@mail.gmail.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>
next in thread | previous in thread | raw e-mail | index | archive | help
On May 12, 2011, at 4:05 PM, Attilio Rao wrote: > 2011/5/12 Attilio Rao <attilio@freebsd.org>: >> 2011/5/12 Attilio Rao <attilio@freebsd.org>: >>> 2011/5/12 Artem Belevich <art@freebsd.org>: >>>> On Thu, May 12, 2011 at 3:22 PM, Attilio Rao <attilio@freebsd.org> = wrote: >>>>> 2011/5/12 Artem Belevich <art@freebsd.org>: >>>>>> On Thu, May 12, 2011 at 5:15 AM, Attilio Rao = <attilio@freebsd.org> wrote: >>>>>>> 2011/5/8 Attilio Rao <attilio@freebsd.org>: >>>>>>>> Author: attilio >>>>>>>> Date: Sun May 8 00:39:49 2011 >>>>>>>> New Revision: 221614 >>>>>>>> URL: http://svn.freebsd.org/changeset/base/221614 >>>>>>>>=20 >>>>>>>> Log: >>>>>>>> All architectures define the size-bounded types (uint32_t, = uint64_t, etc.) >>>>>>>> starting from base C types (int, long, etc). >>>>>>>=20 >>>>>>> mips seems having the same issue, so here is my patch: >>>>>>> http://www.freebsd.org/~attilio/largeSMP/mips-atomic.diff >>>>>>=20 >>>>>> I see at least one problem. N32 ABI is ILP32 even though it can = use >>>>>> 64-bit registers for "long long". >>>>>=20 >>>>> I'm sorry but this _longlong is non standard for our atomic = scheme, >>>>> nothing in the kernel uses it and in general I'd say to not add = them >>>>> if not strictly necessary. >>>>> Said that, in the -CURRENT code it doesn't seem present, or I'm = missing it? >>>>=20 >>>> The key is that N32 ABI does use 64-bit registers and can perform >>>> 64-bit atomic ops. >>>> Unfortunately, 64-bit type is long long or [u]int64_t. The point is >>>> that it's not *long*. >>>>>=20 >>>>>>> #if defined(__mips_n64) || defined(__mips_n32) >>>>>>> static __inline void >>>>>>> -atomic_set_64(__volatile uint64_t *p, uint64_t v) >>>>>>> +atomic_set_long(__volatile u_long *p, u_long v) >>>>>>=20 >>>>>> Using _long here would not match the assembly on N32. >>>>>>=20 >>>>>> If you stick with your changes, then you should drop __mips_n32 = from >>>>>> the ifdef above. >>>>>> You may also want to add atomic_*_longlong for n32. Actually, for = o64 as well. >>>>>=20 >>>>> I'm not entirely sure in which way the above is different... or at >>>>> least, I may have overlooked the _long support for __mips_n32... = can >>>>> you please elaborate a bit more? >>>>=20 >>>> The inline assembly for atomic_set_long above uses 64-bit = load/store. >>>> This is not going to work for 32-bit long in N32 ABI. >>>=20 >>> That cames from my mips assembly ignorance, I apologize. >>> I just gave a look at types.h and what it just I basically see is: >>> - __mips_n32 is just the same as plain !__mips_n32 && !__mips_n64 >>> - __mips_n64 just addes the operations for 64 bits operands >>=20 >> Oh, yeah, I forgot this part. >> - __mips_n32 should be able to crunch the _64 variants, while >> probabilly the !__mips_n32 && !__mips_n64 doesn't >>=20 >>> Thus what I need to make sure is: >>> - __mips_n32 (and !__mips_n32 && !__mips_n64) case work on 32 bits >>> operand on long too >>> - __mips_n64 works on 64 bits long operands >>=20 >> - That __mips_n32 also defines the _64 variants. >=20 > 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 >=20 > I compiled several kernels for MIPS (with sparse configurations and > they all compile well). >=20 > If you agree with this patch, tonight I'll commit to my tree and add > the mips support for cpuset_t soon, so that it all can be tested. This change looks good. Warner=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?E25F78F9-1208-4F9F-960F-C9871A8105E3>