On 8/24/2021 3:30 AM, Jason Gunthorpe wrote:
On Wed, Aug 18, 2021 at 02:24:21PM +0300, Mark Zhang wrote:
From: Aharon Landau <aharonl@xxxxxxxxxx>
Add an alloc_op_port_stats() API, as well as related structures, to support
per-port op_stats allocation during counter module initialization.
Signed-off-by: Aharon Landau <aharonl@xxxxxxxxxx>
Signed-off-by: Neta Ostrovsky <netao@xxxxxxxxxx>
Signed-off-by: Mark Zhang <markzhang@xxxxxxxxxx>
drivers/infiniband/core/counters.c | 18 ++++++++++++++++++
drivers/infiniband/core/device.c | 1 +
include/rdma/ib_verbs.h | 24 ++++++++++++++++++++++++
include/rdma/rdma_counter.h | 1 +
4 files changed, 44 insertions(+)
diff --git a/drivers/infiniband/core/counters.c b/drivers/infiniband/core/counters.c
index df9e6c5e4ddf..b8b6db98bfdf 100644
+++ b/drivers/infiniband/core/counters.c
@@ -611,6 +611,15 @@ void rdma_counter_init(struct ib_device *dev)
port_counter->hstats = dev->ops.alloc_hw_port_stats(dev, port);
if (!port_counter->hstats)
goto fail;
+
+ if (dev->ops.alloc_op_port_stats) {
+ port_counter->opstats =
+ dev->ops.alloc_op_port_stats(dev, port);
+ if (!port_counter->opstats)
+ goto fail;
It would be nicer to change the normal stats to have more detailed
meta information instead of adding an entire parallel interface like
this.
struct rdma_hw_stats {
struct mutex lock;
unsigned long timestamp;
unsigned long lifespan;
const char * const *names;
Change the names to a struct
const struct rdma_hw_stat_desc *descs;
struct rdma_hw_stat_desc {
const char *name;
unsigned int flags;
unsigned int private;
}
and then define a FLAG_OPTIONAL.
Then alot of this oddness goes away.
You might also need a small allocated bitmap to store the
enabled/disabled state
Then the series basically boils down to adding some 'modify counter'
driver op that flips the enable/disable flag
And the netlink patches to expose the additional information.
Maybe it can be defined like this:
struct rdma_stat_desc {
bool enabled;
const char *name;
u64 value;
};
struct rdma_hw_stats {
struct mutex lock; /* Protect lifespan and values[] */
unsigned long timestamp;
unsigned long lifespan;
int num_counters;
unsigned int private; // ?
u64 flags; // 0 or FLAG_OPTIONAL
struct rdma_stat_desc descs[];
//const char * const *names;
//u64 value[];
};
What does the "private" field mean? Driver-specific? Aren't all counters
driver-specific?
Thanks.
