From owner-svn-src-all@freebsd.org Sat Apr 7 22:23:25 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D0540F8AD27; Sat, 7 Apr 2018 22:23:24 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from smtp-out-no.shaw.ca (smtp-out-no.shaw.ca [64.59.134.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "Client", Issuer "CA" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 27CB080EB9; Sat, 7 Apr 2018 22:23:23 +0000 (UTC) (envelope-from cy.schubert@cschubert.com) Received: from spqr.komquats.com ([70.67.125.17]) by shaw.ca with ESMTPA id 4wErfZLAmU5pn4wEtfWqM6; Sat, 07 Apr 2018 16:23:17 -0600 X-Authority-Analysis: v=2.3 cv=Tai4SyYh c=1 sm=1 tr=0 a=VFtTW3WuZNDh6VkGe7fA3g==:117 a=VFtTW3WuZNDh6VkGe7fA3g==:17 a=kj9zAlcOel0A:10 a=Kd1tUaAdevIA:10 a=VxmjJ2MpAAAA:8 a=JzwRw_2MAAAA:8 a=YxBL1-UpAAAA:8 a=6I5d2MoRAAAA:8 a=PPDduXdDeSw3b1VufPgA:9 a=CjuIK1q_8ugA:10 a=jDxBBm0fxBEA:10 a=7gXAzLPJhVmCkEl4_tsf:22 a=_bBvcJC8wCc67rcU61zu:22 a=Ia-lj3WSrqcvXOmTRaiG:22 a=IjZwj45LgO3ly-622nXo:22 Received: from slippy.cwsent.com (slippy [10.1.1.91]) by spqr.komquats.com (Postfix) with ESMTPS id 7B9317EC; Sat, 7 Apr 2018 15:23:13 -0700 (PDT) Received: from slippy.cwsent.com (localhost [127.0.0.1]) by slippy.cwsent.com (8.15.2/8.15.2) with ESMTP id w37MNDY0009251; Sat, 7 Apr 2018 15:23:13 -0700 (PDT) (envelope-from Cy.Schubert@cschubert.com) Received: from slippy (cy@localhost) by slippy.cwsent.com (8.15.2/8.15.2/Submit) with ESMTP id w37MNBQI009248; Sat, 7 Apr 2018 15:23:11 -0700 (PDT) (envelope-from Cy.Schubert@cschubert.com) Message-Id: <201804072223.w37MNBQI009248@slippy.cwsent.com> X-Authentication-Warning: slippy.cwsent.com: cy owned process doing -bs X-Mailer: exmh version 2.8.0 04/21/2012 with nmh-1.7.1 Reply-to: Cy Schubert From: Cy Schubert X-os: FreeBSD X-Sender: cy@cwsent.com X-URL: http://www.cschubert.com/ To: Cy Schubert cc: Bruce Evans , Jeff Roberson , Justin Hibbits , Jeff Roberson , src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r331369 - head/sys/vm In-Reply-To: Message from Cy Schubert of "Thu, 22 Mar 2018 23:15:29 -0700." <201803230615.w2N6FTMJ040628@slippy.cwsent.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Date: Sat, 07 Apr 2018 15:23:11 -0700 X-CMAE-Envelope: MS4wfLmTrm+kXE4yT2WiGQXNAxaj3sHgnMQZFvjqxVSvxkYVLBd6MEgCpgvAWJtQB15ClybSmQkj7bdD/0MScMA76TZvyufO9t/NHjHVN7BdSj0T64jXio1j VVJ+/Cdt4NGkiNEJQiDjO5fWMLZn9SZHEHfoK7HASabke9QE48OX/69GglPqdRaplXgWwQUPOTqM27fFBknAxfmWam7xMKwS0YZfrOntQCJ2cXYiNdRlEcze g7BTmvbuXwMlm1/QImkvIUwneCs1WMIyNNnoCuZMrhRhJQRISoPl79JJophX0nH/SkkFJhpdOTBWD3cB6JyaAuQjSwRVLRraACtWxI5tdPq/oKDWex8PSMYo AhdNIO2B7h78Zn2ygwWb5UMH/ABtbA== X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Apr 2018 22:23:25 -0000 In message <201803230615.w2N6FTMJ040628@slippy.cwsent.com>, Cy Schubert writes: > In message <20180323150709.H968@besplex.bde.org>, Bruce Evans writes: > > On Thu, 22 Mar 2018, Jeff Roberson wrote: > > > > > On Thu, 22 Mar 2018, Cy Schubert wrote: > > > > > >> It broke i386 too. > > > > > > I just did > > > TARGET_ARCH=i386 make buildworld > > > TARGET_ARCH=i386 make buildkernel > > > > > > This worked for me? > > >> > > >> Index: sys/vm/vm_reserv.c > > >> =================================================================== > > >> --- sys/vm/vm_reserv.c (revision 331399) > > >> +++ sys/vm/vm_reserv.c (working copy) > > >> @@ -45,8 +45,6 @@ > > >> > > >> #include > > >> #include > > >> -#include > > >> -#include > > >> #include > > >> #include > > >> #include > > >> @@ -55,6 +53,8 @@ > > >> #include > > >> #include > > >> #include > > >> +#include > > >> +#include > > >> #include > > >> #include > > >> > > >> This is because sys/i386/include/machine.h uses critical_enter() and > > >> critical_exit() which are defined in sys/systm.h. > > > > Wrong fix. I see you committed this. Now there are more bugs to fix. > > > > is a prerequisite for all kernel headers except > > , since it defines and declares things like KASSERT() and > > critical_enter() which might be used in other headers (except > > sys/param.h and its standard pollution). Sometimes sys/systm.h is > > included as undocumented namespace pollution in headers that are > > accidentally included before the (other) ones that use KASSERT(), etc. > > The headers that have this bug have it to work around bugs in .c files > > like the one above. It is more usual to have this bug by not including > > sys/systm.h at all than to have it by including it in a wrong order. > > Sorting it alphabetically almost always gives a wrong order. It must > > be included after sys/param.h and that must be included first. > > Agreed on alphabetic sorting. > > > > > It is a related bug to include only sys/types.h and not sys/param.h. > > This requires chumminess with the current implementation and all > > future implementations. sys/param.h provides certain undocumented > > but standard namespace pollution which might vary with the implementation, > > as necessary to satisfy some of the pollution requirements of all current > > and future implementations of other headers. (The pollution should be > > monotonically decreasing but it was only that for a few years about 20 > > years ago when I worked on fixing it.) .c files that include sys/types.h > > instead of sys/param.h have do some subset of the includes in sys/param.h. > > Since nothing is documented and the subset might depend on the arch and > > user options, it is hard to know the minimal subset. > > That's not the case here. sys/types.h is not included in this file but > point taken. > > > > > .c files that include sys/types.h tend to have lots of other #include > > bugs like not including sys/systm.h. Again it is hard to know the > > minimal replacement for sys/systm.h and its undocumented but standard > > pollution. It is a style bug to include both sys/types.h and sys/param.h. > > style(9) even explicitly forbids including both. It is a larger style > > bug to include the standard pollution in sys/systm.h direction. This > > includes especially and . These > > should be considered as being implemented in sys/systm.h, with the > > headers for them only and implementation detail. Similarly > > for . > > > > >> It built nicely on my amd64's though. > > > > amd64 apparently has more namespace pollution which breaks detection > > of the bug. But I couldn't find where it is. sys/systm.h isn't included > > nested in any amd64 or x86 headers. Apparently some amd64 option gives > > it. > > The reason is amd64 doesn't use critical_enter() and critical_exit() > because counter_enter() and counter_exit() are NOPs. The reason they > are NOPs in amd64 and not in i386 is not all i386 processors support > cmpxchg8b. It is only then that the critical_*() functions are called. > > > > > Bruce > > I can create a phabricator revision to clean this instance up and move > sys/systm.h just after sys/param.h. I'm just about to head out of town > so I'll create it after I get back, after April 4. > > Thank you for your input Bruce. Hi Bruce, Can you please give this a once over? diff --git a/sys/vm/vm_reserv.c b/sys/vm/vm_reserv.c index d8869e3bdbe..6d31d79da39 100644 --- a/sys/vm/vm_reserv.c +++ b/sys/vm/vm_reserv.c @@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$"); #include "opt_vm.h" #include +#include #include #include #include @@ -52,7 +53,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include -- Cheers, Cy Schubert FreeBSD UNIX: Web: http://www.FreeBSD.org The need of the many outweighs the greed of the few.