From owner-freebsd-arch@FreeBSD.ORG Mon Mar 7 20:41:21 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 45927106564A for ; Mon, 7 Mar 2011 20:41:21 +0000 (UTC) (envelope-from fjwcash@gmail.com) Received: from mail-gw0-f44.google.com (mail-gw0-f44.google.com [74.125.83.44]) by mx1.freebsd.org (Postfix) with ESMTP id F222F8FC0C for ; Mon, 7 Mar 2011 20:41:20 +0000 (UTC) Received: by gwaa18 with SMTP id a18so4331178gwa.17 for ; Mon, 07 Mar 2011 12:41:20 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=t9qJfetkCWpY8hohLb1RdGW5FnOQd8XsCwT4bhZcFPU=; b=LKBI8fRwWUQj8jFQlOM9sUckiMuPOy+zhbjWqKiGAnHZTiymjX0mgw702HdgUew0Fp Bya3tQ/yT2UciImFnWWwgTNqJ0F/UXuVpQIXYv9zj8Og/RvivpCyJkyuy1muH+sOYv+E AkY1Nf4JyGuj/99s+lZTl5WrJaHxEOmvLN0nI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=E/FncA+GDsuX1dOHaFhz15lahN0GQLPW9lKyJySWPVsDMDCdCbNS4NrGjy/pwT0BSu tdHWmwJa+cBiJJ82lGvdwyFHHlYVZcBENxKRRu7psdGWo5qjiHmSz/4lMTEoGN6YpPVU jLvXZ+GBbEXyT+nFe1Ldk5zZU4ZVNMJeO0yfk= MIME-Version: 1.0 Received: by 10.101.56.10 with SMTP id i10mr1583153ank.74.1299528878473; Mon, 07 Mar 2011 12:14:38 -0800 (PST) Received: by 10.100.125.14 with HTTP; Mon, 7 Mar 2011 12:14:38 -0800 (PST) In-Reply-To: <4D6BB5E3.6020408@freebsd.org> References: <4D6BB5E3.6020408@freebsd.org> Date: Mon, 7 Mar 2011 12:14:38 -0800 Message-ID: From: Freddie Cash To: Nathan Whitehorn Content-Type: text/plain; charset=UTF-8 Cc: freebsd-current Current , freebsd-sysinstall@freebsd.org, FreeBSD Arch Subject: Re: Request for review/testing: switching the default installer X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 07 Mar 2011 20:41:21 -0000 On Mon, Feb 28, 2011 at 6:49 AM, Nathan Whitehorn wrote: > BSDinstall has acquired at this point its final form (prior to a future > merge with pc-sysinstall), and I believe is ready to replace sysinstall on > the 9.0 snapshot ISOs. Barring any objections, I would like to pull this > switch 2 weeks from today, on the 14th of March. > Bug reports would be very appreciated at this time. There are three known > bugs currently, which will be fixed soon, so please don't report these: > error reporting is not graceful if there are no writable disks in the > system, you must select at least one optional component, and the doc build > is not currently connected to the releases. After much finnaggling and gnashing of teeth around hardware (not related to installer), I have managed to get a bootable 9-CURRENT image with BSDInstall, and used it to get a bootable install of FreeBSD 9-CURRENT. :) Here are my thought and experiences using the new installer. Things I really like: - that the install CD is a LiveCD with a fully functional system; while it won't replace a Frenzy CD, it's very close - very streamlined install without a lot of extra "fluff" that just gets skipped anyway (like everything underneath Standard in the first sysinstall screen) - the ability to use features like GPT, gmirror, zfs right from the get-go - the ability to drop to a fully functional shell at various stages of the install, with access to proper man pages Things that irritated me: - when you drop to a shell from the disk editor screen, it lists the instructions at the top, but then never repeats them ever again - if you get lost in the disk editor shell and type "exit" to get back to the disk editor ... it thinks you are finished partitioning and carries on with the install, which then errors out due to no writable filesystems, requiring you to restart the entire process - the disk editor is very limited, especially in its error handling; I found myself stuck in a loop trying to exit the screen without a / filesystem listed, but I was doing everything from the shell - screen flips between a nice blue background (the curses interface?) and a black background (running shell commands?) which is quite jarring and slightly confusing; - screen elements go from nicely centred (curses interface?) and then jump to the top-left corner of the screen (shell commands?) which is also quite jarring and slightly confusing The last two may be limitations in the curses setup? But it would be nice if "shell command" I/O could be centred like the rest, and if the background could remain a single colour. Not huge issues, just things that irritated me. :) Overall, I am quite impressed with the new installer, as it is *just* an installer and not a system configuration creator (or breaker) like sysinstall. Now that I understand the "new world order" of GPT-based partitioning and booting, I think I'm going to like FreeBSD 9.0 a heck of a lot. ... off to play with dedupe and other ZFSv28 goodies ... -- Freddie Cash fjwcash@gmail.com From owner-freebsd-arch@FreeBSD.ORG Wed Mar 9 09:08:22 2011 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2EAF3106566C for ; Wed, 9 Mar 2011 09:08:22 +0000 (UTC) (envelope-from alexander@leidinger.net) Received: from mail.ebusiness-leidinger.de (mail.ebusiness-leidinger.de [217.11.53.44]) by mx1.freebsd.org (Postfix) with ESMTP id DB5778FC14 for ; Wed, 9 Mar 2011 09:08:21 +0000 (UTC) Received: from outgoing.leidinger.net (p5B1551EE.dip.t-dialin.net [91.21.81.238]) by mail.ebusiness-leidinger.de (Postfix) with ESMTPSA id 3CDE184400E for ; Wed, 9 Mar 2011 09:50:15 +0100 (CET) Received: from webmail.leidinger.net (unknown [IPv6:fd73:10c7:2053:1::2:102]) by outgoing.leidinger.net (Postfix) with ESMTP id 92E7E26A0 for ; Wed, 9 Mar 2011 09:50:10 +0100 (CET) Received: (from www@localhost) by webmail.leidinger.net (8.14.4/8.13.8/Submit) id p298o5rS031074 for arch@freebsd.org; Wed, 9 Mar 2011 09:50:05 +0100 (CET) (envelope-from Alexander@Leidinger.net) Received: from pslux.ec.europa.eu (pslux.ec.europa.eu [158.169.9.14]) by webmail.leidinger.net (Horde Framework) with HTTP; Wed, 09 Mar 2011 09:50:04 +0100 Message-ID: <20110309095004.76506xalyjic5bwg@webmail.leidinger.net> Date: Wed, 09 Mar 2011 09:50:04 +0100 From: Alexander Leidinger To: arch@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; DelSp="Yes"; format="flowed" Content-Disposition: inline Content-Transfer-Encoding: 7bit User-Agent: Dynamic Internet Messaging Program (DIMP) H3 (1.1.4) X-EBL-MailScanner-Information: Please contact the ISP for more information X-EBL-MailScanner-ID: 3CDE184400E.A5B5F X-EBL-MailScanner: Found to be clean X-EBL-MailScanner-SpamCheck: not spam, spamhaus-ZEN, SpamAssassin (not cached, score=1.274, required 6, autolearn=disabled, RDNS_NONE 1.27) X-EBL-MailScanner-SpamScore: s X-EBL-MailScanner-From: alexander@leidinger.net X-EBL-MailScanner-Watermark: 1300265416.90076@aeHN3wLlAe18Ad/AjJPxww X-EBL-Spam-Status: No Cc: Subject: Reasons against adding KDTRACE_HOOKS to GENERIC? X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Mar 2011 09:08:22 -0000 Hi, are there reasons which prevent the addition of KDTRACE_HOOKS (and makeoptions WITH_CTF=yes) to GENERIC? AFAIR the KDTRACE_HOOKS part was specially (re)written to not include CDDL code into the kernel and allow to load all the CDDL parts as a module. I also assume that the data the ctf tools add to the object files falls not within the license of the tool themself. In case we get the critical mass in favour of this, which architectures (besides amd64 and i386) are supported? Bye, Alexander. -- Where is John Carson now that we need him? -- RLG http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137 From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 17:42:30 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BF9841065678 for ; Thu, 10 Mar 2011 17:42:30 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 570BB8FC18 for ; Thu, 10 Mar 2011 17:42:29 +0000 (UTC) Received: by wyf23 with SMTP id 23so2025563wyf.13 for ; Thu, 10 Mar 2011 09:42:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:date:x-google-sender-auth :message-id:subject:from:to:content-type; bh=RwscBag9zn0vfNoIUT9pe/1iwrFpfYciBQRuWy5oumQ=; b=I4/QvEyOf1R9Oj+YyLuPzujTW6wB0nar0gSH2/vDcjrCs+ip9uh/clQ7WfcrmhdTrc NIMp0v9DSdzzvE2iPtZyWGpFtdaskVkBDC5wAIrhb788mGGUESwc683PHlg16mIBn1W2 /4YFiRuCrDaZDZlpWKhpp45HbLG6U4LSPtZCc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:date:x-google-sender-auth:message-id:subject :from:to:content-type; b=PXQun78APoQck1BX/IOBIrnTSUEjyB0rek4h3tA22zhbPAI4cUoGVjpOZQB/tPwW4O faKfFLs8KR7rVqtmFHF8LbFVoG0n/s9oFBxeYyBpgfZ6kIvkU8MxDLa+ZlXNi1uCkvJ6 dDzdBJGOjj+venE+lfBp3lUwFR4hX+1urFwPs= MIME-Version: 1.0 Received: by 10.216.203.168 with SMTP id f40mr6168380weo.25.1299777448735; Thu, 10 Mar 2011 09:17:28 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.216.62.130 with HTTP; Thu, 10 Mar 2011 09:17:28 -0800 (PST) Date: Thu, 10 Mar 2011 09:17:28 -0800 X-Google-Sender-Auth: pj_7MtP0Ov5sxdSTjDg4fhdplxg Message-ID: From: mdf@FreeBSD.org To: FreeBSD Arch Content-Type: text/plain; charset=ISO-8859-1 Subject: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 17:42:30 -0000 I recall a recent discussion/PR about nested includes in the context of and being a few of the only ones allowed. However, I don't see anything in style(9) about this. One rule for nested includes I've heard (that FreeBSD doesn't use, but just mentioning it for variety) is that every header file should compile on its own; that is, a .c file with nothing but #include should compile without errors. This rule also makes some sense with the FreeBSD style convention of alphabetizing includes; otherwise it's kinda just happenstance that e.g. comes alphabetically before , , and , and barely before . Now we come to the reason I ask. I'm working on a patch to change the static sysctl code to use the standard SYSININT/SYSUNINIT code rather than have special treatment in kern_linker.c, but to do this I need to either change quite a few places that include , or include instead of in sysctl.h, as the SI_SUB_SYSCTLS value isn't visible otherwise. This, though, shows a bug in , where it uses the constant MAXPATHLEN in the extern declaration of kernelname[] (which itself I think is not standard C, IIRC, but there are several uses of sizeof(kernelname) that don't work otherwise). So then needs to build. So, which files are okay to include in an include file? Does someone (I'll do it if we can agree on a list) want to put this in style(9)? Thanks, matthew From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 18:35:21 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 8D4CF1065758 for ; Thu, 10 Mar 2011 18:35:21 +0000 (UTC) (envelope-from imp@bsdimp.com) Received: from harmony.bsdimp.com (bsdimp.com [199.45.160.85]) by mx1.freebsd.org (Postfix) with ESMTP id 3D03E8FC14 for ; Thu, 10 Mar 2011 18:35:21 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by harmony.bsdimp.com (8.14.3/8.14.1) with ESMTP id p2AIO97H087193 for ; Thu, 10 Mar 2011 11:24:11 -0700 (MST) (envelope-from imp@bsdimp.com) Message-ID: <4D791749.9020803@bsdimp.com> Date: Thu, 10 Mar 2011 11:24:09 -0700 From: Warner Losh User-Agent: Mozilla/5.0 (X11; U; FreeBSD amd64; en-US; rv:1.9.2.13) Gecko/20101211 Thunderbird/3.1.7 MIME-Version: 1.0 To: freebsd-arch@freebsd.org References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 18:35:21 -0000 On 03/10/2011 10:17, mdf@freebsd.org wrote: > I recall a recent discussion/PR about nested includes in the context > of and being a few of the only ones > allowed. However, I don't see anything in style(9) about this. > > One rule for nested includes I've heard (that FreeBSD doesn't use, but > just mentioning it for variety) is that every header file should > compile on its own; that is, a .c file with nothing but #include > should compile without errors. This rule also makes some > sense with the FreeBSD style convention of alphabetizing includes; > otherwise it's kinda just happenstance that e.g. comes > alphabetically before,, and > , and barely before. We've explicitly not done that to date. In the past, this would cause extra I/O and processing that would slow down compile times by a measurable amount. These days, I think you'd be hard pressed to even measure the effect. However, if we adopted this rule, you tend to get n^2 behavior because each layer of the onion redundantly would be including the inner onion... A survey of the include files shows uneven application of rules. Some require additional includes before them (the most common being sys/param.h or sys/types.h), some include some prereques, etc. There's no hard and fast rule that will describe the current state of affairs. > Now we come to the reason I ask. I'm working on a patch to change the > static sysctl code to use the standard SYSININT/SYSUNINIT code rather > than have special treatment in kern_linker.c, but to do this I need to > either change quite a few places that include, or > include instead of in sysctl.h, as > the SI_SUB_SYSCTLS value isn't visible otherwise. > > This, though, shows a bug in, where it uses the > constant MAXPATHLEN in the extern declaration of kernelname[] (which > itself I think is not standard C, IIRC, but there are several uses of > sizeof(kernelname) that don't work otherwise). So then > needs to build. It is cascade problems like this which both militate for the change and against it. For the change because it causes less churn (in theory). Against the change because you tend to accumulate lots of things that used to be required, but are no longer, but which consumers come to bogusly depend on, making it hard to detangle the thicket. An interesting number of include files assume that param.h has already been included, and avoids the extra parsing overhead that skipping param.h N times would cause. It likely is better to fix the 6 places in the kernel where sizeof is used than to redundantly include param.h. Also, while you're there, fix the strncpy to a strlcpy: ./kern/kern_mib.c: kernelname, sizeof kernelname, "Name of kernel file booted"); ./sun4v/sun4v/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./sparc64/sparc64/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./powerpc/aim/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./amd64/amd64/machdep.c: strlcpy(kernelname, env, sizeof(kernelname)); ./ia64/ia64/machdep.c: strncpy(kernelname, p, sizeof(kernelname) - 1); > So, which files are okay to include in an include file? Does someone > (I'll do it if we can agree on a list) want to put this in style(9)? I doubt it would be wise to make a blanket statement like all headers must be self-contained. There's name-space contamination issues to worry about, etc. Warner From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 19:51:23 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5D21D106564A; Thu, 10 Mar 2011 19:51:23 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 3492A8FC16; Thu, 10 Mar 2011 19:51:23 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id C36F846B89; Thu, 10 Mar 2011 14:51:22 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.10]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 41B378A027; Thu, 10 Mar 2011 14:51:22 -0500 (EST) From: John Baldwin To: freebsd-arch@freebsd.org Date: Thu, 10 Mar 2011 14:46:37 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201103101446.37589.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Thu, 10 Mar 2011 14:51:22 -0500 (EST) Cc: mdf@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 19:51:23 -0000 On Thursday, March 10, 2011 12:17:28 pm mdf@freebsd.org wrote: > I recall a recent discussion/PR about nested includes in the context > of and being a few of the only ones > allowed. However, I don't see anything in style(9) about this. bde@ is probably the most authoritative. My understanding is that the only nested includes allowed in sys/sys/*.h are the two listed above and any header that starts with an underscore (sys/_mutex.h, etc.). The underscore variants were added to allow nested includes when absolutely necessary, but those includes are the bare minimum required to define structures, etc. > Now we come to the reason I ask. I'm working on a patch to change the > static sysctl code to use the standard SYSININT/SYSUNINIT code rather > than have special treatment in kern_linker.c, but to do this I need to > either change quite a few places that include , or > include instead of in sysctl.h, as > the SI_SUB_SYSCTLS value isn't visible otherwise. Hmm, what is the reason to use SYSINIT's instead of a dedicated linker set? -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 20:11:01 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 742AD1065678; Thu, 10 Mar 2011 20:11:01 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-ww0-f50.google.com (mail-ww0-f50.google.com [74.125.82.50]) by mx1.freebsd.org (Postfix) with ESMTP id CD8798FC16; Thu, 10 Mar 2011 20:11:00 +0000 (UTC) Received: by wwc33 with SMTP id 33so2495940wwc.31 for ; Thu, 10 Mar 2011 12:10:59 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=NS1VLriqxlWP9WmJUwlLz5Uf4ZzCEnb9pWnKSDRdDFs=; b=DrNAi36Ml9EoITPq/STbfW64lTBo6LTuf98u0QXSeQ9zV2GfINRZ0BL+qhWPTHXxSq TyYf+Mu7/R0L5ziNpuIjMb7aoA2x15pRmg0CfDvMulLXdW05oUFgO/uXDON3IZdIeGWG BUtjEMmJakRCZdntqYkzZBPOABbE0T54/wMmg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=dXR3QioJ8yQX5wYykULyk7fipCxMnATKKqnbtqMJazP+HVSimg0J+PO0wWMKLNv/tW ho2IILQ9lUz6dERHYnzfZSxeTrdtCvTRFZq4scl7gd3yhdK1ukWs1bTTyYCaNtboVfvj MW37j7Up/7HG0y0FL57XTxoxnWdjPV0vH1NP0= MIME-Version: 1.0 Received: by 10.216.141.225 with SMTP id g75mr3878223wej.10.1299787858811; Thu, 10 Mar 2011 12:10:58 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.216.62.130 with HTTP; Thu, 10 Mar 2011 12:10:58 -0800 (PST) In-Reply-To: <201103101446.37589.jhb@freebsd.org> References: <201103101446.37589.jhb@freebsd.org> Date: Thu, 10 Mar 2011 12:10:58 -0800 X-Google-Sender-Auth: NjaLT1_E1sU-xgoswljWf1KZjaU Message-ID: From: mdf@FreeBSD.org To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 20:11:01 -0000 On Thu, Mar 10, 2011 at 11:46 AM, John Baldwin wrote: > On Thursday, March 10, 2011 12:17:28 pm mdf@freebsd.org wrote: >> I recall a recent discussion/PR about nested includes in the context >> of and being a few of the only ones >> allowed. =A0However, I don't see anything in style(9) about this. > > bde@ is probably the most authoritative. =A0My understanding is that the = only > nested includes allowed in sys/sys/*.h are the two listed above and any h= eader > that starts with an underscore (sys/_mutex.h, etc.). =A0The underscore va= riants > were added to allow nested includes when absolutely necessary, but those > includes are the bare minimum required to define structures, etc. > >> Now we come to the reason I ask. =A0I'm working on a patch to change the >> static sysctl code to use the standard SYSININT/SYSUNINIT code rather >> than have special treatment in kern_linker.c, but to do this I need to >> either change quite a few places that include , or >> include instead of in sysctl.h, as >> the SI_SUB_SYSCTLS value isn't visible otherwise. > > Hmm, what is the reason to use SYSINIT's instead of a dedicated linker se= t? Mostly for consistency. The DB_COMMAND linker set was changed to use SYSINITs for version 8, and AFIAK SYSCTL is the only global kernel thing using a separate linker set. There's also a minor bug in initialization ordering where a static SYSCTL_PROC could use a lock initialized by SX_SYSINIT or MTX_SYSINIT, but at runtime module load the sysctl is exposed before the SI_SUB_LOCK stage has run, so in theory someone doing sysctl -a would crash the kernel on an attempt to lock an uninitialized mtx/sx. We saw this happen once at Isilon. Thanks, matthew From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 20:28:20 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 994C8106564A; Thu, 10 Mar 2011 20:28:20 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 6CDF18FC12; Thu, 10 Mar 2011 20:28:20 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id EBAE846B89; Thu, 10 Mar 2011 15:28:19 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.10]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 799B68A01B; Thu, 10 Mar 2011 15:28:19 -0500 (EST) From: John Baldwin To: mdf@freebsd.org Date: Thu, 10 Mar 2011 15:28:18 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: <201103101446.37589.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201103101528.18987.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Thu, 10 Mar 2011 15:28:19 -0500 (EST) Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 20:28:20 -0000 On Thursday, March 10, 2011 3:10:58 pm mdf@freebsd.org wrote: > On Thu, Mar 10, 2011 at 11:46 AM, John Baldwin wrote: > > On Thursday, March 10, 2011 12:17:28 pm mdf@freebsd.org wrote: > >> I recall a recent discussion/PR about nested includes in the context > >> of and being a few of the only ones > >> allowed. However, I don't see anything in style(9) about this. > > > > bde@ is probably the most authoritative. My understanding is that the only > > nested includes allowed in sys/sys/*.h are the two listed above and any header > > that starts with an underscore (sys/_mutex.h, etc.). The underscore variants > > were added to allow nested includes when absolutely necessary, but those > > includes are the bare minimum required to define structures, etc. > > > >> Now we come to the reason I ask. I'm working on a patch to change the > >> static sysctl code to use the standard SYSININT/SYSUNINIT code rather > >> than have special treatment in kern_linker.c, but to do this I need to > >> either change quite a few places that include , or > >> include instead of in sysctl.h, as > >> the SI_SUB_SYSCTLS value isn't visible otherwise. > > > > Hmm, what is the reason to use SYSINIT's instead of a dedicated linker set? > > Mostly for consistency. The DB_COMMAND linker set was changed to use > SYSINITs for version 8, and AFIAK SYSCTL is the only global kernel > thing using a separate linker set. That was because DB commands in modules just didn't work at all before. :) > There's also a minor bug in initialization ordering where a static > SYSCTL_PROC could use a lock initialized by SX_SYSINIT or MTX_SYSINIT, > but at runtime module load the sysctl is exposed before the > SI_SUB_LOCK stage has run, so in theory someone doing sysctl -a would > crash the kernel on an attempt to lock an uninitialized mtx/sx. We > saw this happen once at Isilon. Hmm, this is a legitimate reason, though I'd be tempted to fix that by just registering sysctls after sysinit's have been invoked and vice versa on unload. It seems that would be a simpler fix with far less code churn and not having to deal with the nested include mess, etc. One thing I often do, btw when dealing these sorts of interdependencies in a loadable module, is to use a single SYSINIT or module event handler to do all the setup and teardown of a single kld. This lets you handle errors far better (SYSINIT's don't allow for that at all) and more explicitly order the set of operations. -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Thu Mar 10 20:45:05 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 09588106566C; Thu, 10 Mar 2011 20:45:05 +0000 (UTC) (envelope-from mdf356@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id 649608FC1D; Thu, 10 Mar 2011 20:45:04 +0000 (UTC) Received: by wyf23 with SMTP id 23so2227258wyf.13 for ; Thu, 10 Mar 2011 12:45:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=bbZ4KTgX0WGbM1E+NkWZRelAEk7If18mGqvSp0cmdzw=; b=xmUDvupPVWi378mKlibc1DDqQZ/qLXuJ5Wm0NQz4obQbzy0bL2yLvuQmOJNhOTHwoj LS8XBKmsptImb8kd8cNZKGRm8Oo+NJejMK67Fq1AUWnotY7FnuAagtYffPUJKNLtklXr YcPZCFGEMfSVznkvEGj/brnoRqi9ycsw3Xy2A= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=lnX2vkJDvI0PDfh2KtJD80QBP7f/7Ol+OfgR2nq/w1UkF/Jel5xQmkLJqJOz25YNLK LYsEjIQ0vKAIoJoYWcsTtc4RQBmaw+ODgSFTpgg6ams6RCB8A3uVj5GBUU/5ese66j0P 2ZE9u66CyWthp36sZyGTkNTL22ub6O3Wh/tpQ= MIME-Version: 1.0 Received: by 10.216.244.7 with SMTP id l7mr6396379wer.40.1299789903469; Thu, 10 Mar 2011 12:45:03 -0800 (PST) Sender: mdf356@gmail.com Received: by 10.216.62.130 with HTTP; Thu, 10 Mar 2011 12:45:03 -0800 (PST) In-Reply-To: <201103101528.18987.jhb@freebsd.org> References: <201103101446.37589.jhb@freebsd.org> <201103101528.18987.jhb@freebsd.org> Date: Thu, 10 Mar 2011 12:45:03 -0800 X-Google-Sender-Auth: rPiUz2Ufu6j4Vh0x3ESQl0SKsd4 Message-ID: From: mdf@FreeBSD.org To: John Baldwin Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2011 20:45:05 -0000 On Thu, Mar 10, 2011 at 12:28 PM, John Baldwin wrote: > On Thursday, March 10, 2011 3:10:58 pm mdf@freebsd.org wrote: >> On Thu, Mar 10, 2011 at 11:46 AM, John Baldwin wrote: >> > On Thursday, March 10, 2011 12:17:28 pm mdf@freebsd.org wrote: >> >> I recall a recent discussion/PR about nested includes in the context >> >> of and being a few of the only ones >> >> allowed. =A0However, I don't see anything in style(9) about this. >> > >> > bde@ is probably the most authoritative. =A0My understanding is that t= he only >> > nested includes allowed in sys/sys/*.h are the two listed above and an= y header >> > that starts with an underscore (sys/_mutex.h, etc.). =A0The underscore= variants >> > were added to allow nested includes when absolutely necessary, but tho= se >> > includes are the bare minimum required to define structures, etc. >> > >> >> Now we come to the reason I ask. =A0I'm working on a patch to change = the >> >> static sysctl code to use the standard SYSININT/SYSUNINIT code rather >> >> than have special treatment in kern_linker.c, but to do this I need t= o >> >> either change quite a few places that include , or >> >> include instead of in sysctl.h, as >> >> the SI_SUB_SYSCTLS value isn't visible otherwise. >> > >> > Hmm, what is the reason to use SYSINIT's instead of a dedicated linker= set? >> >> There's also a minor bug in initialization ordering where a static >> SYSCTL_PROC could use a lock initialized by SX_SYSINIT or MTX_SYSINIT, >> but at runtime module load the sysctl is exposed before the >> SI_SUB_LOCK stage has run, so in theory someone doing sysctl -a would >> crash the kernel on an attempt to lock an uninitialized mtx/sx. =A0We >> saw this happen once at Isilon. > > Hmm, this is a legitimate reason, though I'd be tempted to fix that by ju= st > registering sysctls after sysinit's have been invoked and vice versa on > unload. =A0It seems that would be a simpler fix with far less code churn = and > not having to deal with the nested include mess, etc. I'm not committed to committing this change; I want to see how it looks when finished and run it by -arch. But I hit the nested include file problem first. :-) Changing the sysctl to be after all sysinit I *think* runs into a problem if one has a mix of SYSCTL_ADD_FOO and SYSCTL_FOO on a new static node defined in a loadable module, but I'd have to test that. That is, one would be trying to add a node as a child of a node that hasn't been set up. I think it may work today because the list of child nodes of a static node is actually a separate entity that exists at compile time. Cheers, matthew From owner-freebsd-arch@FreeBSD.ORG Fri Mar 11 06:38:31 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A8C88106564A for ; Fri, 11 Mar 2011 06:38:31 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from fallbackmx07.syd.optusnet.com.au (fallbackmx07.syd.optusnet.com.au [211.29.132.9]) by mx1.freebsd.org (Postfix) with ESMTP id 6582E8FC14 for ; Fri, 11 Mar 2011 06:38:29 +0000 (UTC) Received: from mail07.syd.optusnet.com.au (mail07.syd.optusnet.com.au [211.29.132.188]) by fallbackmx07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id p2B4Ax1l019330 for ; Fri, 11 Mar 2011 15:10:59 +1100 Received: from c122-107-125-80.carlnfd1.nsw.optusnet.com.au (c122-107-125-80.carlnfd1.nsw.optusnet.com.au [122.107.125.80]) by mail07.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id p2B4AmeY005366 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 11 Mar 2011 15:10:50 +1100 Date: Fri, 11 Mar 2011 15:10:48 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Warner Losh In-Reply-To: <4D791749.9020803@bsdimp.com> Message-ID: <20110311141613.N1478@besplex.bde.org> References: <4D791749.9020803@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Mar 2011 06:38:31 -0000 On Thu, 10 Mar 2011, Warner Losh wrote: > On 03/10/2011 10:17, mdf@freebsd.org wrote: >> I recall a recent discussion/PR about nested includes in the context >> of and being a few of the only ones >> allowed. However, I don't see anything in style(9) about this. This is not really a style matter, but a policy for avoiding namespace pollution. >> One rule for nested includes I've heard (that FreeBSD doesn't use, but >> just mentioning it for variety) is that every header file should >> compile on its own; that is, a .c file with nothing but #include >> should compile without errors. This rule also makes some >> sense with the FreeBSD style convention of alphabetizing includes; >> otherwise it's kinda just happenstance that e.g. comes >> alphabetically before,, and >> , and barely before. > > We've explicitly not done that to date. In the past, this would cause extra > I/O and processing that would slow down compile times by a measurable amount. > These days, I think you'd be hard pressed to even measure the effect. Actually, the effect is relatively larger, since i/o is relatively slower. It is easiest to see on nfs file systems since close-to-open consistency makes open() a very slow operation even if the file is cached, since it requires an RPC which takes many tens or hundreds or thousands of microseconds. FreeBSD GENERIC kernels now have over 100 thousands includes (up from 10-20 thousand in ~1998 when I stopped trying to keep up with header pollution after having reduced the number of includes by several thousands). On ref9-i386 now, the ping latency is ~130 usec (not bad), and this results in "make depend" on a kernel with 93128 words in .depend (I use this word count as an approximation to the number of includes) taking "45.91 real 19.33 user 8.31 sys" from cold and "37.46 real 18.95 user 8.91 sys" from warm. My reference FreeBSD-4 kernel with a similar config on a slightly slower CPU but not on nfs takes " 5.65 real 4.88 user 0.75 sys" from warm. It is helped by having only 24319 words in .depend and other missing bloat. The nfs latency accounts for about half of the extra real time in -current. It can be ameliorated by building in parallel with many more threads than CPUS so that not all threads stall on the same RPCs at the same time (usually), but a single "make depend" is mostly serial. > However, if we adopted this rule, you tend to get n^2 behavior because each > layer of the onion redundantly would be including the inner onion... This can be reduced by flattening the include hierarchy, but that is harder. > A survey of the include files shows uneven application of rules. Some > require additional includes before them (the most common being sys/param.h or > sys/types.h), some include some prereques, etc. There's no hard and fast > rule that will describe the current state of affairs. sys/param.h is almost required in the kernel. The other examples, even ones that carefully arrange to use only sys/types.h, can be considered as style bugs. Using only sys/types.h is actually not very careful, since all kernel headers are permitted to depend on sys/param.h (and maybe sys/systm.h, for KASSERT()); whether they actually do is a function of time which cannot reasonably be tracked in .c files wanting to include only . >> Now we come to the reason I ask. I'm working on a patch to change the >> static sysctl code to use the standard SYSININT/SYSUNINIT code rather >> than have special treatment in kern_linker.c, but to do this I need to >> either change quite a few places that include, or >> include instead of in sysctl.h, as >> the SI_SUB_SYSCTLS value isn't visible otherwise. >> >> This, though, shows a bug in, where it uses the >> constant MAXPATHLEN in the extern declaration of kernelname[] (which >> itself I think is not standard C, IIRC, but there are several uses of >> sizeof(kernelname) that don't work otherwise). So then >> needs to build. It just depends on the standard pollution from . (MAXPATHLEN is acually a system parameter, so it is not even pollution in itself). This allows to provide a complete declaration for `kernelname', so that kernelname can be used without worrying about what to include to what it is. > It is cascade problems like this which both militate for the change and > against it. For the change because it causes less churn (in theory). > Against the change because you tend to accumulate lots of things that used to > be required, but are no longer, but which consumers come to bogusly depend > on, making it hard to detangle the thicket. Wanting to detangle this is the main reason that I don't like nested includes. There should be no new ones. No one would repeat the mistakes (pollution and nested includes) made in old headers :-). > An interesting number of include files assume that param.h has already been > included, and avoids the extra parsing overhead that skipping param.h N times > would cause. The parsing overhead seems to be small relative to i/o. I just checked that gcc doesn't repeat the opens for nested includes. (I actually checked 2 non-nested includes of using ktrace. There was only 1 NAMI with "stdio.h".) I knew that gcc is smart about this. Even with a multiply-included header "foo.h" that acts differently for each include, there is only 1 NAMI. gcc apparently caches the whole header. > It likely is better to fix the 6 places in the kernel where sizeof is used > than to redundantly include param.h. Also, while you're there, fix the > strncpy to a strlcpy: I don't like this much (see another reply). NGROUPS in ucred.h and user.h used to have a similar but more interesting (large) problem because the APIs using it are exported to userland. This went away when NGROUPS was replaced by the old NGROUPS in these old APIs. It is now hard-coded as 16 in both, but spelled XU_NGROUPS and KINGROUPS. But the existence of these macros is a related mistake -- someone might use them instead of sizeof() on the array. Bruce From owner-freebsd-arch@FreeBSD.ORG Fri Mar 11 12:36:02 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 85A94106566B; Fri, 11 Mar 2011 12:36:02 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 426398FC15; Fri, 11 Mar 2011 12:36:02 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id BCBE746B03; Fri, 11 Mar 2011 07:36:01 -0500 (EST) Received: from jhbbsd.localnet (unknown [209.249.190.10]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 5A1608A01B; Fri, 11 Mar 2011 07:36:01 -0500 (EST) From: John Baldwin To: mdf@freebsd.org Date: Thu, 10 Mar 2011 16:25:42 -0500 User-Agent: KMail/1.13.5 (FreeBSD/7.4-CBSD-20110107; KDE/4.4.5; amd64; ; ) References: <201103101528.18987.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201103101625.42883.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.6 (bigwig.baldwin.cx); Fri, 11 Mar 2011 07:36:01 -0500 (EST) Cc: freebsd-arch@freebsd.org Subject: Re: style(9) rules for nested includes X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Mar 2011 12:36:02 -0000 On Thursday, March 10, 2011 3:45:03 pm mdf@freebsd.org wrote: > On Thu, Mar 10, 2011 at 12:28 PM, John Baldwin wrote: > > On Thursday, March 10, 2011 3:10:58 pm mdf@freebsd.org wrote: > >> On Thu, Mar 10, 2011 at 11:46 AM, John Baldwin wrote: > >> > On Thursday, March 10, 2011 12:17:28 pm mdf@freebsd.org wrote: > >> >> I recall a recent discussion/PR about nested includes in the context > >> >> of and being a few of the only ones > >> >> allowed. However, I don't see anything in style(9) about this. > >> > > >> > bde@ is probably the most authoritative. My understanding is that the only > >> > nested includes allowed in sys/sys/*.h are the two listed above and any header > >> > that starts with an underscore (sys/_mutex.h, etc.). The underscore variants > >> > were added to allow nested includes when absolutely necessary, but those > >> > includes are the bare minimum required to define structures, etc. > >> > > >> >> Now we come to the reason I ask. I'm working on a patch to change the > >> >> static sysctl code to use the standard SYSININT/SYSUNINIT code rather > >> >> than have special treatment in kern_linker.c, but to do this I need to > >> >> either change quite a few places that include , or > >> >> include instead of in sysctl.h, as > >> >> the SI_SUB_SYSCTLS value isn't visible otherwise. > >> > > >> > Hmm, what is the reason to use SYSINIT's instead of a dedicated linker set? > >> > >> There's also a minor bug in initialization ordering where a static > >> SYSCTL_PROC could use a lock initialized by SX_SYSINIT or MTX_SYSINIT, > >> but at runtime module load the sysctl is exposed before the > >> SI_SUB_LOCK stage has run, so in theory someone doing sysctl -a would > >> crash the kernel on an attempt to lock an uninitialized mtx/sx. We > >> saw this happen once at Isilon. > > > > Hmm, this is a legitimate reason, though I'd be tempted to fix that by just > > registering sysctls after sysinit's have been invoked and vice versa on > > unload. It seems that would be a simpler fix with far less code churn and > > not having to deal with the nested include mess, etc. > > I'm not committed to committing this change; I want to see how it > looks when finished and run it by -arch. But I hit the nested include > file problem first. :-) > > Changing the sysctl to be after all sysinit I *think* runs into a > problem if one has a mix of SYSCTL_ADD_FOO and SYSCTL_FOO on a new > static node defined in a loadable module, but I'd have to test that. > That is, one would be trying to add a node as a child of a node that > hasn't been set up. I think it may work today because the list of > child nodes of a static node is actually a separate entity that exists > at compile time. Yes, that's correct. Hmm, I'd be tempted to say that the modules that want to do what you do should use a single SYSINIT or module event handler to explicitly order all the dependencies and then use SYSCTL_ADD_*() in that routine. -- John Baldwin From owner-freebsd-arch@FreeBSD.ORG Fri Mar 11 13:25:42 2011 Return-Path: Delivered-To: arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1D86D106564A; Fri, 11 Mar 2011 13:25:42 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from mx1.sbone.de (bird.sbone.de [46.4.1.90]) by mx1.freebsd.org (Postfix) with ESMTP id C30358FC0C; Fri, 11 Mar 2011 13:25:41 +0000 (UTC) Received: from mail.sbone.de (mail.sbone.de [IPv6:fde9:577b:c1a9:31::2013:587]) (using TLSv1 with cipher ADH-CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by mx1.sbone.de (Postfix) with ESMTPS id 601A525D386D; Fri, 11 Mar 2011 13:15:05 +0000 (UTC) Received: from content-filter.sbone.de (content-filter.sbone.de [IPv6:fde9:577b:c1a9:31::2013:2742]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.sbone.de (Postfix) with ESMTPS id 7723F159A849; Fri, 11 Mar 2011 13:15:04 +0000 (UTC) X-Virus-Scanned: amavisd-new at sbone.de Received: from mail.sbone.de ([IPv6:fde9:577b:c1a9:31::2013:587]) by content-filter.sbone.de (content-filter.sbone.de [fde9:577b:c1a9:31::2013:2742]) (amavisd-new, port 10024) with ESMTP id Q92XUdKwoFNN; Fri, 11 Mar 2011 13:15:03 +0000 (UTC) Received: by mail.sbone.de (Postfix, from userid 66) id E8812159A79E; Fri, 11 Mar 2011 13:15:02 +0000 (UTC) Received: from maildrop.int.zabbadoz.net (maildrop.int.zabbadoz.net [10.111.66.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.int.zabbadoz.net (Postfix) with ESMTP id 0C4DC444A12; Fri, 11 Mar 2011 13:14:51 +0000 (UTC) Date: Fri, 11 Mar 2011 13:14:51 +0000 (UTC) From: "Bjoern A. Zeeb" X-X-Sender: bz@maildrop.int.zabbadoz.net To: Hajimu UMEMOTO In-Reply-To: Message-ID: <20110311130952.B3039@maildrop.int.zabbadoz.net> References: <20110220181713.C13400@maildrop.int.zabbadoz.net> X-OpenPGP-Key: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: arch@FreeBSD.org, FreeBSD current mailing list Subject: Re: CFR: importing openresolv X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Mar 2011 13:25:42 -0000 On Tue, 22 Feb 2011, Hajimu UMEMOTO wrote: Hi, >>>>>> On Tue, 22 Feb 2011 01:50:17 +0900 >>>>>> Hajimu UMEMOTO said: > > bz> Do you have an updated patch for 3.4.1 or 3.3.6? I'd like to help to > bz> you get it in for 9.0-R. I wouldn't even mind if some ports would > bz> conflict with it for a while not making the situation any worse than > bz> it is these days. > > ume> I didn't notice that openresolv was updated. I'll soon make a new > ume> diff for 3.4.1. > ume> hrs@ noticed that ppp(8) and uhsoctl(8) in our base tree touch > ume> /etc/resolv.conf. We need to change them to use resovlconf(8). > > I've updated a patch for 3.4.1: > > http://www.imasy.or.jp/~ume/FreeBSD/openresolv-20110222.diff.gz > > If you have the previous patch applied, make sure to remove > src/contrib/openresolv and src/sbin/resolvconf before applying new > one. I am still undecided if it's doing what I want it to do;) I am also undecided on the scripts in /libexec/resolvconf/, especially the ports-related ones. For dhclient-script I have found that, though not entirely right I needed + if [ -s $tmpres ]; then /sbin/resolvconf -a ${interface} < $tmpres + fi as it killed my resovl.conf otherwise with the empty input; obviously I should have my defaults in resolvconf.conf probably. We'll also need a man page describing the basics. I would love if there was a simple C interface to use rather than having to go and handle it independetly everywhere; a tiny lib just wrapping the basic calls. Not sure how Linux fixed that? I'll continue to experiment with it in various scenarios and I think it's the right way to go. /bz -- Bjoern A. Zeeb You have to have visions! Stop bit received. Insert coin for new address family. From owner-freebsd-arch@FreeBSD.ORG Sat Mar 12 18:52:12 2011 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 00569106566B; Sat, 12 Mar 2011 18:52:12 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.zoral.com.ua (mx0.zoral.com.ua [91.193.166.200]) by mx1.freebsd.org (Postfix) with ESMTP id DD1018FC08; Sat, 12 Mar 2011 18:52:10 +0000 (UTC) Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id p2CIabr0081680 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 12 Mar 2011 20:36:37 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4) with ESMTP id p2CIabRB045121; Sat, 12 Mar 2011 20:36:37 +0200 (EET) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.4/8.14.4/Submit) id p2CIab9H045120; Sat, 12 Mar 2011 20:36:37 +0200 (EET) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Sat, 12 Mar 2011 20:36:37 +0200 From: Kostik Belousov To: Tim Bird Message-ID: <20110312183637.GY78089@deviant.kiev.zoral.com.ua> References: <4D7AA4E4.3010508@am.sony.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="793AmKFHRt0DieWq" Content-Disposition: inline In-Reply-To: <4D7AA4E4.3010508@am.sony.com> User-Agent: Mutt/1.4.2.3i X-Spam-Status: No, score=-2.2 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_40, DNS_FROM_OPENWHOIS autolearn=no version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua Cc: freebsd-amd64@freebsd.org, freebsd-arch@freebsd.org Subject: Re: [PATCH] Add support for AVX X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 12 Mar 2011 18:52:12 -0000 --793AmKFHRt0DieWq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Mar 11, 2011 at 02:40:36PM -0800, Tim Bird wrote: > Hello all, >=20 > I am representing a group in Sony that wishes to submit the patch > below to add the described feature to AVX. I know this is somewhat > unconventional, but I am not the original author of the patch, nor > do I have a build or run environment for BSD. However, I have > been asked to be a proxy for our BSD developer to help shepherd > this patch into FreeBSD. If you have questions about the patch, > or issues with it, I will relay the information to the original > author and try to resolve any problems. >=20 > As I am new to FreeBSD, I'm not sure the correct individual or > group to direct this patch to, but hopefully someone can provide > me some pointers. I am starting with freebsd-amd64, but maybe this > should be sent to freebsd-hackers??? The list is fine, but freebsd-arch@ or freebsd-current@ would get your contribution a bigger exposure. I added arch@ to the Cc: due to some issues that I think are critical for the patch. >=20 > Thanks for any assistance you can provide. > -- Tim Bird, Principal Software Engineer > Sony Network Entertainment Thank you for taking interest in the FreeBSD and providing a contribution ! >=20 > --------------------------- > This is a patch that provides support for AVX in FreeBSD. After it is > applied to FreeBSD 9.0-CURRENT, user programs on it can use AVX fully > and also the kernel may use it by calling fpu_kern_enter(). AVX is the > newest SIMD extension for x86 CPUs introduced from Intel's Sandy Bridge > and also will be introduced by AMD's next generation processor Bulldozer. >=20 > This patch can be applied to FreeBSD 9.0-CURRENT r217297. (r217297 is a > revision of subversion in the repository http://svn.freebsd.org/base) > You can use clang to make a kernel with it as well as gcc. The kernel > runs on both amd64 processors either AVX has or not without any > configurations. >=20 > The strategy of the patch is the same as for FPU(MMX)/SSE. Namely, code = that > initializes FPU(MMX)/SSE also initializes AVX, when it saves them it also > saves AVX as well, and restores it too only if the processor has AVX. > If the processor doesn't have AVX, it does nothing and it only adds a > test instruction and a conditional branch instruction. >=20 > Actually it does the followings; > - When a processor boots if the processor has AVX then enable its use. > at sys/amd64/amd64/initcpu.c > - Save AVX context when a thread switching occurs. > at sys/amd64/amd64/cpu_switch.S > - Change fxsave()/fxrstor() to xsave()/xrstor() in order to save/restore = AVX > status whenever it saves/restores FPU(MMX)/SSE. > at sys/amd64/amd64/fpu.c > - Initializes AVX. > fpuinit() at sys/amd64/amd64/fpu.c > - Adjust an alignment of xsave/xrstor's target addrress in order to avoid= GP > exception due to miss alignment. > xsave/xrstor instructions require that a boundary of a target address is > 64B. But the alignment of a stack is 32B. So fpugetregs()/fpusetregs() = adjust > it via a bounce buffer before they issue xsave/xrstor instruction. > fpugetregs()/fpusetregs() at sys/amd64/amd64/fpu.c >=20 > There are two issues in the patch; > 1. Machine code is embedded > The default assembler for building a kernel, as of 2.15 doesn't know > instructions which is added for AVX. So, the patch embeds machine code f= or > xsetbv/xsave/xrstor instruction. This is fine, currently there is no other way to handle our old binutils. The issue is with licensing, and it is outside the project. >=20 > 2. Extra data copying > If an alignment of an address of a stack is less than 64B, in order > to avoid GP exception, it copies contents to/from a bounce buffer. > If callers of fpugetregs()/fpusetregs() can be changed, this extra > data coping is removed. Because of the callers are MI, I hesitated > about changing them. I think it is reasonable to require the struct fpu_kern_ctx, supplied to the fpu_kern_enter(), be properly aligned. In fact, the kernel malloc will provide aligned memory silently. I do not think that allocating struct fpu_kern_ctx on the stack is good practice, and definitely would object against such code entering our tree. The reason is that the structure is large, and kernel stack size is limited. The only left place which might require the copying with your patch is the struct pcb itself, pcb_user_save probably ends up unaligned for XSAVE use. This should be fixed in amd64/amd64/vm_machdep.c:cpu_fork() when calculating the pcb location in the memory block allocated for stack. Also, might be, the kernel stack size should be increased for amd64, to account for larger pcb. >=20 > Thanks, >=20 > Index: sys/amd64/include/ucontext.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/include/ucontext.h (revision 217297) > +++ sys/amd64/include/ucontext.h (working copy) > @@ -87,7 +87,7 @@ > /* > * See for the internals of mc_fpstate[]. > */ > - long mc_fpstate[64] __aligned(16); > + long mc_fpstate[104] __aligned(64); >=20 > __register_t mc_fsbase; > __register_t mc_gsbase; And this part is the most problematic for whole patch. The cause is that you changed the ABI of signal delivery and syscalls sigreturn(2), getcontext(2), swapcontext(2) and setcontext(2). Either some measures should be implemented to provide compatibility syscalls and renumber listed syscalls, or existing syscalls and signal frame layout should be changed to be backward-compatible while still provide the place to store XSAVE header and YMM register context. I would prefer to see the second option implemented. I thought about adding new flag to mc_flags for mcontext_t, lets call it _MC_XSAVE, setting of which cause using two words at the end mc_spare. The words would contain a len and pointer to the XSAVE area, added at the end of mcontext_t. The only issue with this scheme is getcontext(2) syscall, which probably requires much more complex treating. The lesser problem with the patch, comparing with the ABI breakage, is the Intel promise to add further bits into XCR0, requiring further breakage of the signal(2) and context(2) ABIs when more architectural cpu context state is added. I would prefer to have it handled once and for all, esp. if ABI changes are required. Also, it would be nice to have i386 taken care of, both in native variant, and COMPAT_FREEBSD32 case on amd64. The issues with i386 ABI breakage are similar to that of amd64. I wanted to work on YMM stuff, but I still do not own sandybridge system yet. I am very glad to see Sony (wow ! :) to contribute there. Will you work further on the items I listed ? If not, I intend to find the time after I have SB machine in posession. > Index: sys/amd64/include/fpu.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/include/fpu.h (revision 217297) > +++ sys/amd64/include/fpu.h (working copy) > @@ -51,6 +51,11 @@ > u_char xmm_bytes[16]; > }; >=20 > +/* Contents of each AVX extended accumulator */ > +struct ymmacc { > + u_char ymm_bytes[16]; > +}; > + > struct envxmm { > u_int16_t en_cw; /* control word (16bits) */ > u_int16_t en_sw; /* status word (16bits) */ > @@ -71,7 +76,9 @@ > } sv_fp[8]; > struct xmmacc sv_xmm[16]; > u_char sv_pad[96]; > -} __aligned(16); > + u_char xsv_hd[64]; > + struct ymmacc sv_ymm[16]; > +} __aligned(64); >=20 > #ifdef _KERNEL > struct fpu_kern_ctx { > Index: sys/amd64/include/signal.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/include/signal.h (revision 217297) > +++ sys/amd64/include/signal.h (working copy) > @@ -98,7 +98,7 @@ > */ > long sc_fpformat; > long sc_ownedfp; > - long sc_fpstate[64] __aligned(16); > + long sc_fpstate[104] __aligned(64); >=20 > long sc_fsbase; > long sc_gsbase; > Index: sys/amd64/include/specialreg.h > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/include/specialreg.h (revision 217297) > +++ sys/amd64/include/specialreg.h (working copy) > @@ -66,6 +66,7 @@ > #define CR4_PCE 0x00000100 /* Performance monitoring counter enable */ > #define CR4_FXSR 0x00000200 /* Fast FPU save/restore used by OS */ > #define CR4_XMM 0x00000400 /* enable SIMD/MMX2 to use except 16 */ > +#define CR4_XSAVE 0x00040000 /* enable XSETBV/XGETBV */ >=20 > /* > * Bits in AMD64 special registers. EFER is 64 bits wide. > @@ -134,6 +135,9 @@ > #define CPUID2_MOVBE 0x00400000 > #define CPUID2_POPCNT 0x00800000 > #define CPUID2_AESNI 0x02000000 > +#define CPUID2_XSAVE 0x04000000 > +#define CPUID2_OSXSAVE 0x08000000 > +#define CPUID2_AVX 0x10000000 >=20 > /* > * Important bits in the Thermal and Power Management flags > Index: sys/amd64/amd64/initcpu.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/amd64/initcpu.c (revision 217297) > +++ sys/amd64/amd64/initcpu.c (working copy) > @@ -142,6 +142,19 @@ > } > } >=20 > + > +static void > +xsetbv(unsigned int index, unsigned long value) > +{ > + register_t eax =3D value; > + register_t edx =3D value >> 32; > + > + /* xsetbv */ > + __asm __volatile(".byte 0x0f,0x01,0xd1" :: > + "a"(eax), "d"(edx), "c"(index)); > +} > + > + > /* > * Initialize CPU control registers > */ > @@ -154,6 +167,11 @@ > load_cr4(rcr4() | CR4_FXSR | CR4_XMM); > cpu_fxsr =3D hw_instruction_sse =3D 1; > } > + if (cpu_feature2 & CPUID2_XSAVE) { > + load_cr4(rcr4() | CR4_XSAVE); > + /* enable to use xsave/xrstor */ > + xsetbv(0, 7); > + } > if ((amd_feature & AMDID_NX) !=3D 0) { > msr =3D rdmsr(MSR_EFER) | EFER_NXE; > wrmsr(MSR_EFER, msr); > Index: sys/amd64/amd64/cpu_switch.S > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/amd64/cpu_switch.S (revision 217297) > +++ sys/amd64/amd64/cpu_switch.S (working copy) > @@ -115,7 +115,20 @@ > jne 1f > movq PCB_SAVEFPU(%r8),%r8 > clts > + > + /* if the cpu has AVX, use xsave instead of fxsave */ > + testl $CPUID2_XSAVE,cpu_feature2 > + jne 2f > fxsave (%r8) > + jmp 3f > +2: > + mov $0x7, %eax > + mov %rdx, %rbx > + mov $0x0, %edx > + /* xsave (%r8) fix me when xsave is available */ > + .byte 0x41, 0x0f, 0xae, 0x20 > + mov %rbx, %rdx > +3: > smsw %ax > orb $CR0_TS,%al > lmsw %ax > @@ -302,8 +315,80 @@ > * Update pcb, saving current processor state. > */ > ENTRY(savectx) > + /* Fetch PCB. */ > + movq %rdi,%rcx > + > /* Save caller's return address. */ > movq (%rsp),%rax > + movq %rax,PCB_RIP(%rcx) > + > + movq %cr3,%rax > + movq %rax,PCB_CR3(%rcx) > + > + movq %rbx,PCB_RBX(%rcx) > + movq %rsp,PCB_RSP(%rcx) > + movq %rbp,PCB_RBP(%rcx) > + movq %r12,PCB_R12(%rcx) > + movq %r13,PCB_R13(%rcx) > + movq %r14,PCB_R14(%rcx) > + movq %r15,PCB_R15(%rcx) > + > + /* > + * If fpcurthread =3D=3D NULL, then the fpu h/w state is irrelevant and= the > + * state had better already be in the pcb. This is true for forks > + * but not for dumps (the old book-keeping with FP flags in the pcb > + * always lost for dumps because the dump pcb has 0 flags). > + * > + * If fpcurthread !=3D NULL, then we have to save the fpu h/w state to > + * fpcurthread's pcb and copy it to the requested pcb, or save to the > + * requested pcb and reload. Copying is easier because we would > + * have to handle h/w bugs for reloading. We used to lose the > + * parent's fpu state for forks by forgetting to reload. > + */ > + pushfq > + cli > + movq PCPU(FPCURTHREAD),%rax > + testq %rax,%rax > + je 1f > + > + movq TD_PCB(%rax),%rdi > + leaq PCB_SAVEFPU(%rdi),%rdi > + clts > + > + /* if the cpu has AVX, use xsave instead of fxsave */ > + testl $CPUID2_XSAVE,cpu_feature2 > + jne 2f > + fxsave (%rdi) > + jmp 3f > +2: > + mov $0x7, %eax > + mov $0x0, %edx > + /* xsave (%rdi) fix me when xsave is available */ > + .byte 0x0f, 0xae, 0x27 > +3: > + smsw %ax > + orb $CR0_TS,%al > + lmsw %ax > + > + movq $PCB_SAVEFPU_SIZE,%rdx /* arg 3 */ > + leaq PCB_SAVEFPU(%rcx),%rsi /* arg 2 */ > + /* arg 1 (%rdi) already loaded */ > + call bcopy > +1: > + popfq > + > + ret > +END(savectx) > + > +/* > + * savectx2(xpcb) > + * Update xpcb, saving current processor state. > + */ > +ENTRY(savectx2) > + /* Fetch XPCB. */ > + movq %rdi,%r8 > + /* Save caller's return address. */ > + movq (%rsp),%rax > movq %rax,PCB_RIP(%rdi) >=20 > movq %rbx,PCB_RBX(%rdi) > @@ -355,7 +440,17 @@ > str PCB_TR(%rdi) >=20 > clts > + /* if the cpu has AVX, use xsave instead of fxsave */ > + testl $CPUID2_XSAVE,cpu_feature2 > + jne 1f > fxsave PCB_USERFPU(%rdi) > + jmp 2f > +1: > + mov $0x7, %eax > + mov $0x0, %edx > + /* xsave PCB_USERFPU(%rdi) fix me when xsave is available */ > + .byte 0x0f, 0xae, 0xa7, 0x00, 0x01, 0x00, 0x00 > +2:=09 > movq %rsi,%cr0 /* The previous %cr0 is saved in %rsi. */ >=20 > movl $1,%eax > Index: sys/amd64/amd64/fpu.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/amd64/fpu.c (revision 217297) > +++ sys/amd64/amd64/fpu.c (working copy) > @@ -105,6 +105,40 @@ >=20 > static struct savefpu fpu_initialstate; >=20 > + > +static void > +xsave(struct savefpu *addr) > +{ > + if (cpu_feature2 & CPUID2_XSAVE) { > + /* __asm("xsave %0" : "=3Dm" (*(addr))) -> xsave (%rdi) */ > + __asm __volatile("mov $0x7, %%eax\n" > + "mov $0x0, %%edx\n" > + ".byte 0x0f, 0xae, 0x27" > + :: "D"(addr) > + : "%eax", "%edx", "memory"); > + } else { > + fxsave(addr); > + } > +} > + > + > +static void > +xrstor(struct savefpu *addr) > +{ > + if (cpu_feature2 & CPUID2_XSAVE) { > + /* __asm("xrstor %0" : : "m" (*(addr))) -> xrstor (%rdi) */ > + __asm __volatile("mov $0x7, %%eax\n" > + "mov $0x0, %%edx\n" > + ".byte 0x0f, 0xae, 0x2f" > + :: "D"(addr) > + : "%eax", "%edx"); > + } else { > + fxrstor(addr); > + } > +} > + > + > + > /* > * Initialize the floating point unit. On the boot CPU we generate a > * clean state that is used to initialize the floating point unit when > @@ -128,13 +162,14 @@ > mxcsr =3D __INITIAL_MXCSR__; > ldmxcsr(mxcsr); > if (PCPU_GET(cpuid) =3D=3D 0) { > - fxsave(&fpu_initialstate); > + xsave(&fpu_initialstate); > if (fpu_initialstate.sv_env.en_mxcsr_mask) > cpu_mxcsr_mask =3D fpu_initialstate.sv_env.en_mxcsr_mask; > else > cpu_mxcsr_mask =3D 0xFFBF; > bzero(fpu_initialstate.sv_fp, sizeof(fpu_initialstate.sv_fp)); > bzero(fpu_initialstate.sv_xmm, sizeof(fpu_initialstate.sv_xmm)); > + bzero(fpu_initialstate.sv_ymm, sizeof(fpu_initialstate.sv_ymm)); > } > start_emulating(); > intr_restore(saveintr); > @@ -150,7 +185,7 @@ > critical_enter(); > if (curthread =3D=3D PCPU_GET(fpcurthread)) { > stop_emulating(); > - fxsave(PCPU_GET(curpcb)->pcb_save); > + xsave(PCPU_GET(curpcb)->pcb_save); > start_emulating(); > PCPU_SET(fpcurthread, 0); > } > @@ -423,7 +458,7 @@ > * the PCB doesn't contain a clean FPU state. Explicitly > * load an initial state. > */ > - fxrstor(&fpu_initialstate); > + xrstor(&fpu_initialstate); > if (pcb->pcb_initial_fpucw !=3D __INITIAL_FPUCW__) > fldcw(pcb->pcb_initial_fpucw); > if (PCB_USER_FPU(pcb)) > @@ -432,7 +467,7 @@ > else > set_pcb_flags(pcb, PCB_FPUINITDONE); > } else > - fxrstor(pcb->pcb_save); > + xrstor(pcb->pcb_save); > critical_exit(); > } >=20 > @@ -469,7 +504,26 @@ > } > critical_enter(); > if (td =3D=3D PCPU_GET(fpcurthread) && PCB_USER_FPU(pcb)) { > - fxsave(&pcb->pcb_user_save); > + /* > + * FIXME: > + * xsave requires that a boundary of a target address is 64B > + * and struct savefpu has an attribute for aligned 64B. But > + * gcc doesn't treat a variable on stack as 64B boundary. > + * So, it uses a buffer to do xsave safely and copy them the > + * target if a miss aligned address is passed. > + */ > + if ((unsigned long)&pcb->pcb_user_save % 64) { > + char tmp[sizeof(struct savefpu) + 63]; > + void *p =3D (void *)((unsigned long)(tmp + 63) & ~0x3f); > + > + bzero(((struct savefpu*)p)->xsv_hd, 64); > + > + xsave((struct savefpu*)p); > + bcopy(p, &pcb->pcb_user_save, sizeof(struct savefpu)); > + } else { > + bzero(pcb->pcb_user_save.xsv_hd, 64); > + xsave(&pcb->pcb_user_save); > + } > critical_exit(); > return (_MC_FPOWNED_FPU); > } else { > @@ -502,7 +556,24 @@ > pcb =3D td->td_pcb; > critical_enter(); > if (td =3D=3D PCPU_GET(fpcurthread) && PCB_USER_FPU(pcb)) { > - fxrstor(addr); > + > + /* In order to avoid #GP, initialize some of XSAVE.HEADER. */ > + u_long *header =3D (u_long*)(addr->xsv_hd); > + header[0] =3D 7; > + header[1] =3D 0; > + header[2] =3D 0; > + > + /* > + * FIXME: See fpugetregs() > + */ > + if ((unsigned long)addr % 64) { > + char tmp[sizeof(struct savefpu) + 63]; > + void *p =3D (void *)((unsigned long)(tmp + 63) & ~0x3f); > + bcopy(addr, p, sizeof(struct savefpu)); > + xrstor((struct savefpu*)p); > + } else { > + xrstor(addr); > + } > critical_exit(); > set_pcb_flags(pcb, PCB_FPUINITDONE | PCB_USERFPUINITDONE); > } else { > Index: sys/amd64/amd64/identcpu.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > --- sys/amd64/amd64/identcpu.c (revision 217297) > +++ sys/amd64/amd64/identcpu.c (working copy) > @@ -275,7 +275,7 @@ > "\012SSSE3" /* SSSE3 */ > "\013CNXT-ID" /* L1 context ID available */ > "\014" > - "\015" > + "\015FMA" > "\016CX16" /* CMPXCHG16B Instruction */ > "\017xTPR" /* Send Task Priority Messages*/ > "\020PDCM" /* Perf/Debug Capability MSR */ > @@ -291,7 +291,7 @@ > "\032AESNI" /* AES Crypto*/ > "\033XSAVE" > "\034OSXSAVE" > - "\035" > + "\035AVX" > "\036" > "\037" > "\040" >=20 >=20 >=20 > _______________________________________________ > freebsd-amd64@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-amd64 > To unsubscribe, send any mail to "freebsd-amd64-unsubscribe@freebsd.org" --793AmKFHRt0DieWq Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEARECAAYFAk17vTQACgkQC3+MBN1Mb4g45QCgywazhcRbcACHyMaBpog/ueXM MiMAnjRnrIo6LmnoZARyytIdtXVEwcSG =xt2a -----END PGP SIGNATURE----- --793AmKFHRt0DieWq--