svn commit: r340676 - in head/sys: kern sys

[Warner Losh]

On Thu, Nov 22, 2018 at 10:28 AM Mateusz Guzik <mjguzik at gmail.com> wrote:

> On 11/20/18, Warner Losh <imp at bsdimp.com> wrote:
> > On Tue, Nov 20, 2018 at 8:28 AM Mateusz Guzik <mjguzik at gmail.com> wrote:
> >
> >> On 11/20/18, Konstantin Belousov <kostikbel at 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
>

We should move the defining of this to machine/atomic.h as suggested
elsewhere. Once it's more than one phrase long, it makes no sense to
pollute the MI header with MD stuff like this.

Warner