Date: Thu, 22 Nov 2018 18:28:19 +0100 From: Mateusz Guzik <mjguzik@gmail.com> To: Warner Losh <imp@bsdimp.com> Cc: Konstantin Belousov <kostikbel@gmail.com>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r340676 - in head/sys: kern sys Message-ID: <CAGudoHFL-B6e5oufAKMY=_gvgto1o07DPHxfv3rZGnaiEwB9yg@mail.gmail.com> In-Reply-To: <CANCZdfrDyj1qjkp1XGjYP_bCHJfmcdbHxUQ%2BncNE=quQfOabUw@mail.gmail.com> References: <201811201458.wAKEwftP033152@repo.freebsd.org> <20181120150756.GD2378@kib.kiev.ua> <CAGudoHG-8VpSpTMX=ZL4LhSsfUe9fEkjr_-KE83K1NQGsskihw@mail.gmail.com> <CANCZdfrDyj1qjkp1XGjYP_bCHJfmcdbHxUQ%2BncNE=quQfOabUw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On 11/20/18, Warner Losh <imp@bsdimp.com> wrote: > On Tue, Nov 20, 2018 at 8:28 AM Mateusz Guzik <mjguzik@gmail.com> wrote: > >> On 11/20/18, Konstantin Belousov <kostikbel@gmail.com> wrote: >> >> +#if defined(__mips__) || defined(__powerpc__) >> > Please note what I asked about this #ifdefs in the review. mips >> > and powerpc machine/atomic.h should define some symbol like >> > __ATOMIC_NO_64_OPS and this case should be handled in less >> > arch-explicit >> > manner. >> > >> >> Right, should have mentioned that in the commit message. >> >> Anyhow, mips has some degree of 64 bit ops even for 32 bits so >> this becomes more iffy. In particular it does have atomic_add_64. >> I don't have a good way to test mips atomics and since non-atomic >> version for powerpc was needed anyway I decided not to try to >> add one. >> > > I thought the 64-bit stuff was not present in true 32-bit mips at all. You > need the lld/scd instructions to do 64-bit atomics which are only available > in a 64-bit environment (eg n32 or n64 execution). They throw a fatal > machine error if you execute them in o32 land. > > And I think the proper ifdef for this is defined(mips) && > !defined(__mips_n64) && !defined(__mips_n32) or something horrible like > that to not pessimize 64-bit executions. > And powerpc will require something of similar sort (as reported by Mark MIllard). It gets quite ugly indeed. I don't have strong opinion how to express the ifdefs, I think this is least bad if sticking to a mi header: diff --git a/sys/sys/systm.h b/sys/sys/systm.h index a1b98c5660c..fab94ee7979 100644 --- a/sys/sys/systm.h +++ b/sys/sys/systm.h @@ -523,7 +523,11 @@ int alloc_unr_specific(struct unrhdr *uh, u_int item); int alloc_unrl(struct unrhdr *uh); void free_unr(struct unrhdr *uh, u_int item); -#if defined(__mips__) || defined(__powerpc__) +#if defined(mips) && !defined(__mips_n64) && !defined(__mips_n32) +#define UNR64_LOCKED +#endif + +#if defined(__powerpc__) && !defined(__powerpc64__) #define UNR64_LOCKED #endif -- Mateusz Guzik <mjguzik gmail.com>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAGudoHFL-B6e5oufAKMY=_gvgto1o07DPHxfv3rZGnaiEwB9yg>