From owner-svn-src-all@FreeBSD.ORG Thu Mar 17 17:02:37 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id BCD6E106566C; Thu, 17 Mar 2011 17:02:31 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: Bruce Evans Date: Thu, 17 Mar 2011 13:02:14 -0400 User-Agent: KMail/1.6.2 References: <201103161609.p2GG98q6097329@svn.freebsd.org> <20110317234320.E1128@besplex.bde.org> In-Reply-To: <20110317234320.E1128@besplex.bde.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201103171302.16038.jkim@FreeBSD.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r219698 - head/sys/i386/include X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 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: Thu, 17 Mar 2011 17:02:37 -0000 On Thursday 17 March 2011 08:55 am, Bruce Evans wrote: > On Wed, 16 Mar 2011, Jung-uk Kim wrote: > > Log: > > Rework r219679. Always check CPU class at run-time to make it > > predictable. Unfortunately, it pulls in but > > it is small enough and namespace pollution is minimal, I hope. > > > > Pointed out by: bde > > Well, I don't like the namespace pollution, and the old code > carefully avoided it by using the tsc_present global. > > > Modified: head/sys/i386/include/cpu.h > > ================================================================= > >============= --- head/sys/i386/include/cpu.h Wed Mar 16 12:40:58 > > 2011 (r219697) +++ head/sys/i386/include/cpu.h Wed Mar 16 > > 16:09:08 2011 (r219698) @@ -39,6 +39,7 @@ > > /* > > * Definitions unique to i386 cpu support. > > */ > > +#include > > #include > > #include > > #include > > @@ -69,14 +70,13 @@ void swi_vm(void *); > > static __inline uint64_t > > get_cyclecount(void) > > { > > -#if defined(I486_CPU) || defined(KLD_MODULE) > > struct bintime bt; > > > > - binuptime(&bt); > > - return ((uint64_t)bt.sec << 56 | bt.frac >> 8); > > -#else > > + if (cpu_class == CPUCLASS_486) { > > + binuptime(&bt); > > + return ((uint64_t)bt.sec << 56 | bt.frac >> 8); > > + } > > return (rdtsc()); > > -#endif > > } > > > > #endif > > cpu_class shouldn't be used for anything, and might not be able to > correctly classify whether the CPU has a TSC. The cpu feature for > this should be used. Using it directly with the correct includes > would give considerably more namespace pollution (md_var.h for > cpu_feature and specialreg.h for CPUID_TSC). > > Although I said that tsc_present should go, it seems to be the best > thing to use here. Rename it __cpu_feature_TSC to reflect that it > is exactly (cpu_feature & CPUID_TSC) and put underscores in its > name so as to start being even more careful about namespace > pollution in cpu.h. Or redeclare cpu_feature and CPUID_TSC. Actually, I think this function must be move to machdep.c as a real function unless there is any serious objection. There is no reason to pollute this file for awful things like this. I know it was done because of performance reason in the past (such as CPU ticker, performance measurement, I guess.) but it is rarely used now and it does only one serious thing, i.e., early entropy seeding. There is a minor usage in SCTP for debugging but moving it to machdep.c won't hurt much, I think. What do you think? Jung-uk Kim