Re: hwmon: trace event support?

[Guenter Roeck <linux@roeck-us.net>]

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