phosphor-hwmon bottleneck potential

[Rick Altherr]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170505/acb43998/attachment-0001.html>