From owner-svn-src-head@FreeBSD.ORG Sat Dec 4 09:44:59 2010 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B56D0106564A; Sat, 4 Dec 2010 09:44:59 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail04.syd.optusnet.com.au (mail04.syd.optusnet.com.au [211.29.132.185]) by mx1.freebsd.org (Postfix) with ESMTP id D2F8B8FC12; Sat, 4 Dec 2010 09:44:58 +0000 (UTC) Received: from c211-30-187-94.carlnfd1.nsw.optusnet.com.au (c211-30-187-94.carlnfd1.nsw.optusnet.com.au [211.30.187.94]) by mail04.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id oB49itMg013791 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Sat, 4 Dec 2010 20:44:56 +1100 Date: Sat, 4 Dec 2010 20:44:55 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Jung-uk Kim In-Reply-To: <201012031802.40083.jkim@FreeBSD.org> Message-ID: <20101204194458.C1665@besplex.bde.org> References: <201012032154.oB3LsADC035461@svn.freebsd.org> <201012031708.30232.jhb@freebsd.org> <201012031744.01956.jkim@FreeBSD.org> <201012031802.40083.jkim@FreeBSD.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, John Baldwin Subject: Re: svn commit: r216161 - in head/sys: amd64/amd64 i386/i386 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 04 Dec 2010 09:44:59 -0000 On Fri, 3 Dec 2010, Jung-uk Kim wrote: > On Friday 03 December 2010 05:43 pm, Jung-uk Kim wrote: >> On Friday 03 December 2010 05:08 pm, John Baldwin wrote: >>> On Friday, December 03, 2010 4:54:10 pm Jung-uk Kim wrote: >>>> Author: jkim >>>> Date: Fri Dec 3 21:54:10 2010 >>>> New Revision: 216161 >>>> URL: http://svn.freebsd.org/changeset/base/216161 >>>> >>>> Log: >>>> Explicitly initialize TSC frequency. To calibrate TSC >>>> frequency, we use DELAY(9) and it may use TSC in turn if TSC >>>> frequency is non-zero. >>> >>> We zero the BSS, so these were already zero. This just makes the >>> actual kernel file on disk larger by wasting space in .data >>> instead of .bss. >> >> Please note that I didn't touch other variables, e.g., >> tsc_is_broken, because I knew that. However, I just wanted to do >> that *explicitly*. Anyway, it is reverted now and SVN will remember >> what I wanted to do. ;-) >> >> BTW, if my memory serves, GCC (and all modern C compilers) put(s) >> zero-initialized variables back in .bss. Yes, this is just a style bug, since it doesn't even waste space :-). It still gives control over the layout, at least with gcc-3.3.3 on i386, but most places don't care about the layout. Why are you doing style bugs explicitly? :-) > I just tried it. GCC generates identical binaries as I thought. > However, Clang doesn't do the optimization. :-/ I get large differences even with gcc for assembler files (.zero and .p2align directives for explicit initialization -- this gives control over the layout) and small differences in .o files). BTW, at least i386 has some nonsense initialization related to this: from i386/initcpu.c: % /* Must *NOT* be BSS or locore will bzero these after setting them */ % int cpu = 0; /* Are we 386, 386sx, 486, etc? */ % u_int cpu_feature = 0; /* Feature flags */ % u_int cpu_feature2 = 0; /* Feature flags */ % u_int amd_feature = 0; /* AMD feature flags */ % u_int amd_feature2 = 0; /* AMD feature flags */ % u_int amd_pminfo = 0; /* AMD advanced power management info */ % u_int via_feature_rng = 0; /* VIA RNG features */ % u_int via_feature_xcrypt = 0; /* VIA ACE features */ % u_int cpu_high = 0; /* Highest arg to CPUID */ % u_int cpu_id = 0; /* Stepping ID */ % u_int cpu_procinfo = 0; /* HyperThreading Info / Brand Index / CLFUSH */ % u_int cpu_procinfo2 = 0; /* Multicore info */ % char cpu_vendor[20] = ""; /* CPU Origin code */ % u_int cpu_vendor_id = 0; /* CPU vendor ID */ % u_int cpu_clflush_line_size = 32; But all of these except the ones initialized to nonzero ARE in the BSS. The comment is wrong because locore does the bzeroing as early as possible so as to avoid this problem. It has done the bss bzeroing early enough since at least RELENG_3, and probably always in FreeBSD. Perhaps there was a problem with bzeroing NOT related to the bss (for page tables?), but it doesn't seem to be a problem now. The comment was added in 2001 not long before gcc invalidated it. BSS. These declarations also have many comments that do less than echo the variable names. ISTR than there are a few variables in locore that should be declared here but aren't, since they were put in locore for some bad reason. bootinfo may be one. Now, only the PC98 pc98_system_parameter variable seems to be written to by locore before the bzero. After fixing this, only easier control over the layout justifies locore allocating any non-static variable. I fixed the comment and removed the explicit initializations long ago. and haven't noticed any problems: % Index: initcpu.c % =================================================================== % RCS file: /home/ncvs/src/sys/i386/i386/initcpu.c,v % retrieving revision 1.49 % diff -u -2 -r1.49 initcpu.c % --- initcpu.c 10 Nov 2003 15:48:30 -0000 1.49 % +++ initcpu.c 11 Nov 2003 08:07:01 -0000 % @@ -77,15 +77,13 @@ % &hw_instruction_sse, 0, "SIMD/MMX2 instructions available in CPU"); % % -/* Must *NOT* be BSS or locore will bzero these after setting them */ % -int cpu = 0; /* Are we 386, 386sx, 486, etc? */ % -u_int cpu_feature = 0; /* Feature flags */ % -u_int cpu_high = 0; /* Highest arg to CPUID */ % -u_int cpu_id = 0; /* Stepping ID */ % -u_int cpu_procinfo = 0; /* HyperThreading Info / Brand Index / CLFUSH */ % -char cpu_vendor[20] = ""; /* CPU Origin code */ % - % +int cpu; /* Are we 386, 386sx, 486, etc? */ % +u_int cpu_feature; /* Feature flags */ % #ifdef CPU_ENABLE_SSE % u_int cpu_fxsr; /* SSE enabled */ % #endif % +u_int cpu_high; /* Highest arg to CPUID */ % +u_int cpu_id; /* Stepping ID */ % +u_int cpu_procinfo; /* HyperThreading Info / Brand Index / CLFUSH */ % +char cpu_vendor[20]; /* CPU Origin code */ % % #ifdef I486_CPU This fixes some style bugs (unsorted declarations) but not the comments. There many more variables now, with O(number of new variables) sorting errors. The largest sorting error is attaching a nonzero-initialized variable at the end of this list variables that is misclaimed to need special handling because they are zero. Bruce