On 8/1/06, Jean Delvare <khali@linux-fr.org> wrote:> Disclaimer: I have no idea how the input interface works currently. And> I don't know what problem you are trying to solve. I just thought my> hwmon-oriented comments might help.

Your comments are highly relevant! Almost everything said in thisthread, apart from "where should we put battery readouts", isapplicable to hwmon.

The problem we're trying to solve is minimizing system load and timerinterrupts caused by apps that track kernel-issued readouts (e.g.,hwmon's), without the application making unnecessary assumptions aboutthe driver or vice versa.

> > 1. A new ioctl DELAYED_UPDATE, with parameters min_wait and> > min_fresh, meaning: "I want an a fresh readout. If I poll() this FD> > with POLLIN then send an input-ready event at time is T+min_wait, or> > when you have a readout that was received from the hardware at time> > T+min_fresh, whichever is *later*. Likewise if I select()".> > Here T is the time of the ioctl call and min_wait>=min_fresh.>> "A or B, whichever is later", effectively means "A and B". Or am I> missing something?

The equivlent phrasing using "and" is s follows:

"as soon as the current time is at least T+min_wait *and* you have areadout that was received from the hardware at time at leastT+min_fresh."

> I fail to see the difference between min_wait and min_fresh.

Here's an example illustrating all parameters.

Suppose the app does a DELAYED_UPDATE with min_wait=700 andmin_fresh=1000 at time T, and then poll()s with a timeout of 9999.

At time T+600 the driver receives update events (yeah, it' that kindof driver). If this is all that happens, the app will remain blockedfor 9999ms -- the requirement for a readout fresher than T+700 isnever fulfilled.

If the driver get a second event at time T+800, the app will beunblocked at time T+1000. If, instead, the driver gets the secondevent at time T+1200, the app will be unblocked immediately.

> The hwmon interface (sysfs now, procfs before) has been returning> cached values by defaut for years. Changing this at this point might be> confusing.

Yes. All those should be told to use O_NONBLOCK, or be delayed by onereadout cycle whenever they poll an attribute. How serious is this?

The essential problem is that we don't want drivers to query thehardware and cause timer interrups when nobody's listening, so cachedvalues can get arbitrarily stale. Thus, an out-of-the-blue read by a*must* wait for a refresh (i.e., hardware query). I don't see anon-kludgy way out of it.

> I don't see much benefit in waiting for updated values> compared to reading them from the cache. The driver knows better how> frequently it can read from the chip.

Yes, but the driver doesn't know how frequently apps *need* it to readfrom the chip.

Take the hdaps driver, for example. The hardware can provide freshreadouts at a rate of 500Hz. Some apps (e.g., hdapsd) actually need ahigh poll rate. Others (e.g., joystick emulation and hdaps-gl) worknicely with 20Hz. And the tp-theft app will be useful even with 1Hz,and may change its poll rate depending on circumstances. So, what isthe driver supposed to do?

In my proposal, the the driver and all app (implicitly) negotiate apoll rate which makes sense for all parties involved.

> And no hardware monitoring chip I> know of can tell when the monitored value has changed - you have to read> the chip registers to know.

Yes, this may not be relevant for traditional hwmon chips, but we'retrying to handle event-based data sources too. You might get an ACPIevent on temperature change, or a "critical temperature" interrupt(which flips a boolean sysfs attr) , etc. I can't think of a reallyconvincing example, but we certainly want an interface that willsupport such drivers transparently to userspace.

> > To illustrate, here's an example of a proper polling loop (sans error> > checking). This app wants to refresh its display when the data has> > changed, but not more often than once per second. It wants the> > readouts to be reasonly spaced: they should be obtained at least 700ms> > apart. And it needs to update its GUI every 3 seconds regardless of> > readouts.>> I don't see the point in the 700ms rule. If you don't want new data> more often than once per second, the readouts will be spaced by one> second, which implies > 700ms already.

Only on average. If you use min_wait=1000 and min_fresh=0, you mightbe seeing cached readouts that were obtained at times999,1001,2999,3001,4999,5001,...which is probably not what you want.