Re: enable INVARIANT_SUPPORT in GENERIC in release builds

On Wed, Apr 17, 2024 at 4:16 AM Konstantin Belousov <kostikbel@gmail.com>
wrote:

> On Tue, Apr 16, 2024 at 09:53:06PM -0600, Warner Losh wrote:
> > On Tue, Apr 16, 2024 at 8:35 PM Colin Percival <cperciva@tarsnap.com>
> wrote:
> >
> > > On 4/16/24 14:00, Lexi Winter wrote:
> > > > currently release version of GENERIC (or GENERIC-NODEBUG in main)
> does
> > > > not have INVARIANT_SUPPORT enabled.
> > > >
> > > > unfortunately, the presence or absense of this option breaks the KABI
> > > > because, as i understand it, modules built with INVARIANTS won't
> load on
> > > > a kernel without INVARIANT_SUPPORT.
> > > >
> > > > is there a reason INVARIANT_SUPPORT can't just be enabled by default?
> > >
> > > I think while it had much lower overhead than INVARIANTS, there was
> still
> > > a significant overhead cost at least in the early days.  Maybe that's
> no
> > > longer the case.
> > >
> >
> > I thought it had no overhead (despite the comments saying it does). It
> > only increases runtime from what I can see if INVARIANTS or WITNESS
> > are defined.
> >
> >
> > > > this would remove one roadblock to separating kernel modules from the
> > > > kernel config in both pkgbase and ports, because there would be no
> need
> > > > to build a KABI-incompatible kernel just to build a single module
> with
> > > > INVARIANTS.
> > >
> > > If the overhead cost of INVARIANT_SUPPORT is no longer relevant, I'd be
> > > fine with including it in stable/15.  Of course we can't turn it on for
> > > stable/1[34] for the ABI reasons you just mentioned
> > >
> >
> > I think that it just exports more functions, so that's something that
> could
> > be exported.
>
> No, it does not. For instance, for buffer cache, INVARIANTS_SUPPORT
> makes buffer lock asserts into real calls into lockmgr. It might do
> something similar to the inpcb locks as well.
>

Why not make those INVARIANTS then? All the ones for mutexes (which is the
bulk of the other uses) just provide the routines, but don't actually make
things slow unless one or both of INVARIANTS and WITNESS are included.

But I see this in kern_lock.c, which I'm not sure about:

#ifndef INVARIANTS
#define _lockmgr_assert(lk, what, file, line)
#endif

which looks like it too requires INVARIANTS. What am I missing?


> Fixing such case and making INVARIANTS_SUPPORT indeed only export some
> functions would be a pre-requisite to enabling it for all users.
>
> But then, it raises a question, what are the KBI differences between
> no-SUPPORT and SUPPORT kernels are, except exported functions?
>

I think it is only exported functions. I didn't see anything other than
adding calls or defining functions...

Warner