On Tue, 11 Jun 2024, Shravan Ramani wrote: > > > When 2 32-bit counters are coupled to form a 64-bit counter using this setting, > > > one counter will hold the lower 32 bits while the other will hold the upper 32. > > > So the other counter (or syses corresponding to it) also needs to be accessed. > > > > > > > For 64-bit counter, I suppose the userspace is expected to read the full > > > > counter from two sysfs files and combine the value (your documentation > > > > doesn't explain this)? That seems non-optimal, why cannot kernel just > > > > return the full combined 64-value directly in kernel? > > > > > > I will add more clear comments for this. > > > While it is true that the driver could combine the 2 fields and present a > > > 64-bit value via one of the sysfs, the reason for the current approach is that > > > there are other interfaces which expose the same counters for our platform > > > and there are tools that are expected to work on top of both interfaces for > > > the purpose of collecting performance stats. > > > > > The other interfaces follow this > > > approach of having lower and upper 32-bits separately in each counter, and > > > the tools expect the same. Hence the driver follows this approach to keep > > > things consistent across the BlueField platform. > > > > Hi, > > > > I went to look through the existing arrays in mlxbf-pmc.c but did not find > > any entries that would have clearly indicated the counters being hi/lo > > parts of the same counter. There were a few 0/1 ones which could be the > > same counter although I suspect even they are not parts of the same > > counter but two separate entities called 0 and 1 having the same counter. > > > > Could you please elaborate further what you meant with the note about > > other interfaces above so I can better assess the claim? > > When combining 2 counters using the "use_odd_counter" setting, the mechanism > of joining them or assigning upper or lower 32 bits to a counter is handled in HW > and not by the driver. For example, if bit0 of "use_odd_counter" is set, counter0 > and counter1 (which were originally separate counters) automatically become > the lower and upper bits of one 64-bit value. The user needs to read both these > sysfs separately to get the full 64-bit value. The driver does not do any special > handling for such cases, merely provides access to both counter0 and counter1. I know all this by now, but we're discussion here is whether kernel should do "special handling". Although, it's not really correct to depict representing 64-bit counter in its entirety as "special handling". I think the kernel should combine the 64-bit halved and you argumented it shouldn't. When I went to confirm the claim your argument was based on, I couldn't find on what basis the claim was made. > Since the events supported by the blocks are quite HW centric and low-level in > nature, the driver is generally used alongside various tools which work on top of > this driver to collect telemetry info and provide more readable statistics to the > end-user. Similar to this driver, there are other FW interfaces providing access to > these counters (same and other additional ones as well that belong to other HW > blocks). For the sake of consistency and to allow the tools to be compatible with > all interfaces, the counter data needs to be accessible in the same way, ie, as 32-bit > upper and lower values in counter0 and counter1 sysfs as in the above case. This does nothing to answer my question. Where in the kernel, there's an example where a 64-bit counter for BlueField platform is presented as 2 32-bit counters? If there isn't any examples in the kernel, your statement about consistency within the platform doesn't hold water, quoted (again) here for clarity what I'm refering to: "The other interfaces follow this approach of having lower and upper 32-bits separately in each counter, and the tools expect the same. Hence the driver follows this approach to keep things consistent across the BlueField platform." Where I can find those "other interfaces" that already follow this convention? -- i.
