Re: phosphor-hwmon bottleneck potential

[Patrick Williams <patrick@stwcx.xyz>]

[-- Attachment #1: Type: text/plain, Size: 1902 bytes --]

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@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@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

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]