svn commit: r336025 - in head/sys: amd64/include i386/include

[Warner Losh]

On Fri, Jul 6, 2018 at 9:32 AM, Rodney W. Grimes <
freebsd at pdx.rh.cn85.dnsmgr.net> wrote:

> > Author: hselasky
> > Date: Fri Jul  6 10:13:42 2018
> > New Revision: 336025
> > URL: https://svnweb.freebsd.org/changeset/base/336025
> >
> > Log:
> >   Make sure kernel modules built by default are portable between UP and
> >   SMP systems by extending defined(SMP) to include defined(KLD_MODULE).
> >
> >   This is a regression issue after r335873 .
> >
> >   Discussed with:             mmacy@
> >   Sponsored by:               Mellanox Technologies
>
> Though this fixes the issue, it also means that now when
> anyone intentionally builds a UP kernel with modules
> they are getting SMP support in the modules and I am
> not sure they would want that.  I know I don't.
>


On UP systems, these additional opcodes are harmless. They take a few extra
cycles (since they lock an uncontested bus) and add a couple extra memory
barriers (which will be NOPs). On MP systems, atomics now work by default.
Had we not defaulted like this, all modules built outside of a kernel build
env would have broken atomics. Given that (a) the overwhelming majority
(99% or more) is SMP and (b) the MP code merely adds a few cycles to what's
already a not-too-expensive operation, this was the right choice.

It simply doesn't matter for systems that are relevant to the project
today. While one could try to optimize this a little (for example, by
having SMP defined to be 0 or 1, say, and changing all the ifdef SMP to if
(defined(SMP) && SMP != 0)), it's likely not going to matter enough for
anybody to make the effort. UP on x86 is simply not relevant enough to
optimize for it. Even in VMs, people run SMP kernels typically even when
they just allocate one CPU to the VM.

So while we still support the UP config, and we'll let people build
optimized kernels for x86, we've flipped the switch from pessimized for SMP
modules to pessimized for UP modules, which seems like quite the reasonable
trade-off.

Were it practical to do so, I'd suggest de-orbiting UP on x86. However,
it's a lot of work for not much benefit and we'd need to invent much crazy
to get there.

Warner


> > Modified:
> >   head/sys/amd64/include/atomic.h
> >   head/sys/i386/include/atomic.h
> >
> > Modified: head/sys/amd64/include/atomic.h
> > ============================================================
> ==================
> > --- head/sys/amd64/include/atomic.h   Fri Jul  6 10:10:00 2018
> (r336024)
> > +++ head/sys/amd64/include/atomic.h   Fri Jul  6 10:13:42 2018
> (r336025)
> > @@ -132,7 +132,7 @@ void              atomic_store_rel_##TYPE(volatile
> u_##TYPE *p, u_
> >   * For userland, always use lock prefixes so that the binaries will run
> >   * on both SMP and !SMP systems.
> >   */
> > -#if defined(SMP) || !defined(_KERNEL)
> > +#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE)
> >  #define      MPLOCKED        "lock ; "
> >  #else
> >  #define      MPLOCKED
> > @@ -354,7 +354,7 @@ atomic_testandclear_long(volatile u_long *p, u_int
> v)
> >   */
> >  #define      OFFSETOF_MONITORBUF     0x100
> >
> > -#if defined(SMP)
> > +#if defined(SMP) || defined(KLD_MODULE)
> >  static __inline void
> >  __storeload_barrier(void)
> >  {
> >
> > Modified: head/sys/i386/include/atomic.h
> > ============================================================
> ==================
> > --- head/sys/i386/include/atomic.h    Fri Jul  6 10:10:00 2018
> (r336024)
> > +++ head/sys/i386/include/atomic.h    Fri Jul  6 10:13:42 2018
> (r336025)
> > @@ -143,7 +143,7 @@ void              atomic_subtract_64(volatile
> uint64_t *, uint64_t
> >   * For userland, always use lock prefixes so that the binaries will run
> >   * on both SMP and !SMP systems.
> >   */
> > -#if defined(SMP) || !defined(_KERNEL)
> > +#if defined(SMP) || !defined(_KERNEL) || defined(KLD_MODULE)
> >  #define      MPLOCKED        "lock ; "
> >  #else
> >  #define      MPLOCKED
> > @@ -302,7 +302,7 @@ atomic_testandclear_int(volatile u_int *p, u_int v)
> >   */
> >
> >  #if defined(_KERNEL)
> > -#if defined(SMP)
> > +#if defined(SMP) || defined(KLD_MODULE)
> >  #define      __storeload_barrier()   __mbk()
> >  #else /* _KERNEL && UP */
> >  #define      __storeload_barrier()   __compiler_membar()
> >
> >
>
> --
> Rod Grimes
> rgrimes at freebsd.org
>
>