svn commit: r219698 - head/sys/i386/include

Jung-uk Kim jkim at FreeBSD.org
Thu Mar 17 17:02:37 UTC 2011


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 <machine/cputypes.h> 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 <machine/cputypes.h>
> > #include <machine/psl.h>
> > #include <machine/frame.h>
> > #include <machine/segments.h>
> > @@ -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


More information about the svn-src-head mailing list