On Tue, Jul 20, 2010 at 04:15:44AM -0400, Jean Delvare wrote: > Hi Guenter, > > On Mon, 19 Jul 2010 08:27:52 -0700, Guenter Roeck wrote: > > On Mon, Jul 19, 2010 at 07:24:13AM -0400, Jean Delvare wrote: > > > On Mon, 19 Jul 2010 00:44:02 -0300, Henrique de Moraes Holschuh wrote: > > > > On Sun, 18 Jul 2010, Guenter Roeck wrote: > > > > > Why ? Just make the sysfs entry conditional. All you need to do is to detect > > > > > in the probe function if there is a package sensor, > > > > > > How? > > > > > One method would be to use rdmsr_safe_on_cpu() and try to read the register. > > If the read fails, the register isn't there, and the sensor is not supported. > > If it is that easy, and reading a non-existent MSR can't cause any > nasty side effect, then alright. > Should be, since the comment for rdmsr_safe_on_cpu() indicates that it should be used instead of rdmsr_on_cpu() if it is unknown if the register exists. > > > > > and only create the sysfs > > > > > entry if there is. Plus, make sure that it is only created once. > > > > > > As said above, this isn't trivial. If you just add the sysfs attribute > > > to existing hwmon devices, you'll have duplicates. If you don't want > > > duplicates, you'll get an asymmetry (an arbitrary hwmon entry will get > > > the extra temperature, the user will get inevitably confused by this) > > > and the hotplug code will be difficult to deal with. > > > > > > The most direct approach is probably to have a separate hwmon entry for > > > the package temperature. But I start feeling dizzy when looking at > > > these 8 coretemp hwmon entries I already have on my system, and adding > > > a couple more doesn't exactly please me. So it may be the right time to > > > start thinking of a different strategy - even though this comes with > > > backwards compatibility pain. > > > > > Personally I would prefer to have one entry per CPU, with one sensor per core. > > In other words, something like > > > > 1st CPU: > > > > coretemp-xxx-0000 > > temp1_input core 0 > > temp2_input core 1 > > .. > > tempN_input core N-1 > > tempN+1_input package temperature, if the sensor exists > > > > 2nd CPU: > > coretemp-xxx-0001 > > > > and so on. > > Yes, I think it would make a lot of sense. I was just not sure if we > could know the value of N before seeing all the cores, but apparently > there is a way to get that value, as /proc/cpuinfo has it (under "cpu > cores"). > Yes, that would be "((struct cpuinfo_x86 *)c)->booted_cores". Question is how that changes during hotplug, but that is just a matter of looking into the code. > So I would be perfectly fine with a patch implementing the above > proposal. Preferably two patches, BTW: one moving core temperatures to > the same hwmon entry, and a second one adding the package temperature. > Yes, I agree. > We will have to update libsensors 2 to prevent a regression here. > What's the highest count of cores for these CPUs? 4? > Search for "Intel, AMD to Continue Raising Chip Core Count". Intel talks about up to 10 cores per chip, AMD aboud up to 16. Current limit is 8 cores for Intel, and 12 for AMD. To be halfway safe, 16 should be good enough for the next couple of years. Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
