phosphor-hwmon bottleneck potential

[Patrick Venture]

For what it's worth, I'm experimenting with shorter measurement periods for
this specific driver.

Patrick

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
> 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/9db6882a/attachment.html>