Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 17 Apr 2024 15:41:06 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        Konstantin Belousov <kostikbel@gmail.com>
Cc:        Colin Percival <cperciva@tarsnap.com>, Lexi Winter <lexi@le-fay.org>, arch@freebsd.org
Subject:   Re: enable INVARIANT_SUPPORT in GENERIC in release builds
Message-ID:  <CANCZdfq3JWzwLPc_mWp%2BU0nFZFL4YsBqq-%2BosGDMmZOMutPo2Q@mail.gmail.com>
In-Reply-To: <Zh-hbQqsR0XsSwnw@kib.kiev.ua>
References:  <Zh7m7yKbNKafuU0J@ilythia.eden.le-fay.org> <0100018ee9e8a381-2e0a8845-5321-4841-bfaf-184376e88112-000000@email.amazonses.com> <CANCZdfr6TBCja6eD8MD7MxZN3N_NwVzpU8KzszqukO2yNFrMNg@mail.gmail.com> <Zh-hbQqsR0XsSwnw@kib.kiev.ua>

next in thread | previous in thread | raw e-mail | index | archive | help
--000000000000fb43c5061651b65a
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Apr 17, 2024 at 4:16=E2=80=AFAM Konstantin Belousov <kostikbel@gmai=
l.com>
wrote:

> On Tue, Apr 16, 2024 at 09:53:06PM -0600, Warner Losh wrote:
> > On Tue, Apr 16, 2024 at 8:35=E2=80=AFPM Colin Percival <cperciva@tarsna=
p.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 KA=
BI
> > > > 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 defaul=
t?
> > >
> > > 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 t=
he
> > > > 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 f=
or
> > > 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

--000000000000fb43c5061651b65a
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><br></div><br><div class=3D"gmail_quote">=
<div dir=3D"ltr" class=3D"gmail_attr">On Wed, Apr 17, 2024 at 4:16=E2=80=AF=
AM Konstantin Belousov &lt;<a href=3D"mailto:kostikbel@gmail.com">kostikbel=
@gmail.com</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding=
-left:1ex">On Tue, Apr 16, 2024 at 09:53:06PM -0600, Warner Losh wrote:<br>
&gt; On Tue, Apr 16, 2024 at 8:35=E2=80=AFPM Colin Percival &lt;<a href=3D"=
mailto:cperciva@tarsnap.com" target=3D"_blank">cperciva@tarsnap.com</a>&gt;=
 wrote:<br>
&gt; <br>
&gt; &gt; On 4/16/24 14:00, Lexi Winter wrote:<br>
&gt; &gt; &gt; currently release version of GENERIC (or GENERIC-NODEBUG in =
main) does<br>
&gt; &gt; &gt; not have INVARIANT_SUPPORT enabled.<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; unfortunately, the presence or absense of this option breaks=
 the KABI<br>
&gt; &gt; &gt; because, as i understand it, modules built with INVARIANTS w=
on&#39;t load on<br>
&gt; &gt; &gt; a kernel without INVARIANT_SUPPORT.<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; is there a reason INVARIANT_SUPPORT can&#39;t just be enable=
d by default?<br>
&gt; &gt;<br>
&gt; &gt; I think while it had much lower overhead than INVARIANTS, there w=
as still<br>
&gt; &gt; a significant overhead cost at least in the early days.=C2=A0 May=
be that&#39;s no<br>
&gt; &gt; longer the case.<br>
&gt; &gt;<br>
&gt; <br>
&gt; I thought it had no overhead (despite the comments saying it does). It=
<br>
&gt; only increases runtime from what I can see if INVARIANTS or WITNESS<br=
>
&gt; are defined.<br>
&gt; <br>
&gt; <br>
&gt; &gt; &gt; this would remove one roadblock to separating kernel modules=
 from the<br>
&gt; &gt; &gt; kernel config in both pkgbase and ports, because there would=
 be no need<br>
&gt; &gt; &gt; to build a KABI-incompatible kernel just to build a single m=
odule with<br>
&gt; &gt; &gt; INVARIANTS.<br>
&gt; &gt;<br>
&gt; &gt; If the overhead cost of INVARIANT_SUPPORT is no longer relevant, =
I&#39;d be<br>
&gt; &gt; fine with including it in stable/15.=C2=A0 Of course we can&#39;t=
 turn it on for<br>
&gt; &gt; stable/1[34] for the ABI reasons you just mentioned<br>
&gt; &gt;<br>
&gt; <br>
&gt; I think that it just exports more functions, so that&#39;s something t=
hat could<br>
&gt; be exported.<br>
<br>
No, it does not. For instance, for buffer cache, INVARIANTS_SUPPORT<br>
makes buffer lock asserts into real calls into lockmgr. It might do<br>
something similar to the inpcb locks as well.<br></blockquote><div><br></di=
v><div>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&#39;t act=
ually make things slow unless one or both of INVARIANTS and WITNESS are inc=
luded.</div><div><br></div><div>But I see this in kern_lock.c, which I&#39;=
m not sure about:</div><div><br></div><div>#ifndef INVARIANTS<br>#define _l=
ockmgr_assert(lk, what, file, line)<br>#endif<br></div><div><br></div><div>=
which looks like it too requires INVARIANTS. What am I missing?</div><div>=
=C2=A0</div><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0=
.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Fixing such case and making INVARIANTS_SUPPORT indeed only export some<br>
functions would be a pre-requisite to enabling it for all users.<br>
<br>
But then, it raises a question, what are the KBI differences between<br>
no-SUPPORT and SUPPORT kernels are, except exported functions?<br></blockqu=
ote><div><br></div><div>I think it is only exported functions. I didn&#39;t=
 see anything other than adding calls or defining functions...</div><div><b=
r></div><div>Warner=C2=A0</div></div></div>

--000000000000fb43c5061651b65a--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfq3JWzwLPc_mWp%2BU0nFZFL4YsBqq-%2BosGDMmZOMutPo2Q>