From nobody Tue Jan 3 13:53:49 2023 X-Original-To: dev-commits-src-all@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 4NmZ2s47fFz2lTHM; Tue, 3 Jan 2023 13:53:57 +0000 (UTC) (envelope-from mike@karels.net) Received: from mail.karels.net (mail.karels.net [216.160.39.52]) by mx1.freebsd.org (Postfix) with ESMTP id 4NmZ2r712Wz3Fsx; Tue, 3 Jan 2023 13:53:56 +0000 (UTC) (envelope-from mike@karels.net) Authentication-Results: mx1.freebsd.org; none Received: from mail.karels.net (localhost [127.0.0.1]) by mail.karels.net (8.16.1/8.16.1) with ESMTP id 303Drnps012006; Tue, 3 Jan 2023 07:53:50 -0600 (CST) (envelope-from mike@karels.net) Received: from [10.0.2.130] ([10.0.1.1]) by mail.karels.net with ESMTPSA id vu82NG0ztGPkLgAA4+wvSQ (envelope-from ); Tue, 03 Jan 2023 07:53:49 -0600 From: Mike Karels To: Konstantin Belousov Cc: Mateusz Guzik , markj@freebsd.org, src-committers@freebsd.org, dev-commits-src-all@freebsd.org, dev-commits-src-main@freebsd.org Subject: Re: git: 68912701700c - main - ffs_suspend.c: clean up includes Date: Tue, 03 Jan 2023 07:53:49 -0600 X-Mailer: MailMate (1.14r5921) Message-ID: <09ECBA5E-D44B-41A8-8E16-F192E2D1237B@karels.net> In-Reply-To: References: <202212292056.2BTKuOIu047460@gitrepo.freebsd.org> List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain Content-Transfer-Encoding: quoted-printable X-Rspamd-Queue-Id: 4NmZ2r712Wz3Fsx X-Spamd-Bar: ---- X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:209, ipnet:216.160.36.0/22, country:US] X-Rspamd-Pre-Result: action=no action; module=replies; Message is reply to one we originated X-ThisMailContainsUnwantedMimeParts: N On 3 Jan 2023, at 5:53, Konstantin Belousov wrote: > On Mon, Jan 02, 2023 at 06:29:57PM +0100, Mateusz Guzik wrote: >> On 12/29/22, Konstantin Belousov wrote: >>> The branch main has been updated by kib: >>> >>> URL: >>> https://cgit.FreeBSD.org/src/commit/?id=3D68912701700ca3230f3e2d4b785= 8a038f884a327 >>> >>> commit 68912701700ca3230f3e2d4b7858a038f884a327 >>> Author: Konstantin Belousov >>> AuthorDate: 2022-12-28 18:17:53 +0000 >>> Commit: Konstantin Belousov >>> CommitDate: 2022-12-29 20:55:39 +0000 >>> >>> ffs_suspend.c: clean up includes >>> >>> Order includes alphabetically. >>> Remove unneeded sys/param.h, it is already included by sys/systm.= h. >>> >>> Reviewed by: mckusick >>> Sponsored by: The FreeBSD Foundation >>> MFC after: 1 week >>> Differential revision: https://reviews.freebsd.org/D37896 >>> --- >>> sys/ufs/ffs/ffs_suspend.c | 7 +++---- >>> 1 file changed, 3 insertions(+), 4 deletions(-) >>> >>> diff --git a/sys/ufs/ffs/ffs_suspend.c b/sys/ufs/ffs/ffs_suspend.c >>> index d13097109758..e7c976b6e921 100644 >>> --- a/sys/ufs/ffs/ffs_suspend.c >>> +++ b/sys/ufs/ffs/ffs_suspend.c >>> @@ -33,15 +33,14 @@ >>> #include >>> __FBSDID("$FreeBSD$"); >>> >>> -#include >>> #include >>> #include >>> -#include >>> -#include >>> -#include >>> #include >>> +#include >>> #include >>> +#include >>> #include >>> +#include >>> >>> #include >>> >>> >> >> tinderbox fails the KCSAN et al kernels: >> >> In file included from /usr/src/sys/ufs/ffs/ffs_suspend.c:36: >> In file included from /usr/src/sys/sys/systm.h:44: >> In file included from ./machine/atomic.h:73: >> /usr/src/sys/sys/atomic_san.h:117:24: error: unknown type name 'uint8_= t' >> ATOMIC_SAN_FUNCS(char, uint8_t); >> ^ >> it bisects to this commit >> > So the problem is that sys/systm.h includes machine/atomic.h which alwa= ys > had the pre-requisite sys/types.h, and this is documented in atomic(9).= > But sys/atomic_san.h uses C89 types in prototypes, not just macros, and= > this breaks for real. > > I can > 1. Just add sys/types.h to ufs_suspend.c (I prefer not) > 2. Add sys/types.h to sys/atomic_san.h. > 3. Add sys/types.h to sys/systm.h. > > IMO #2 is not the best solution, since it pollutes systm.h only sometim= es, > when the right kernel options are used. I would prefer #3, it seems, b= ut > want to ensure that there is a consensus about the addition to sys/syst= m.h. Or you could restore sys/param.h to ufs_suspend.c. Although systm.h incl= udes param.h, it does it too late; it is supposed to be first. Depending on systm.h to include param.h is therefore not reliable. And it violates style(9) to include both types.h and param.h. In my opinion, restoring param.h is the right fix. Mike