Re: enable INVARIANT_SUPPORT in GENERIC in release builds

On Wed, Apr 17, 2024 at 03:41:06PM -0600, Warner Losh wrote:
> 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?
Right, I mean that some parts of INVARIANTS_SUPPORT needs to be cleaned
first ...

> 
> 
> > 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...
> 
... and then this option can be removed altogether, by providing the
exported functions unconditionally.