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 GPG KeyID: 0E572FDD