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

[Warner Losh]

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:
> > On Tue, Nov 20, 2018 at 02:58:41PM +0000, Mateusz Guzik wrote:
> >> Author: mjg
> >> Date: Tue Nov 20 14:58:41 2018
> >> New Revision: 340676
> >> URL: https://svnweb.freebsd.org/changeset/base/340676
> >>
> >> Log:
> >>   Implement unr64
> >>
> >>   Important users of unr like tmpfs or pipes can get away with just
> >>   ever-increasing counters, making the overhead of managing the state
> >>   for 32 bit counters a pessimization.
> >>
> >>   Change it to an atomic variable. This can be further sped up by making
> >>   the counts variable "allocate" ranges and store them per-cpu.
> >>
> >>   Reviewed by:       kib
> >>   Sponsored by:      The FreeBSD Foundation
> >>   Differential Revision:     https://reviews.freebsd.org/D18054
> >>
> >> Modified:
> >>   head/sys/kern/subr_unit.c
> >>   head/sys/sys/systm.h
> >>
> >> Modified: head/sys/kern/subr_unit.c
> >>
> ==============================================================================
> >> --- head/sys/kern/subr_unit.c        Tue Nov 20 14:52:43 2018
> (r340675)
> >> +++ head/sys/kern/subr_unit.c        Tue Nov 20 14:58:41 2018
> (r340676)
> >> @@ -98,6 +98,19 @@ static struct mtx unitmtx;
> >>
> >>  MTX_SYSINIT(unit, &unitmtx, "unit# allocation", MTX_DEF);
> >>
> >> +#ifdef UNR64_LOCKED
> >> +uint64_t
> >> +alloc_unr64(struct unrhdr64 *unr64)
> >> +{
> >> +    uint64_t item;
> >> +
> >> +    mtx_lock(&unitmtx);
> >> +    item = unr64->counter++;
> >> +    mtx_unlock(&unitmtx);
> >> +    return (item);
> >> +}
> >> +#endif
> >> +
> >>  #else /* ...USERLAND */
> >>
> >>  #include <bitstring.h>
> >>
> >> Modified: head/sys/sys/systm.h
> >>
> ==============================================================================
> >> --- head/sys/sys/systm.h     Tue Nov 20 14:52:43 2018        (r340675)
> >> +++ head/sys/sys/systm.h     Tue Nov 20 14:58:41 2018        (r340676)
> >> @@ -523,6 +523,32 @@ 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__)
> > 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.


> With this in place a global macro would have to explicitly indicate
> lack of fetchadd, not 64 ops. So I think the current state is good
> enough for now. Imo in the long run 64 bit ops should just get
> emulated with a lock protected code in general manner.
>

We already do this for ARM. On armv4/5, there are no atomic instructions at
all, so we emulate them by doing the normal operation with interrupts
disabled. Since there's no MP designs from that era we support, this works
out well. For most MIPS designs, this would work well too since the 32-bit
mips designs we support are mostly UP (though to be fair, one can build
32-bit kernels for our 64-bit stuff, but they could cope with the lock).

Warner