Project

General

Profile

Actions

Feature #12830

closed

Want centralized ksensor framework

Added by Robert Mustacchi over 1 year ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

To date, we have introduced a couple of different temperature sensors through a burgeoning series of character devices. This has been done in changes like #12137, #11273, and #11184. This has been useful, but there have been a few challenges with the current design. Mainly that it requires every driver to have a character device entry point and implement the same ioctls. Having a centralized means of doing this has a few advantages:

  • Reduces per-driver complexity to implement support for sensors by eliminating the requirements for implementing character device entry points
  • Centralizes the character device entry points into one implementation
  • Provides the same attach/detach retriggering that a normal character device does
  • Easily co-exists with the existing devices that have their own implementations
  • Makes it easier to test the general sensor path and framework

This also adds a test suite for this.


Related issues

Related to illumos gate - Feature #12835: Want support for I350 temperature sensorClosedRobert Mustacchi

Actions
Related to illumos gate - Feature #12831: temperature sensors could describe accuracyClosedRobert Mustacchi

Actions
Related to illumos gate - Feature #12832: topo support for generic PCI device temp sensorsClosedRobert Mustacchi

Actions
Actions #1

Updated by Robert Mustacchi over 1 year ago

  • Related to Feature #12835: Want support for I350 temperature sensor added
Actions #2

Updated by Robert Mustacchi over 1 year ago

  • Related to Feature #12831: temperature sensors could describe accuracy added
Actions #3

Updated by Robert Mustacchi over 1 year ago

  • Related to Feature #12832: topo support for generic PCI device temp sensors added
Actions #4

Updated by Robert Mustacchi over 1 year ago

  • Gerrit CR set to 724
Actions #5

Updated by Robert Mustacchi about 1 year ago

This was tested in a number of different avenues:

  • The basic ksensor test suite that I wrote for this (and the rest of the os tests).
  • I ran the extended stress test for 12-24 hours, which it survived. I used DTrace to verify that the stress test actually passed and then unloaded everything and panicked the system on purpose. I then searched for memory leaks and didn't find any.
  • I rewrote two drivers to use the new APIs (amdnbtemp, pchtemp in #12833 and #12834)
  • I added support to igb(7D) in #12835 which was pretty much only possible because of this.
  • I used the sensor framework to do a proof of concept of a rust kernel module
  • I verified that everything still works if you have drivers that don't implement the new framework. I tested this with amdf17nbdf and Ryan Zezeski tested this with coretemp.
  • I have a work in progress rewritten coretemp(7D) that I'll get out for more testing after this is done.
  • Toomas helpfully verified that all of tihs builds cleanly on SPARC
  • I verified that there was no problem doing an pkg update (via onu) from a system that already had the pkg:/driver/cpu/sensor package installed onto these bits which moved around some of the system components around.
Actions #6

Updated by Joshua M. Clulow about 1 year ago

  • Description updated (diff)
Actions #7

Updated by Electric Monk about 1 year ago

  • Status changed from New to Closed
  • % Done changed from 80 to 100

git commit 3ce5372277f4657ad0e52d36c979527c4ca22de2

commit  3ce5372277f4657ad0e52d36c979527c4ca22de2
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2020-07-12T14:24:19.000Z

    12830 Want centralized ksensor framework
    12831 temperature sensors could describe accuracy
    12832 topo support for generic PCI device temp sensors
    Reviewed by: Ryan Zezeski <ryan@zinascii.com>
    Reviewed by: Toomas Soome <toomas@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF