phosphor-hwmon bottleneck potential

[Robi Buranyi]

my 50 cents on this.

We could change the driver(s) to periodically poll the sensors like every
second, and store the latest value. Then the sysfs read shall always return
the last completed measurement data. We should do this for both the
tachometers reading and the temperature sensors. There shall be some logic
implemented for aging (when sensor read constantly fails)

On Fri, May 5, 2017 at 11:37 AM, Nancy Yuen <yuenn at google.com> wrote:

> 1. General design issue if time between reads of a sensor is dependent on
> the number of sensors in the system.  And drivers can fail, due to miss
> behaving code or hardware.  In this design, sensor report could be
> significantly delayed if one sensor/driver were bad or misbehaving.
>
> 2. So putting the onus on each application to implement timestamps is
> better design than having timestamps recorded by a central source, the
> source that actually is reading from the driver?
>
> ----------
> Nancy
>
> On Fri, May 5, 2017 at 11:05 AM, Rick Altherr <raltherr at google.com> wrote:
>
>> Jagha is currently on leave.
>>
>> On Fri, May 5, 2017 at 11:02 AM, Patrick Williams <patrick at stwcx.xyz>
>> wrote:
>>
>>> Jaghu,
>>>
>>> Can you comment?  I'm looking at the version that was merged into Linus'
>>> master for 4.12:
>>>
>>> commit 2d7a548a3eff382da5cd743670693b7657327714 <(765)%20732-7714>
>>> Author: Jaghathiswari Rankappagounder Natarajan <jaghu at google.com>
>>> Date:   Tue Apr 4 17:52:41 2017 -0700
>>>
>>>     drivers: hwmon: Support for ASPEED PWM/Fan tach
>>>
>>>
>>> On Fri, May 05, 2017 at 10:48:39AM -0700, Rick Altherr wrote:
>>> > I can't comment on the driver implementation choices.  They have been
>>> > reviewed and accepted by upstream.  I'm not sure what version of the
>>> driver
>>> > you are looking at.
>>> >
>>> > On Fri, May 5, 2017 at 10:43 AM, Patrick Williams <patrick at stwcx.xyz>
>>> wrote:
>>> >
>>> > > Rick,
>>> > >
>>> > > On Fri, May 05, 2017 at 09:48:01AM -0700, Rick Altherr wrote:
>>> > > > I've chatted with Patrick V. separately about the driver.
>>> AST2400/2500
>>> > > fan
>>> > > > tach hardware measures only one fan at a time.  I think we can
>>> adjust the
>>> > > > driver settings to reduce the measurement time but it will scale
>>> with #
>>> > > of
>>> > > > tachs being read.
>>> > >
>>> > > I never looked at this driver before but it looks like it is doing an
>>> > > 'msleep' in the hwmon read path after resetting a counter register
>>> and
>>> > > then counting rotations?  Two comments:
>>> > >
>>> > > 1. As it stands, it doesn't appear that this driver is actually
>>> > > multi-reader safe (either thread or process).  There is no locking or
>>> > > queueing to prevent one reader from resetting the result register
>>> while
>>> > > another is performing the msleep loop.  Multi-threading might "go
>>> > > faster" but it will give entirely wrong results by my reading.
>>> > >
>>> > > 2. It seems bad to do a long-running msleep in the hwmon read path to
>>> > > begin with.  Should this driver be restructured to have a kthread
>>> read
>>> > > the channels in the background on a polling interval instead of
>>> > > initiating by userspace action?
>>> > >
>>> > > --
>>> > > Patrick Williams
>>> > >
>>>
>>> --
>>> Patrick Williams
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170505/31a6ec04/attachment.html>