Date: Thu, 12 May 2011 22:05:13 +0200 From: Attilio Rao <attilio@freebsd.org> To: Artem Belevich <art@freebsd.org> Cc: svn-src-projects@freebsd.org, Oleksandr Tymoshenko <gonzo@freebsd.org>, src-committers@freebsd.org, Warner Losh <imp@freebsd.org>, Bruce Evans <brde@optusnet.com.au> Subject: Re: svn commit: r221614 - projects/largeSMP/sys/powerpc/include Message-ID: <BANLkTinHGpL5tC3-5jOPUq6bJ2Ks7j_Dww@mail.gmail.com> In-Reply-To: <BANLkTikSgEXZz8vjj7kuyeWQE_oKqzB8ug@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>
next in thread | previous in thread | raw e-mail | index | archive | help
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> wrot= e: >>>> 2011/5/12 Artem Belevich <art@freebsd.org>: >>>>> On Thu, May 12, 2011 at 5:15 AM, Attilio Rao <attilio@freebsd.org> wr= ote: >>>>>> 2011/5/8 Attilio Rao <attilio@freebsd.org>: >>>>>>> Author: attilio >>>>>>> Date: Sun May =C2=A08 00:39:49 2011 >>>>>>> New Revision: 221614 >>>>>>> URL: http://svn.freebsd.org/changeset/base/221614 >>>>>>> >>>>>>> Log: >>>>>>> =C2=A0All architectures define the size-bounded types (uint32_t, ui= nt64_t, etc.) >>>>>>> =C2=A0starting from base C types (int, long, etc). >>>>>> >>>>>> mips seems having the same issue, so here is my patch: >>>>>> http://www.freebsd.org/~attilio/largeSMP/mips-atomic.diff >>>>> >>>>> I see at least one problem. N32 ABI is ILP32 even though it can use >>>>> 64-bit registers for "long long". >>>> >>>> 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 missin= g it? >>> >>> 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*. >>>> >>>>>> #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) >>>>> >>>>> Using _long here would not match the assembly on N32. >>>>> >>>>> 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. >>>> >>>> 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? >>> >>> 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. >> >> 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 > > 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 > >> 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 > > - That __mips_n32 also defines the _64 variants. 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 I compiled several kernels for MIPS (with sparse configurations and they all compile well). 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. Thanks, Attilio --=20 Peace can only be achieved by understanding - A. Einstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?BANLkTinHGpL5tC3-5jOPUq6bJ2Ks7j_Dww>