Re: [PATCH 1/3] ib core: Make device counter infrastructure dynamic

[Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

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

On 05/17/2016 01:00 PM, Jason Gunthorpe wrote:
> On Tue, May 17, 2016 at 12:00:39PM -0400, Doug Ledford wrote:
> 
>> operation O(n^2).  Since we can't break out mailbox commands to only
>> provide part of the data, I think we need to consider using a cached
>> struct for each device.  If the cached data is less than a certain age
>> on subsequent reads, we use the cached data.  If it's too old, we
>> discard it and get new data.
> 
> I noticed this too, but for sysfs reading I just felt it doesn't
> matter.

I prefer not to have O(n^2), even for sysfs.  You say it doesn't matter,
but if someone creates a script to check all of their stats via sysfs
once per second, and we start talking about mlx4 or mlx5 where they are
going to export a lot more variables, it eventually gets bad.

> Ultimately a followup patch should export the entire stats block
> through netlink as one operation

That's perfectly fine.

> and nothing should use sysfs except
> debugging.

Nobody manually checking on numbers themselves will use netlink.  And if
the stats are there, people will check them.  You can't depend on this
being used for debug access only.  If I recall correctly, ibstatus uses
all sysfs entries, and people would easily think that using it uses the
"preferred" method.  So, like I said, if it's there, it *will* get used,
and not just for debug,

> Keeping the driver interface simple seems valuable.

If you cache it in the core, the driver interface is simple.  And given
that the core is where the stats block is allocated and deallocated,
that's where the caching would need to be.  And it's not really that
hard.  I'll whip something up here in a bit...

> Caching is going to detrimental to apps that sync stats with external
> time. (which is almost every real-world app)

That's problematic with or without caching as the stats don't have a
timestamp, so scheduling delays could easily make the stats that you get
have a significant variance in time between the time they were retrieved
and when you actually processed them.  Are you suggesting we should go
ahead and add a timestamp to the stats output?

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]