phosphor-hwmon bottleneck potential
patrick at stwcx.xyz
Sat May 6 04:02:39 AEST 2017
Can you comment? I'm looking at the version that was merged into Linus'
master for 4.12:
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
-------------- next part --------------
A non-text attachment was scrubbed...
Size: 833 bytes
Desc: Digital signature
More information about the openbmc