On Thu, Oct 04, 2018 at 11:58:47AM -0700, Nicolin Chen wrote: > Hello, > > On Thu, Oct 04, 2018 at 09:48:02AM -0700, Guenter Roeck wrote: > > > > I would not object to adding trace support into the hwmon subsystem. > > > > However, it should be tied to the new API. I would resist patches > > > > adding trace support to individual hwmon drivers unless the new API > > > > is used and additional driver specific trace support is warranted. > > > > > > Yes, my idea is to implement it with the _info API inside the hwmon > > > core. What do you think about the mentioned solution? Would you be > > > in favor of a polling work queue? > > > > > > "----------. Similar to tz->poll_queue in thermal_core, hwmon core > > > could also have a work queue polling the registered sensor inputs > > > (by default disabled; enabled only if users configure poll_delay) > > > so that the power data can be generated to Ftrace outputs as well." > > > > > > > I am not really in favor of it. This goes well beyond tracing. Tracing > > by its nature should be non-invasive and impact the system as little as > > possible. Adding a thread which polls thermal sensors, which are often > > connected with a slow i2c interface or even blocking, is quite invasive. > > > > I don't mind adding tracing support to trace sensor access. Adding code > > to poll thermal sensors on a regular basis is a completely different > > beast. I am not convinced that this should really be done in the kernel. > > The same could be accomplished with a simple loop from userspace. > > I ain't 100% convinced either. I think at this point we can just > insert a trace event to the hwmon_attr_show(), unless there is a > substantial polling queue in the hwmon core as thermal_core has, > although I am not sure what would be a legit reason to add one. > > > while true; do > > cat /sys/class/hwmon/hwmon1/temp1_input > > sleep 1 > > done > > The power/perf folks were asking about something hands-free, as > neither thermal nor cpufreq requires extra readings or polling, > but I feel this should work for them too, reluctantly though. > ... the difference being that both are active kernel subsystems, meaning they do something on their own, while hwmon is by its nature passive unless triggered by userspace or, sometimes, interrupts. > > ... and you could actually trace those accesses in the kernel. > > > > Now, if the problem is added overhead due to requiring a sysfs access > > for each sensor read, we can discuss introducing a different and more > > efficient user-space ABI (such as adding a hwmon->iio bridge). > > That would however be a different discussion. > > Yea, that's beyond the topic yet it sounds more interesting for > certain people I guess, considering the fact that there are two > ina2xx drivers in both hwmon and iio subsystems. > This of course is quite undesirable, and a solution permitting support for both ABIs with a single driver would be quite useful. Guenter
