From nobody Wed Apr 17 21:41:06 2024 X-Original-To: arch@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VKZ9D6tzRz5HCK7 for ; Wed, 17 Apr 2024 21:41:20 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-ej1-x635.google.com (mail-ej1-x635.google.com [IPv6:2a00:1450:4864:20::635]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VKZ9D3r0Nz4ZBJ for ; Wed, 17 Apr 2024 21:41:20 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Authentication-Results: mx1.freebsd.org; none Received: by mail-ej1-x635.google.com with SMTP id a640c23a62f3a-a526d381d2fso232471566b.0 for ; Wed, 17 Apr 2024 14:41:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20230601.gappssmtp.com; s=20230601; t=1713390078; x=1713994878; darn=freebsd.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=+m9rfAKflSysNI6FQrVhDUorWxZHTeUAW2KI7EGdHkc=; b=Y31WAaEORrCXBpIUMeXbLwMIjd10ygSA3wFsXqQru1TV8fBQZ09i/gkCPG0LsxQyyL afaAafwhxXGWxuLQG+mD0tBtHZtiGuF5PIFID37taOZequoEhugWNoeHw8jXlpP8pKM0 PS2eJXcyiOiK85VAGS+OKPr8Yzpmv+Dj//99rkfQpU4FxEUyu8lua2hts281Dw255XMd O+2IYzVxg7Mde24LRb7LHs65BbEG7iwM8Lkr5lbhzL/Y6lk0dfEJKG2pJmZ3kHckgkvo C/5ep+ne3aR/BLpbwcH4bzP3ECfdq+CYGNNLs/vrIGE+4XKpdtC5rhBmwrXcMFcPoU3f zWhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713390078; x=1713994878; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=+m9rfAKflSysNI6FQrVhDUorWxZHTeUAW2KI7EGdHkc=; b=hikb0mXg/HaK+6mll4N9cBPemohubem8qIL5+Rv04Bw8jSiQ3iiO+b485Oifsy8TC6 ssuINfmvPKWU6KECrskRjRCMON5VE7A8ieepXEl0xwL/oWm1cxpwv8DSBZdi7BJ6oYN8 qdX//c5XTllZZAhI1u7ZuZA5ZtcReNVfHhNlq9G/tkgrf9Mojoe8+/0Drk336xTlF0ue aVvnOZQwj628luoaCMi2Gjuvwh8oRqay+QEkUH3rF/3FsS9l+yIyVA3TvEmO4Bo00+ma U4gyoFE/Z9+RLSGCVbFxFICipvQbkvPTeH+ASje/n7xlu7fvk47VlphmkqKnJYeOVw10 bMnQ== X-Forwarded-Encrypted: i=1; AJvYcCWIzdQ4dOokX20qE23so4R1MUXo/uWJMyCqNterspXVQ39nYOQktqKBMQ2WyAnJOCuAporTAw1R7bKAbJ9fVTga X-Gm-Message-State: AOJu0YxjJiiHAXKirBdcPYZ+M2MIeSbkC68RNNMSVK6Mb8SVKz0i5fpm 1aWRy9EcM2v9+LfCCH0CFC6Tsc1J/558eRIaFxes6Hve/tgGvPyzx0rcv0KD823RXpZDw149Tn7 ByPFsUxpCdIuA3tTN72MihBHvACnzwkn37v0+Qg== X-Google-Smtp-Source: AGHT+IG4KgBpg0EukW4Lq68BCYJHnSlbuBk0L5w6aIHLvNzkqN4W/bGgMqjstWZ5eO3EYqmNKpII2rxuSVuzGjx3r+s= X-Received: by 2002:a17:906:b199:b0:a4e:9a13:5090 with SMTP id w25-20020a170906b19900b00a4e9a135090mr267016ejy.9.1713390078338; Wed, 17 Apr 2024 14:41:18 -0700 (PDT) List-Id: Discussion related to FreeBSD architecture List-Archive: https://lists.freebsd.org/archives/freebsd-arch List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-freebsd-arch@FreeBSD.org MIME-Version: 1.0 References: <0100018ee9e8a381-2e0a8845-5321-4841-bfaf-184376e88112-000000@email.amazonses.com> In-Reply-To: From: Warner Losh Date: Wed, 17 Apr 2024 15:41:06 -0600 Message-ID: Subject: Re: enable INVARIANT_SUPPORT in GENERIC in release builds To: Konstantin Belousov Cc: Colin Percival , Lexi Winter , arch@freebsd.org Content-Type: multipart/alternative; boundary="000000000000fb43c5061651b65a" X-Spamd-Bar: ---- X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:15169, ipnet:2a00:1450::/32, country:US] X-Rspamd-Queue-Id: 4VKZ9D3r0Nz4ZBJ --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 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 > 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


=
On Wed, Apr 17, 2024 at 4:16=E2=80=AF= 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=E2=80=AFPM 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 w= on't load on
> > > a kernel without INVARIANT_SUPPORT.
> > >
> > > is there a reason INVARIANT_SUPPORT can't just be enable= d by default?
> >
> > I think while it had much lower overhead than INVARIANTS, there w= as still
> > a significant overhead cost at least in the early days.=C2=A0 May= be 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 m= odule with
> > > INVARIANTS.
> >
> > If the overhead cost of INVARIANT_SUPPORT is no longer relevant, = I'd be
> > fine with including it in stable/15.=C2=A0 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 t= hat 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 act= ually make things slow unless one or both of INVARIANTS and WITNESS are inc= luded.

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

#ifndef INVARIANTS
#define _l= ockmgr_assert(lk, what, file, line)
#endif

= which looks like it too requires INVARIANTS. What am I missing?
= =C2=A0
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=C2=A0
--000000000000fb43c5061651b65a--