Date: Sun, 19 Aug 2007 02:45:39 GMT From: "Constantine A. Murenin" <cnst@FreeBSD.org> To: Perforce Change Reviews <perforce@FreeBSD.org> Subject: PERFORCE change 125328 for review Message-ID: <200708190245.l7J2jdxN040553@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=125328 Change 125328 by cnst@dale on 2007/08/19 02:45:16 make coretemp_get_temp() more robust in preparation for a patch converting coretemp(4) to sysctl(3) hw.sensors framework. If coretemp_get_temp() is called before smp is started, then it will generate a panic around sched_bind (at least on sched_4bsd). (This is what will happen if we try to load coretemp at boot time, and then shortly thereafter call coretemp_get_temp() from, for example, sensor_task thread.) To account for this, we are now going to check that the number of initialised processors is greater than 1, and return -1 if someone tries to read a temperature from an uninitialised CPU. Special thanks go to rpaulo@ for discussing this issue with me on IRC, and pointing me at smp_cpus. rpaulo suggested that I include <sys/smp.h>, and use #ifdef SMP around smp_cpus, but it turns out that: 1. when we compile coretemp(4) as a module, SMP is not defined 2. in <sys/smp.h>, smp_cpus is only extern'ed if SMP is defined 3. smp_cpus is declared regardless of SMP define in kern/subr_smp.c Thus a better solution is to use `extern int smp_cpus` at this time, although a fix for <sys/smp.h> might be better in the long-term. Discussed with rpaulo@, thanks also go to Nick Barkas of moduli.net for running publicly accessible OpenGrok service for FreeBSD. :) Affected files ... .. //depot/projects/soc2007/cnst-sensors/sys.dev.coretemp/coretemp.c#3 edit Differences ... ==== //depot/projects/soc2007/cnst-sensors/sys.dev.coretemp/coretemp.c#3 (text+ko) ==== @@ -50,6 +50,8 @@ #include <machine/cpufunc.h> #include <machine/md_var.h> +extern int smp_cpus; + struct coretemp_softc { device_t sc_dev; int sc_tjmax; @@ -208,9 +210,17 @@ int cpu = device_get_unit(dev); struct coretemp_softc *sc = device_get_softc(dev); - thread_lock(curthread); - sched_bind(curthread, cpu); - thread_unlock(curthread); + /* + * Bind to specific CPU to read the correct temperature. + * If not all CPUs are initialised, then only read from + * cpu0, returning -1 on all other CPUs. + */ + if (smp_cpus > 1) { + thread_lock(curthread); + sched_bind(curthread, cpu); + thread_unlock(curthread); + } else if (cpu != 0) + return (-1); /* * The digital temperature reading is located at bit 16
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200708190245.l7J2jdxN040553>